Bug 75519 – [GTK] [AC] Introduce AcceleratedCompositingContext to isolate different accelerated compositing implementations

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Martin Robinson]

This class will isolate each of the implementations, hopefully preventing code conflicts as we post patches for acceleated compositing.

[Martin Robinson]

Created attachment 121064 [details]
Patch

[Alejandro G. Castro]

Comment on attachment 121064 [details]
Patch

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

Great patch.

> Source/WebKit/gtk/WebCoreSupport/AcceleratedCompositingContext.h:34
> +class TextureMapper;
> +class TextureMapperNode;

Should this be avoided in case of clutter compilation?

> Source/WebKit/gtk/WebCoreSupport/AcceleratedCompositingContextClutter.cpp:87
> +    if (rect == IntRect())
> +        m_rootGraphicsLayer->setNeedsDisplay();

Shouln't we return in this clause to avoid the next setNeedsDisplay with emtpy rect? Are we testing if rect.isEmpty()?

[Martin Robinson]

(In reply to comment #2)
> (From update of attachment 121064 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=121064&action=review
> 
> Great patch.
> 
> > Source/WebKit/gtk/WebCoreSupport/AcceleratedCompositingContext.h:34
> > +class TextureMapper;
> > +class TextureMapperNode;
> 
> Should this be avoided in case of clutter compilation?

Yep! This was left over from my work with TextureMapperGL. I've removed it.

> > Source/WebKit/gtk/WebCoreSupport/AcceleratedCompositingContextClutter.cpp:87
> > +    if (rect == IntRect())
> > +        m_rootGraphicsLayer->setNeedsDisplay();
> 
> Shouln't we return in this clause to avoid the next setNeedsDisplay with emtpy rect? Are we testing if rect.isEmpty()?

Right again on both counts. I've changed the test to rect.isEmpty() and added an early return.

Thanks for the review!

[Martin Robinson]

Committed r104194: <http://trac.webkit.org/changeset/104194>