Bug 143561 – Remove TextureMapperImageBuffer

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Csaba Osztrogonác]

It isn't maintained and doesn't work, see https://bugs.webkit.org/show_bug.cgi?id=143214#c20 for details.

[Csaba Osztrogonác]

Created attachment 250431 [details]
Patch

[Csaba Osztrogonác]

Created attachment 250432 [details]
Patch

removing setShouldSupportContentsTiling too

[Csaba Osztrogonác]

Maybe we can remove AccelerationMode everywhere. Am I right?

[Csaba Osztrogonác]

Created attachment 250433 [details]
Patch

removing accelerationMode + GTK buildfix

[Csaba Osztrogonác]

Created attachment 250435 [details]
Patch

more GTK fix

[Gwang Yoon Hwang]

(In reply to comment #3)
> Maybe we can remove AccelerationMode everywhere. Am I right?

Oh, yes. Please remove those!

[Csaba Osztrogonác]

Created attachment 250436 [details]
Patch

removing BitmapTextureImageBuffer too to fix the GTK build

[Csaba Osztrogonác]

Just a question is there anybody build with
USE(TEXTURE_MAPPER) && !USE(TEXTURE_MAPPER_GL) ?

[Gwang Yoon Hwang]

(In reply to comment #8)
> Just a question is there anybody build with
> USE(TEXTURE_MAPPER) && !USE(TEXTURE_MAPPER_GL) ?

I don't think so. Maybe we can merge TEXTURE_MAPPER_GL and TEXTURE_MAPPER

[Gwang Yoon Hwang]

Comment on attachment 250436 [details]
Patch

Looks good for me. I'll merge TextureMapper and TextureMapperGL in separate patch.

[Zan Dobersek]

Comment on attachment 250436 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=250436&action=review

> Source/WebCore/platform/graphics/texmap/TextureMapper.cpp:42
>      return platformCreateAccelerated();

platformCreateAccelerated() can be removed as well, perhaps in the patch that merges USE(TEXTURE_MAPPER) and USE(TEXTURE_MAPPER_GL).

[Zan Dobersek]

On the note of merging USE(TEXTURE_MAPPER) and USE(TEXTURE_MAPPER_GL), can the TextureMapper and TextureMapperGL (and the related classes) be merged as well?

[Gwang Yoon Hwang]

(In reply to comment #12)
> On the note of merging USE(TEXTURE_MAPPER) and USE(TEXTURE_MAPPER_GL), can
> the TextureMapper and TextureMapperGL (and the related classes) be merged as
> well?

Yes, we can merge it. But I think it is safe to do it in separated bug.

[Csaba Osztrogonác]

Unfortunately it didn't solve the problem, but introduced another
crashes, see https://bugs.webkit.org/show_bug.cgi?id=143214#c23
for details. I'll upload detailed results soon.

[Csaba Osztrogonác]

Created attachment 250512 [details]
results without the patch

[Csaba Osztrogonác]

Created attachment 250513 [details]
results with the patch

[Csaba Osztrogonác]

(In reply to comment #16)
> Created attachment 250513 [details]
> results with the patch

There are two major problem:
- Expected to fail, but passed: (267) - these tests don't pass at all in the
real life.
- Regressions: Unexpected crashes (72) - using TextureMapperGL with EFL which disabling GL isn't so good and cause this crashes

[Csaba Osztrogonác]

Created attachment 250515 [details]
crash log for accessibility/accessibility-node-memory-management.html

[Csaba Osztrogonác]

Created attachment 250516 [details]
llvmpipe experiment

I tried to adapt GTK's solution to use llvmpipe for XVFBDriver,
additionally force EFL to use GL with setting ELM_ENGINE=opengl_x11
and setting EVAS_GL_NO_BLACKLIST to force EFL not to blacklist
llvmpipe ... but I got crash at the beginning somewhere in EFL
when it called glGetString.

[Csaba Osztrogonác]

I don't have enough graphics skill and any EFL internal skill to fix 
this bug and bug143214 myself in the near future, so feel free to
pick it up, if anybody interested in fixing this bug.

[Csaba Osztrogonác]

Created attachment 252385 [details]
Patch

Patch for landing. No regression with USE_NATIVE_XDISPLAY=1, which is already set on the EFL bots, so it's safe to land it now.

[Csaba Osztrogonác]

(In reply to comment #21)
> Created attachment 252385 [details]
> Patch
> 
> Patch for landing. No regression with USE_NATIVE_XDISPLAY=1, which is
> already set on the EFL bots, so it's safe to land it now.

Landed in http://trac.webkit.org/changeset/183807