Summary: | Repaint all graphics layers when their renderer offset changes | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adrienne Walker <enne> | ||||||||
Component: | New Bugs | Assignee: | Adrienne Walker <enne> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | enne, jamesr, kbr, simon.fraser, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Adrienne Walker
2012-01-06 12:35:15 PST
Created attachment 121478 [details]
Patch
This bug is intended to fix http://crbug.com/107769. As the map scrolls left to right, different elements end up going into the compositing layer, changing its bounds and its offset from the renderer. It looks like the layer in question is the foreground layer, which doesn't have the same logic as the main graphics layer to invalidate itself when the offset changes. I'm working on reducing a test case for this. Comment on attachment 121478 [details]
Patch
Looks reasonable, but I think we'll need some tests
Comment on attachment 121478 [details]
Patch
I think I'd prefer that the responsibility for calling setNeedsDisplay remains with the GraphicsLayer client, and is not pushed down to GraphicsLayer. My justification is that GraphicsLayer doesn't actually use m_offsetFromRenderer in its painting code at all (but perhaps it should?)
(In reply to comment #4) > (From update of attachment 121478 [details]) > I think I'd prefer that the responsibility for calling setNeedsDisplay remains with the GraphicsLayer client, and is not pushed down to GraphicsLayer. My justification is that GraphicsLayer doesn't actually use m_offsetFromRenderer in its painting code at all (but perhaps it should?) I'm happy to change this. However, I'm curious; is there a use case where a GraphicsLayerClient would not want to call setNeedsDisplay when the offset from the renderer changed? (In reply to comment #5) > (In reply to comment #4) > > (From update of attachment 121478 [details] [details]) > > I think I'd prefer that the responsibility for calling setNeedsDisplay remains with the GraphicsLayer client, and is not pushed down to GraphicsLayer. My justification is that GraphicsLayer doesn't actually use m_offsetFromRenderer in its painting code at all (but perhaps it should?) > > I'm happy to change this. However, I'm curious; is there a use case where a GraphicsLayerClient would not want to call setNeedsDisplay when the offset from the renderer changed? No, I think that's a sensible thing to do (at least for layers where drawsContent() == true). The first thing that RenderLayerBacking::paintContents() does is LayoutSize offset = graphicsLayer->offsetFromRenderer(); context.translate(-offset); so that could move into the caller, in which case it makes sense for GraphicsLayer to invalidate when offsetFromRenderer changes. Created attachment 122143 [details]
Test case, move offset handling
Comment on attachment 122143 [details]
Test case, move offset handling
Nice!
Comment on attachment 122143 [details] Test case, move offset handling Attachment 122143 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/11118219 Created attachment 122150 [details]
Add include for EFL, no review needed
(In reply to comment #8) > (From update of attachment 122143 [details]) > Nice! Thanks for the super fast review. :) Comment on attachment 122150 [details] Add include for EFL, no review needed Clearing flags on attachment: 122150 Committed r104782: <http://trac.webkit.org/changeset/104782> All reviewed patches have been landed. Closing bug. |