Bug 179476 - [GTK][WPE] CoordinatedGraphicsLayer::setNeedsDisplayInRect() converts FloatRect to IntRect erroneously
Summary: [GTK][WPE] CoordinatedGraphicsLayer::setNeedsDisplayInRect() converts FloatRe...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Miguel Gomez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-11-09 05:37 PST by Miguel Gomez
Modified: 2017-11-10 00:51 PST (History)
8 users (show)

See Also:


Attachments
Patch (1.77 KB, patch)
2017-11-09 06:33 PST, Miguel Gomez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Miguel Gomez 2017-11-09 05:37:36 PST
I found this while digging into a rendering issue that is not reproducible upstream, but in a WPE stable branch downstream, and only in certain platforms (I was only able to reproduce it in MIPS, while the same code on ARM doesn't show the problem). Nevertheless I think this is a bug upstream as well and, despite I'm not able to reproduce with the test I'm using, I think it could be reproduced eventually.

I'll try to explain what happens:

- A page renders a div at position (100.5,100) with size 200x200 with a property that makes it have its own GraphicsLayer.
- The CoordinatedGraphicsLayer gets created. The size and position of that GraphicsLayer gets calculated in RenderLayerBacking::computePrimaryGraphicsLayerRect(), which properly turns the float values into integers, setting the position to (100,100) and the size to 201x200 (notice that the resulting rect encloses the original one).
- The CoordinatedGraphicsLayer will create a backingStore, then a Tile with the size set to the layer, and use an UpdateAtlas with size 201x200
- Then RenderBox will draw the div content using a rect with position (0.5,0) (the offset of the div inside the layer) and size 200x200, and everything works fine

- Then for some reason the div is modified and needs to be repainted.

- After the layout and rendering calculations, RenderLayerBacking::setContentsNeedDisplayInRect() calls CoordinatedGraphicsLayer::setNeedsDisplayInRect() passing a rect with position (0.5,0) and size 200x200, which is the rect really used by the div inside the CoordinatedGraphicsLayer that needs to be repainted.
- CoordinatedGraphicsLayer::setNeedsDisplayInRect() turns the FloatRect into an IntRect using IntRect(FloatRect), turning the position from (0.5,0) to (0,0) and keeping the size to 200x200. Then calls invalidate on the backingStore and then the Tile, which will store that rect as the dirty one.
- During the layer flush, the tile will update its contents when Tile::updateBackBuffer() gets called. For that it will allocate a new UpdateAtlas with the size of the dirty rect (200x200)
- When drawing the div, RenderBox tries to draw a rectangle with position (0.5,0) and size 200x200, but that doesn't fit into the UpdateAtlas we are using (we would need it to be 201x200), so we are not able to draw the last column of the div, which is the glitch I found.

As I said, I'm not able to reproduce the glitch upstream with the page I'm testing, because the RenderBox seems to get a size of 201x200 instead of 200x200 there, which causes the UpdateAtlas to have the appropriate size. But still, converting a FloatRect into an IntRect with IntRect(FloatRect) will generate a rect that, in almost all of the cases, will be smaller than the original one. I think the appropriate way to do it is by using enclosingIntRect(FloatRect).
Comment 1 Miguel Gomez 2017-11-09 06:33:32 PST
Created attachment 326445 [details]
Patch
Comment 2 Miguel Gomez 2017-11-09 06:48:08 PST
I've just remembered that upstream the UpdateAtlas we use are always 1024x1024. They are not created just with the size requested as we do downstream. But the glitch could happen anyway, as the rect we are painting to inside the UpdateAtlas is smaller than the div we want to paint.
Comment 3 WebKit Commit Bot 2017-11-10 00:51:06 PST
Comment on attachment 326445 [details]
Patch

Clearing flags on attachment: 326445

Committed r224671: <https://trac.webkit.org/changeset/224671>
Comment 4 WebKit Commit Bot 2017-11-10 00:51:08 PST
All reviewed patches have been landed.  Closing bug.