A z-index change on a layer backed node shouldn't really need redisplay - the backend can determine the drawing order and if cached just use the old contents.
Created attachment 60326 [details] Patch
Attachment 60326 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 WebCore/rendering/style/RenderStyle.cpp:490: An else should appear on the same line as the preceding } [whitespace/newline] [4] Total errors found: 1 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 60342 [details] Patch
Comment on attachment 60342 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=60342&action=review > WebCore/ChangeLog:10 > + title of the bug should go first, then the bug URL. then after a new line, the comments about the bug. > WebCore/rendering/RenderObject.cpp:1499 > + // If zindex changed, and we are not composited, need to repaint please make this comment into a full sentence, with period
The comment that is there is (more or less) a duplicate of the comment in the conditions around it - are you asking me to fix those as well? Is the patch otherwise acceptable?
Created attachment 70169 [details] Patch
Comment on attachment 70169 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=70169&action=review > WebCore/rendering/style/RenderStyle.cpp:510 > + } else if (m_box->zIndex() != other->m_box->zIndex() || m_box->hasAutoZIndex() != other->m_box->hasAutoZIndex() > + || visual->clip != other->visual->clip || visual->hasClip != other->visual->hasClip) > +#if USE(ACCELERATED_COMPOSITING) > + changedContextSensitiveProperties |= ContextSensitivePropertyZIndex; You're setting ContextSensitivePropertyZIndex when the clip changes, which is wrong.
Created attachment 71503 [details] Patch
Comment on attachment 71503 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=71503&action=review > WebCore/rendering/style/RenderStyle.cpp:515 > + } else if (visual->clip != other->visual->clip || visual->hasClip != other->visual->hasClip) { > + return StyleDifferenceRepaintLayer; > + } Style.
(In reply to comment #9) > (From update of attachment 71503 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=71503&action=review > > > WebCore/rendering/style/RenderStyle.cpp:515 > > + } else if (visual->clip != other->visual->clip || visual->hasClip != other->visual->hasClip) { > > + return StyleDifferenceRepaintLayer; > > + } > > Style. I'm not clear what is wrong here - what is it that is undesirable?
Single line clauses don't need braces.
Created attachment 75702 [details] Patch
Comment on attachment 75702 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=75702&action=review cq- for the bad ChagneLgo entry. > WebCore/ChangeLog:27382 > +2010-07-01 Sam Magnuson <smagnuson@netflix.com> Your ChangeLog entry shoudl be at the top of the file.
Created attachment 78617 [details] Patch
Comment on attachment 78617 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=78617&action=review > Source/WebCore/ChangeLog:5 > + GraphicsLayer: z-index causes a repaint of a layer Should be "z-index change causes..." > Source/WebCore/ChangeLog:8 This needs to be re-worded; it's not very clear. > Source/WebCore/ChangeLog:9 > + https://bugs.webkit.org/show_bug.cgi?id=41497 Bug URL should come after the bug title. webkit-patch will format this for you. > Source/WebCore/ChangeLog:11 > + No new tests; this is an optimization. But I don't think that are there tests that exercise this code path, so new tests should be added (in LayoutTests/compositing). > Source/WebCore/rendering/style/RenderStyle.cpp:530 > - } else if (m_box->zIndex() != other->m_box->zIndex() || m_box->hasAutoZIndex() != other->m_box->hasAutoZIndex() || > - visual->clip != other->visual->clip || visual->hasClip != other->visual->hasClip) > + } else if (m_box->zIndex() != other->m_box->zIndex() || m_box->hasAutoZIndex() != other->m_box->hasAutoZIndex() > + || visual->clip != other->visual->clip || visual->hasClip != other->visual->hasClip) > +#if USE(ACCELERATED_COMPOSITING) This is wrong. The previous version of the patch was more correctly. I'm a bit worried about going from auto z-index to explicit z-index, because this also affects whether the element creates a stacking context. I think you should only do this optimization if old and new style both have explicit z-index.