Bug 112606

Summary: Refactor conditions for setCompositingLayersNeedRebuild in RenderLayer::styleChanged
Product: WebKit Reporter: Max Vujovic <mvujovic>
Component: Layout and RenderingAssignee: Max Vujovic <mvujovic>
Status: RESOLVED FIXED    
Severity: Normal CC: achicu, eric, esprehn+autocc, ojan.autocc, simon.fraser, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 109098    
Attachments:
Description Flags
Patch for Review
mvujovic: commit-queue-
Patch for Review none

Max Vujovic
Reported 2013-03-18 11:53:18 PDT
To fix bug 109098, we will need to add more conditions for calling RenderLayerCompositor::setCompositingLayersNeedRebuild in RenderLayer::styleChanged. Adding more non-trivial conditions will make this block of code even harder to follow. I suggest we refactor the conditions into functions like: - needsCompositingLayersRebuiltForClip - needsCompositingLayersRebuiltForOverflow - needsCompositingLayersRebuiltForFilters (This function would be added in bug 109098.) Currently, the block of code in RenderLayer::styleChanged looks like this: #if USE(ACCELERATED_COMPOSITING) updateNeedsCompositedScrolling(); if (compositor()->updateLayerCompositingState(this)) compositor()->setCompositingLayersNeedRebuild(); else if (oldStyle && (oldStyle->clip() != renderer()->style()->clip() || oldStyle->hasClip() != renderer()->style()->hasClip())) compositor()->setCompositingLayersNeedRebuild(); else if (m_backing) m_backing->updateGraphicsLayerGeometry(); else if (oldStyle && oldStyle->overflowX() != renderer()->style()->overflowX()) { if (stackingContainer()->hasCompositingDescendant()) compositor()->setCompositingLayersNeedRebuild(); } #endif
Attachments
Patch for Review (4.35 KB, patch)
2013-03-18 14:02 PDT, Max Vujovic 		mvujovic: commit-queue-
DetailsFormatted DiffDiff
Patch for Review (4.61 KB, patch)
2013-03-18 14:29 PDT, Max Vujovic 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Max Vujovic
Comment 1 2013-03-18 14:02:13 PDT
Created attachment 193644 [details] Patch for Review Running this patch through the bots to make sure the logic hasn't changed.
Alexandru Chiculita
Comment 2 2013-03-18 14:09:25 PDT
Comment on attachment 193644 [details] Patch for Review View in context: https://bugs.webkit.org/attachment.cgi?id=193644&action=review Looks good, thanks! > Source/WebCore/rendering/RenderLayer.cpp:5989 > +static inline bool needsCompositingLayersRebuiltForClip(const RenderStyle* oldStyle, const RenderStyle* newStyle) I would keep both function have similar signature, so in this case, this function could also be part of the RenderLayer. > Source/WebCore/rendering/RenderLayer.cpp:5997 > + return !isComposited() && oldStyle && (oldStyle->overflowX() != renderer()->style()->overflowX()) && stackingContainer()->hasCompositingDescendant(); Why not pass the new style here instead of using renderer()->style()?
Max Vujovic
Comment 3 2013-03-18 14:29:24 PDT
Created attachment 193656 [details] Patch for Review
Max Vujovic
Comment 4 2013-03-18 14:31:13 PDT
Comment on attachment 193644 [details] Patch for Review Thanks for looking, Alex. View in context: https://bugs.webkit.org/attachment.cgi?id=193644&action=review >> Source/WebCore/rendering/RenderLayer.cpp:5989 >> +static inline bool needsCompositingLayersRebuiltForClip(const RenderStyle* oldStyle, const RenderStyle* newStyle) > > I would keep both function have similar signature, so in this case, this function could also be part of the RenderLayer. Done. That looks better. >> Source/WebCore/rendering/RenderLayer.cpp:5997 >> + return !isComposited() && oldStyle && (oldStyle->overflowX() != renderer()->style()->overflowX()) && stackingContainer()->hasCompositingDescendant(); > > Why not pass the new style here instead of using renderer()->style()? Done. Now the function signatures match.
Max Vujovic
Comment 5 2013-03-19 09:27:49 PDT
Comment on attachment 193656 [details] Patch for Review Bots look good. Setting cq+.
WebKit Review Bot
Comment 6 2013-03-19 09:51:24 PDT
Comment on attachment 193656 [details] Patch for Review Clearing flags on attachment: 193656 Committed r146213: <http://trac.webkit.org/changeset/146213>
WebKit Review Bot
Comment 7 2013-03-19 09:51:28 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.