RESOLVED FIXED 112606
Refactor conditions for setCompositingLayersNeedRebuild in RenderLayer::styleChanged 
https://bugs.webkit.org/show_bug.cgi?id=112606
Summary Refactor conditions for setCompositingLayersNeedRebuild in RenderLayer::style...
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.