[webkit-reviews] review denied: [Bug 84410] Terrible performance on http://alliances.commandandconquer.com/ and http://www.lordofultima.com/ : [Attachment 143434] Part 1: use a geometry map to optimize absolute layer bounds

[bugzilla-daemon at webkit.org]

Dave Hyatt <hyatt at apple.com> has denied Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 84410: Terrible performance on http://alliances.commandandconquer.com/ and
http://www.lordofultima.com/
https://bugs.webkit.org/show_bug.cgi?id=84410

Attachment 143434: Part 1: use a geometry map to optimize absolute layer bounds
https://bugs.webkit.org/attachment.cgi?id=143434&action=review

------- Additional Comments from Dave Hyatt <hyatt at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=143434&action=review


Overall looks nice. I'd like a bit of an explanation regarding why flipped
blocks writing modes can't be dealt with. I'm a bit confused about that part. I
thought offsetFromContainer did the right thing for those.

> Source/WebCore/rendering/RenderGeometryMap.cpp:157
> +	       // The root get special treatment for fixed position

Should be "The root gets"

> Source/WebCore/rendering/RenderInline.cpp:1148
> +    if (container->isBox() &&
container->style()->isFlippedBlocksWritingMode())
> +	   offsetDependsOnPoint = true;

Confused regarding why you can't just flip (somewhere) to handle this.

> Source/WebCore/rendering/RenderLayerCompositor.cpp:725
> +//	     absBounds =
layer->renderer()->localToAbsoluteQuad(FloatRect(layer->localBoundingBox())).en
closingBoundingBox();

Probably shouldn't leave commented out code in.

> Source/WebCore/rendering/RenderLayerCompositor.cpp:734
> -    
> +

Whitespace.

> Source/WebCore/rendering/RenderObject.cpp:2060
> +    LayoutSize offset;
> +    if (container->hasOverflowClip())
> +	   offset = -toRenderBox(container)->scrolledContentOffset();

I'm a little confused about why this has to be special cased.

> Source/WebCore/rendering/RenderObject.cpp:2062
> +    bool isNonUniform = hasColumns() || (container->isBox() &&
container->style()->isFlippedBlocksWritingMode());

Is it really that hard to deal with flipped blocks? Don't you just have to
flipForWritingMode along the correct axis?

> Source/WebCore/rendering/RenderObject.h:885
> +    // Pushes state onto RenderGeometryMap about how to map coordinates from
this renderer to its condtainer, or ancestorToStopAt (whichever is encountered
first).

Typo. "condtainer" should be "container"