[webkit-reviews] review granted: [Bug 69151] [Qt][WK2] Synchronize tiling with accelerated compositing : [Attachment 110383] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 11 03:19:58 PDT 2011


Kenneth Rohde Christiansen <kenneth at webkit.org> has granted Noam Rosenthal
<noam.rosenthal at nokia.com>'s request for review:
Bug 69151: [Qt][WK2] Synchronize tiling with accelerated compositing
https://bugs.webkit.org/show_bug.cgi?id=69151

Attachment 110383: Patch
https://bugs.webkit.org/attachment.cgi?id=110383&action=review

------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=110383&action=review


> Source/WebCore/ChangeLog:9
> +	   Enable "externally managed" tiles in TextureMapperNodes.
> +	   Currently, TextureMapperNodes manage tiles themselves, the tiles
being there
> +	   only to overcome the 2k texture size limitation. For WebKit2, we
want those tiles to be managed
> +	   externally, namely through the web process via the remote tile
backend for TiledBackingStore.

Why is the line length so different? This is a changelog.. and I would wrap
around 80-100 chars and try to keep all lines at the about same length

> Source/WebCore/ChangeLog:18
> +	   Reviewed by NOBODY (OOPS!).

Move this up

> Source/WebCore/ChangeLog:21
> +	   No new tests, code is still disabled. Eventually existing
compositing tests running on WK2 would
> +	   test this.

Better write:

Code is disabled for now, but covered by existing compositing tests.

> Source/WebCore/platform/graphics/texmap/TextureMapperNode.cpp:391
> +
> +

double newline

> Source/WebCore/platform/graphics/texmap/TextureMapperNode.cpp:406
> +	       // This code creates a transitional effect, showing tiles
rendered in the "old" contents scale behind tiles rendered in the current
contents scale.
> +	       // This effect looks good only when there's no transparency, so
we only use it when the opacity for the layer is above a certain threshold.

I would keep the comment to around 100 chars

> Source/WebCore/platform/graphics/texmap/TextureMapperNode.h:255
> +    struct ExternallyManagedTile {
> +	   bool isBackBufferUpdated;
> +	   bool isDirectlyCompositedImage;
> +	   ExternallyManagedTileBuffer frontBuffer;
> +	   ExternallyManagedTileBuffer backBuffer;
> +	   ExternallyManagedTile(float scale = 1.0)
> +	       : isBackBufferUpdated(false)
> +	       , isDirectlyCompositedImage(false)
> +	       , scale(scale)
> +	   {
> +	   }
> +	   float scale;
> +    };

your ordering here is a bit weird


More information about the webkit-reviews mailing list