[webkit-reviews] review requested: [Bug 94743] Automatically use composited scrolling : [Attachment 163913] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Sep 13 11:06:38 PDT 2012


vollick at chromium.org has asked	for review:
Bug 94743: Automatically use composited scrolling
https://bugs.webkit.org/show_bug.cgi?id=94743

Attachment 163913: Patch
https://bugs.webkit.org/attachment.cgi?id=163913&action=review

------- Additional Comments from vollick at chromium.org
> > Source/WebCore/rendering/RenderLayer.cpp:769
> > +	 double zMin = std::numeric_limits<double>::max();
> > +	 double zMax = -std::numeric_limits<double>::max();
> > +	 findMinAndMaxZIndices(this, zMin, zMax);
> > +
> > +	 if (zMin >= zMax)
> > +	     return true;
> > +
> > +	 return
!nonDescendantInThisStackingContextCouldFallBetweenDescendants(stackingContext(
), this, zMin, zMax);
>
> This code seems very hard to understand, and I can't easily tell if it's
correct.
>
> What you're really asking is whether, by making a layer a stacking context,
you affect the traversal order of the z-order tree. It seems like it would be
easier to determine that from the z-order tree, rather than crawling around
looking at zMin and zMax.

This is true. The current patch walks the z-order tree (and I agree that this
is much easier to understand), but there's a down side: the z-order lists are
usually dirty.
I was expecting this to happen occasionally, and that they'd sort themselves
out eventually, but they seem to stay dirty.
Ideally, we could cache the lists when we rebuild them and clear the dirty
bits, but ::usesCompositedScrolling
gets called in a deep stack of const functions, so the options I see are

  1) Recompute the lists each time they are needed.
  2) Remove a bunch of const's.
  3) Make the lists mutable.

None of these options seem very nice to me. Do you have a suggestion for how to
proceed?


More information about the webkit-reviews mailing list