[webkit-reviews] review denied: [Bug 101918] [Qt] REGRESSION(134142): overscaled tiles in pixel test results and MiniBrowser : [Attachment 173795] Patch

[bugzilla-daemon at webkit.org]

Noam Rosenthal <noam.rosenthal at nokia.com> has denied Huang Dongsung
<luxtella at company100.net>'s request for review:
Bug 101918: [Qt] REGRESSION(134142): overscaled tiles in pixel test results and
MiniBrowser
https://bugs.webkit.org/show_bug.cgi?id=101918

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

------- Additional Comments from Noam Rosenthal <noam.rosenthal at nokia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=173795&action=review


Good catch!
Some suggestions for improvement inline.

> Source/WebKit2/ChangeLog:16
> +	   This patch updates the size of CoordinatedBackingStore when
receiving an
> +	   UpdateTile or RemoveTile message also, because changing the size of
> +	   GrahpicsLayer causes an UpdateTile or RemoveTile message too.

Some wording improvement:
This patch makes sure that we reset the backing store's size to the layer size
when UpdateTile or RemoveTile are called.

> Source/WebKit2/UIProcess/CoordinatedGraphics/LayerTreeRenderer.cpp:428
> +void LayerTreeRenderer::setBackingStoreSize(CoordinatedBackingStore*
backingStore, WebLayerID layerID)

This seems somewhat inefficient and verbose, since you call layerByID twice
(getBackingStore calls layerByID).
How about if we changed the function signature to
getBackingStore(GraphicsLayer*) , and then you can have a function
resetBackingStoreSizeToLayerSize(GraphicsLayer*)