[webkit-reviews] review requested: [Bug 74203] [chromium] Create iterators for the RenderSurface-Layer tree : [Attachment 120478] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Dec 23 13:56:44 PST 2011


Dana Jansens <danakj at chromium.org> has asked  for review:
Bug 74203: [chromium] Create iterators for the RenderSurface-Layer tree
https://bugs.webkit.org/show_bug.cgi?id=74203

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

------- Additional Comments from Dana Jansens <danakj at chromium.org>
(In reply to comment #18)
> (In reply to comment #16)
> > on top of that, can we just have one bidirectional iterator type, or
perhaps an iterator where you specify the direction at construction time or
something to avoid having the loop logic in there twice?

A bidirectional implementation would need to be stateless other than what is
stored in CCLayerIteratorPosition. For times when history is used currently,
this means walking from the start of the tree to your current position to build
up that history (and then toss it away afterward). I think that this walk would
look exactly like the code we have now (one forward one backward) with a
while(){ } around it. Personally I think what we have now is nicer than that..
unless someone has another idea how to do it.

While we were on the topic of complexity. I should point out that FrontToBack
does have an extra cost (vs BackToFront or DepOrdered), with a for-loop to find
the next target RenderSurface's owning layer. BackToFront was doing this also,
but I replaced the for-loop with an extra int state variable in the iterator,
m_highestTargetRenderSurfaceLayer.

I've pulled the CCLayerIteratorActions over to a .cpp file, and gotten rid of
the Vectors. It was actually very easy and I like the result a lot!


More information about the webkit-reviews mailing list