Bug 101874 – Eliminate ancestor tree walk computing outlineBoundsForRepaint() when updating layer positions

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Simon Fraser (smfr)]

Eliminate ancestor tree walk computing outlineBoundsForRepaint() when updating layer positions

[Simon Fraser (smfr)]

Created attachment 173532 [details]
Patch

Comment on attachment 173532 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=173532&action=review

> Source/WebCore/ChangeLog:34
> +        parameter with an optional RenderGeometryMap, and it use to map the compute rect to

Typo: “it use to map the compute”

> Source/WebCore/page/FrameView.cpp:1213
> +        RenderGeometryMap geometryMap(UseTransforms);

Why does this need to be scoped?

> Source/WebCore/rendering/RenderGeometryMap.cpp:58
>  FloatPoint RenderGeometryMap::absolutePoint(const FloatPoint& p) const
>  {
> -    FloatPoint result;
> -    
> -    if (!hasFixedPositionStep() && !hasTransformStep() && !hasNonUniformStep())
> -        result = p + m_accumulatedOffset;
> -    else {
> -        TransformState transformState(TransformState::ApplyTransformDirection, p);
> -        mapToAbsolute(transformState);
> -        result = transformState.lastPlanarPoint();
> -    }
> -
> -#if !ASSERT_DISABLED
> -    FloatPoint rendererMappedResult = m_mapping.last().m_renderer->localToAbsolute(p, UseTransforms | SnapOffsetForTransforms);
> -    ASSERT(rendererMappedResult == result);
> -#endif
> -
> -    return result;
> +    return mapToContainer(p, 0);
>  }
>  
>  FloatRect RenderGeometryMap::absoluteRect(const FloatRect& rect) const
>  {
> -    FloatRect result;
> -    
> -    if (!hasFixedPositionStep() && !hasTransformStep() && !hasNonUniformStep()) {
> -        result = rect;
> -        result.move(m_accumulatedOffset);
> -    } else {
> -        TransformState transformState(TransformState::ApplyTransformDirection, rect.center(), rect);
> -        mapToAbsolute(transformState);
> -        result = transformState.lastPlanarQuad().boundingBox();
> -    }
> -
> -#if !ASSERT_DISABLED
> -    FloatRect rendererMappedResult = m_mapping.last().m_renderer->localToAbsoluteQuad(rect).boundingBox();
> -    // Inspector creates renderers with negative width <https://bugs.webkit.org/show_bug.cgi?id=87194>.
> -    // Taking FloatQuad bounds avoids spurious assertions because of that.
> -    ASSERT(enclosingIntRect(rendererMappedResult) == enclosingIntRect(FloatQuad(result).boundingBox()));
> -#endif
> -
> -    return result;
> +    return mapToContainer(rect, 0).boundingBox();
>  }
>  

Should these functions be moved into the header now so that they can be inlined?

[Simon Fraser (smfr)]

(In reply to comment #2)
> (From update of attachment 173532 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=173532&action=review
> 
> > Source/WebCore/ChangeLog:34
> > +        parameter with an optional RenderGeometryMap, and it use to map the compute rect to
> 
> Typo: “it use to map the compute”

Thanks

> 
> > Source/WebCore/page/FrameView.cpp:1213
> > +        RenderGeometryMap geometryMap(UseTransforms);
> 
> Why does this need to be scoped?

It doesn't. It just seemed neater, since it's only used for this one call. Another way to do this would be to make a wrapper in RenderLayer for those few lines.

> > Source/WebCore/rendering/RenderGeometryMap.cpp:58
> >  FloatPoint RenderGeometryMap::absolutePoint(const FloatPoint& p) const
...
> >  FloatRect RenderGeometryMap::absoluteRect(const FloatRect& rect) const
...
> Should these functions be moved into the header now so that they can be inlined?

Sure!

[Build Bot]

Comment on attachment 173532 [details]
Patch

Attachment 173532 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14794781

New failing tests:
fast/spatial-navigation/snav-imagemap-overlapped-areas.html

[Peter Beverloo (cr-android ews)]

Comment on attachment 173532 [details]
Patch

Attachment 173532 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/14785966

[EFL EWS Bot]

Comment on attachment 173532 [details]
Patch

Attachment 173532 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/14803513

[Simon Fraser (smfr)]

Created attachment 173549 [details]
Patch

[Simon Fraser (smfr)]

Created attachment 173726 [details]
Patch

[Build Bot]

Comment on attachment 173726 [details]
Patch

Attachment 173726 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/14818408

New failing tests:
http/tests/inspector/stacktraces/csp-inline-warning-contains-stacktrace.html

[Dave Hyatt]

Comment on attachment 173726 [details]
Patch

r=me

[Simon Fraser (smfr)]

I can't land this because it's causing some assertions in RenderGeometryMap because of pixel snapping differences.

[Radar WebKit Bug Importer]

<rdar://problem/12705362>

[Simon Fraser (smfr)]

http://trac.webkit.org/changeset/135025