Bug 143561

Summary: Remove TextureMapperImageBuffer
Product: WebKit Reporter: Csaba Osztrogonác <ossy>
Component: New BugsAssignee: Csaba Osztrogonác <ossy>
Status: RESOLVED FIXED    
Severity: Normal CC: clopez, gyuyoung.kim, ossy, yoon, zan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=143214
https://bugs.webkit.org/show_bug.cgi?id=137842
Bug Depends on:    
Bug Blocks: 144654    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
zan: review+
results without the patch
none
results with the patch
none
crash log for accessibility/accessibility-node-memory-management.html
none
llvmpipe experiment
none
Patch none

Description Csaba Osztrogonác 2015-04-09 02:57:02 PDT
It isn't maintained and doesn't work, see https://bugs.webkit.org/show_bug.cgi?id=143214#c20 for details.
Comment 1 Csaba Osztrogonác 2015-04-09 02:57:51 PDT
Created attachment 250431 [details]
Patch
Comment 2 Csaba Osztrogonác 2015-04-09 03:08:05 PDT
Created attachment 250432 [details]
Patch

removing setShouldSupportContentsTiling too
Comment 3 Csaba Osztrogonác 2015-04-09 03:12:32 PDT
Maybe we can remove AccelerationMode everywhere. Am I right?
Comment 4 Csaba Osztrogonác 2015-04-09 03:33:01 PDT
Created attachment 250433 [details]
Patch

removing accelerationMode + GTK buildfix
Comment 5 Csaba Osztrogonác 2015-04-09 03:43:47 PDT
Created attachment 250435 [details]
Patch

more GTK fix
Comment 6 Gwang Yoon Hwang 2015-04-09 04:04:30 PDT
(In reply to comment #3)
> Maybe we can remove AccelerationMode everywhere. Am I right?

Oh, yes. Please remove those!
Comment 7 Csaba Osztrogonác 2015-04-09 05:23:20 PDT
Created attachment 250436 [details]
Patch

removing BitmapTextureImageBuffer too to fix the GTK build
Comment 8 Csaba Osztrogonác 2015-04-09 05:29:48 PDT
Just a question is there anybody build with
USE(TEXTURE_MAPPER) && !USE(TEXTURE_MAPPER_GL) ?
Comment 9 Gwang Yoon Hwang 2015-04-09 05:37:42 PDT
(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
Comment 10 Gwang Yoon Hwang 2015-04-09 22:30:04 PDT
Comment on attachment 250436 [details]
Patch

Looks good for me. I'll merge TextureMapper and TextureMapperGL in separate patch.
Comment 11 Zan Dobersek 2015-04-10 00:44:05 PDT
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).
Comment 12 Zan Dobersek 2015-04-10 00:44:59 PDT
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?
Comment 13 Gwang Yoon Hwang 2015-04-10 00:49:49 PDT
(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.
Comment 14 Csaba Osztrogonác 2015-04-10 06:01:44 PDT
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.
Comment 15 Csaba Osztrogonác 2015-04-10 06:11:12 PDT
Created attachment 250512 [details]
results without the patch
Comment 16 Csaba Osztrogonác 2015-04-10 06:12:08 PDT
Created attachment 250513 [details]
results with the patch
Comment 17 Csaba Osztrogonác 2015-04-10 06:15:14 PDT
(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
Comment 18 Csaba Osztrogonác 2015-04-10 06:16:12 PDT
Created attachment 250515 [details]
crash log for accessibility/accessibility-node-memory-management.html
Comment 19 Csaba Osztrogonác 2015-04-10 06:23:42 PDT
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.
Comment 20 Csaba Osztrogonác 2015-04-10 06:27:07 PDT
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.
Comment 21 Csaba Osztrogonác 2015-05-05 08:36:39 PDT
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.
Comment 22 Csaba Osztrogonác 2015-05-05 08:51:57 PDT
(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