Bug 142386 – [TexMap] Seperate BitmapTexture related classes implementations from TextureMapper

• Component: New Bugs
• Status: RESOLVED FIXED

[Gwang Yoon Hwang]

[TexMap] Seperate BitmapTexture related classes implementations from TextureMapper

[Gwang Yoon Hwang]

Created attachment 248048 [details]
Patch

[Zan Dobersek]

Comment on attachment 248048 [details]
Patch

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

Looks nice, the position of these classes always bothered me.

There's plenty of space for further cleanups, but this is a great start.

> Source/WebCore/platform/graphics/texmap/BitmapTexture.cpp:39
> +    std::unique_ptr<ImageBuffer> imageBuffer = ImageBuffer::create(targetRect.size());
> +    GraphicsContext* context = imageBuffer->context();

Unrelated to the patch, but this would desperately need some improvement. It's creating a new ImageBuffer and the related cairo surface, allocating the necessary memory each time it's called.

[Gwang Yoon Hwang]

(In reply to comment #2)
> Comment on attachment 248048 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=248048&action=review
> 
> Looks nice, the position of these classes always bothered me.
> 
> There's plenty of space for further cleanups, but this is a great start.
>

Not only clean ups, but also lots of improvement can be done after all. :)
 
> > Source/WebCore/platform/graphics/texmap/BitmapTexture.cpp:39
> > +    std::unique_ptr<ImageBuffer> imageBuffer = ImageBuffer::create(targetRect.size());
> > +    GraphicsContext* context = imageBuffer->context();
> 
> Unrelated to the patch, but this would desperately need some improvement.
> It's creating a new ImageBuffer and the related cairo surface, allocating
> the necessary memory each time it's called.

I agree, but in most of cases this method would not be called. I think in
Coordinated Graphics, we uses UpdateAtlas so we reuses ImageBuffer and Cairo surfaces,
but I need to check.

I think we need to remove TextureMapperImageBuffer and
remove all non-accelerated compositing code paths to simplify code paths.

[WebKit Commit Bot]

Comment on attachment 248048 [details]
Patch

Clearing flags on attachment: 248048

Committed r182101: <http://trac.webkit.org/changeset/182101>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

(In reply to comment #4)
> Comment on attachment 248048 [details]
> Patch
> 
> Clearing flags on attachment: 248048
> 
> Committed r182101: <http://trac.webkit.org/changeset/182101>

It made compositing tests crash on the EFL bot.

[Gwang Yoon Hwang]

(In reply to comment #6)
> (In reply to comment #4)
> > Comment on attachment 248048 [details]
> > Patch
> > 
> > Clearing flags on attachment: 248048
> > 
> > Committed r182101: <http://trac.webkit.org/changeset/182101>
> 
> It made compositing tests crash on the EFL bot.

Ossy, could you give me a more log about the crash?
I couldn't find any additional failure on EFL bot after this patch.

[Csaba Osztrogonác]

before:
- https://build.webkit.org/builders/EFL%20Linux%2064-bit%20Release%20WK2/builds/20777
- 46 fail, only 1 crash from it

after:
- https://build.webkit.org/builders/EFL%20Linux%2064-bit%20Release%20WK2/builds/20778
- 64 fails, 18 compositing test crashes from it

[Gwang Yoon Hwang]

(In reply to comment #8)
> before:
> -
> https://build.webkit.org/builders/EFL%20Linux%2064-bit%20Release%20WK2/
> builds/20777
> - 46 fail, only 1 crash from it
> 
> after:
> -
> https://build.webkit.org/builders/EFL%20Linux%2064-bit%20Release%20WK2/
> builds/20778
> - 64 fails, 18 compositing test crashes from it

Thanks for let me know.
I didn't noticed number changes. :/

But the problem is I cannot run WebKitEFL either using vm's driver or llvmpipe.
Do you know any of magic variable to run WebKitEFL?
I cannot enter to the opengl_x11 mode in EFL.

[Csaba Osztrogonác]

(In reply to comment #9)
> (In reply to comment #8)
> > before:
> > -
> > https://build.webkit.org/builders/EFL%20Linux%2064-bit%20Release%20WK2/
> > builds/20777
> > - 46 fail, only 1 crash from it
> > 
> > after:
> > -
> > https://build.webkit.org/builders/EFL%20Linux%2064-bit%20Release%20WK2/
> > builds/20778
> > - 64 fails, 18 compositing test crashes from it
> 
> Thanks for let me know.
> I didn't noticed number changes. :/
> 
> But the problem is I cannot run WebKitEFL either using vm's driver or
> llvmpipe.
> Do you know any of magic variable to run WebKitEFL?
> I cannot enter to the opengl_x11 mode in EFL.

Let's continue in bug143214. Unfortunately I don't have any 
time today, but I'll try to check it in details tomorrow.