Bug 31278 – ASSERTION FAILURE: mode & UseTransforms in Assertion Failure in RenderSVGRoot::mapLocalToContainer

• Component: SVG
• Status: NEW

[Jing]

Loading the URL gives the following assertion:
ASSERTION FAILED: !fixed
(WebCore/rendering/RenderSVGRoot.cpp:269 virtual void WebCore::RenderSVGRoot::mapLocalToContainer(WebCore::RenderBoxModelObject*, bool, bool, WebCore::TransformState&) const)

[Dean Jackson]

Assertion comes because the page has position:fixed on the svg diagram used in the page footer. The page works fine in release builds, and the fixed position element is rendered correctly, so I'm not sure why the ASSERT is there.

[Dean Jackson]

Created attachment 42819 [details]
simple testcase

Notice that the test has both position:fixed and a CSS transform on the svg element.

Removing the ASSERT(!fixed) will cause the 2nd ASSERT to fire. Removing that will have the correct behaviour, but only when the CSS transform is not present.

[Simon Fraser (smfr)]

Created attachment 42843 [details]
Testcase that shows that position:fixed on svg is busted

[Simon Fraser (smfr)]

Comment on attachment 42843 [details]
Testcase that shows that position:fixed on svg is busted

Never mind; that's currently expected with -webkit-transform and position:fixed (bug 31283).

[Simon Fraser (smfr)]

I think there are two issues here:
1. We haven't cleaned up the "fixed inside of transformed" codepath. I don' think the code in convertToLayerCoords() is doing the right thing there now by just calling localToAbsolute() ignoring transforms.

2. There needs to be some explicit hand-off code in the interface between SVG and HTML (in both directions), that maps the expectations of one into those of the other.

[Michael Gratton]

Something similar is happening on YouTube HTML5 video embeds.

For an example: disable Flash, visit http://www.trackosaurusrex.com/pblog/comments.php?y=11&m=03&entry=entry110329-122230 and click the play button in comment #3.

Segfault in trunk rev 82358, webkitgtk/gtk3, caused by an assertion:

ASSERTION FAILED: useTransforms
Source/WebCore/rendering/svg/RenderSVGRoot.cpp(300) : virtual void WebCore::RenderSVGRoot::mapLocalToContainer(WebCore::RenderBoxModelObject*, bool, bool, WebCore::TransformState&) const

[Dirk Schulze]

*** Bug 52961 has been marked as a duplicate of this bug. ***

[Sylvain Galineau]

Created attachment 248491 [details]
Patch update

The ASSERT seems obsolete since RenderReplaced::mapLocalToContainer() does apply transforms.

[Said Abou-Hallawa]

Comment on attachment 248491 [details]
Patch update

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

> Source/WebCore/rendering/svg/RenderSVGRoot.cpp:379
>      ASSERT(mode & ~IsFixed); // We should have no fixed content in the SVG rendering tree.

I think this comment is wrong. I think the assertion means that, the mode should have UseTransforms and/or ApplyContainerFlip. And if it does, it does not matter whether it has IsFixed or not. So it can be deleted if we decide to keep ASSERT(mode & UseTransforms) as it is right now.

> Source/WebCore/rendering/svg/RenderSVGRoot.cpp:-380
> -    ASSERT(mode & UseTransforms); // mapping a point through SVG w/o respecting trasnforms is useless.

I do not think this is correct. The assertion is there to confirm that all the callers are passing the UseTransform flag always. If the caller does not pass this flag, the mapping from local to container will not include the css transformation. You can see the problem if apply your patch and open the attached test case and set a breakpoint in RenderSVGRoot::mapLocalToContainer() and wait for the following call stack.

#0	0x0000000106ac3831 in WebCore::RenderSVGRoot::mapLocalToContainer(WebCore::RenderLayerModelObject const*, WebCore::TransformState&, unsigned int, bool*) const at /Volumes/Data/WebKit/OpenSource/Source/WebCore/rendering/svg/RenderSVGRoot.cpp:382
#1	0x0000000106a69f4a in WebCore::RenderObject::localToAbsolute(WebCore::FloatPoint const&, unsigned int) const at /Volumes/Data/WebKit/OpenSource/Source/WebCore/rendering/RenderObject.cpp:1585
#2	0x00000001069a91fe in WebCore::accumulateOffsetTowardsAncestor(WebCore::RenderLayer const*, WebCore::RenderLayer const*, WebCore::LayoutPoint&, WebCore::RenderLayer::ColumnOffsetAdjustment) at /Volumes/Data/WebKit/OpenSource/Source/WebCore/rendering/RenderLayer.cpp:2009
#3	0x00000001069a9061 in WebCore::RenderLayer::convertToLayerCoords(WebCore::RenderLayer const*, WebCore::LayoutPoint const&, WebCore::RenderLayer::ColumnOffsetAdjustment) const at /Volumes/Data/WebKit/OpenSource/Source/WebCore/rendering/RenderLayer.cpp:2125
#4	0x00000001069a2062 in WebCore::RenderLayer::offsetFromAncestor(WebCore::RenderLayer const*) const at /Volumes/Data/WebKit/OpenSource/Source/WebCore/rendering/RenderLayer.cpp:2131
#5	0x00000001069b6ca9 in WebCore::performOverlapTests(WTF::HashMap<WebCore::OverlapTestRequestClient*, WebCore::IntRect, WTF::PtrHash<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::OverlapTestRequestClient*>, WTF::HashTraits<WebCore::IntRect> >&, WebCore::RenderLayer const*, WebCore::RenderLayer const*) at /Volumes/Data/WebKit/OpenSource/Source/WebCore/rendering/RenderLayer.cpp:3818

Because mode is equal to 5, the mapLocalToContainer() does not apply the transformation. In WebCore::performOverlapTests(), if you look at the final value of boundingBox.location(), you will see that it is set to (8,42) which is the fixed value of the svg as if there were no css transform applied to it.  The actual value should be (108,142) which is fixed position translated by the css transform.

[Sylvain Galineau]

I clearly misunderstood what the flag meant. Thanks for the clarification.

smfr points out the issue seems to be that WebRenderObject::WebRenderObject() does not set UseTransforms thus triggering the ASSERT.

I am unsure why the former doesn't though.

[Simon Fraser (smfr)]

WebRenderObject::WebRenderObject() is just stupid Safari-only debug code. We should make it a bit less stupid.

[Said Abou-Hallawa]

*** Bug 120903 has been marked as a duplicate of this bug. ***

[Said Abou-Hallawa]

*** Bug 104636 has been marked as a duplicate of this bug. ***

[Said Abou-Hallawa]

<rdar://problem/14703176>

[Said Abou-Hallawa]

<rdar://problem/45542171>

[Daniel Bates]

Seen with debug WebKit in iPad simulator with WebKit r244742

Just hit ASSERT(mode & UseTransforms) today (05/02) when doing the following:

1. Visit <http://wordpress.com>.
2. Click login.

[Daniel Bates]

(In reply to Daniel Bates from comment #16)
> Seen with debug WebKit in iPad simulator with WebKit r244742
> 
> Just hit ASSERT(mode & UseTransforms) today (05/02) when doing the following:
> 
> 1. Visit <http://wordpress.com>.
> 2. Click login.

Some debug notes when I hit this bug:

(lldb) p mode
(WebCore::MapCoordinatesFlags) $2 = 5 { size = 2 } {
  IsFixed = 1
  ApplyContainerFlip = 4
}
(lldb) p repaintContainer
(const WebCore::RenderLayerModelObject *) $5 = 0x0000000000000000
(lldb) p transformState
(WebCore::TransformState) $6 = {
  m_lastPlanarPoint = { x = 0.0, y = 0.0 }
  m_lastPlanarQuad = (m_p1 = { x = 0.0, y = 0.0 }, m_p2 = { x = 0.0, y = 0.0 }, m_p3 = { x = 0.0, y = 0.0 }, m_p4 = { x = 0.0, y = 0.0 })
  m_lastPlanarSecondaryQuad = {
    __ptr_ = {
      std::__1::__compressed_pair_elem<WebCore::FloatQuad *, 0, false> = {
        __value_ = 0x0000000000000000
      }
    }
  }
  m_accumulatedTransform = {
    __ptr_ = {
      std::__1::__compressed_pair_elem<WebCore::TransformationMatrix *, 0, false> = {
        __value_ = 0x0000000000000000
      }
    }
  }
  m_accumulatedOffset = { width = 0px (0), height = 0px (0) }
  m_accumulatingTransform = false
  m_mapPoint = true
  m_mapQuad = false
  m_direction = ApplyTransformDirection
}
(lldb) p wasFixed
(bool *) $7 = 0x0000000000000000
(lldb) 

In calling frame, RenderObject::localToAbsolute():

(lldb) p localPoint
(const WebCore::FloatPoint) $8 = { x = 0.0, y = 0.0 }
(lldb) p mode
(WebCore::MapCoordinatesFlags) $9 = 1 { size = 1 } {
  IsFixed = 1
}

[Daniel Bates]

(In reply to Daniel Bates from comment #17)
> (In reply to Daniel Bates from comment #16)
> > Seen with debug WebKit in iPad simulator with WebKit r244742
> > 
> > Just hit ASSERT(mode & UseTransforms) today (05/02) when doing the following:
> > 
> > 1. Visit <http://wordpress.com>.
> > 2. Click login.
> 
> Some debug notes when I hit this bug:
> 
> (lldb) p mode
> (WebCore::MapCoordinatesFlags) $2 = 5 { size = 2 } {
>   IsFixed = 1
>   ApplyContainerFlip = 4
> }
> (lldb) p repaintContainer
> (const WebCore::RenderLayerModelObject *) $5 = 0x0000000000000000
> (lldb) p transformState
> (WebCore::TransformState) $6 = {
>   m_lastPlanarPoint = { x = 0.0, y = 0.0 }
>   m_lastPlanarQuad = (m_p1 = { x = 0.0, y = 0.0 }, m_p2 = { x = 0.0, y = 0.0
> }, m_p3 = { x = 0.0, y = 0.0 }, m_p4 = { x = 0.0, y = 0.0 })
>   m_lastPlanarSecondaryQuad = {
>     __ptr_ = {
>       std::__1::__compressed_pair_elem<WebCore::FloatQuad *, 0, false> = {
>         __value_ = 0x0000000000000000
>       }
>     }
>   }
>   m_accumulatedTransform = {
>     __ptr_ = {
>       std::__1::__compressed_pair_elem<WebCore::TransformationMatrix *, 0,
> false> = {
>         __value_ = 0x0000000000000000
>       }
>     }
>   }
>   m_accumulatedOffset = { width = 0px (0), height = 0px (0) }
>   m_accumulatingTransform = false
>   m_mapPoint = true
>   m_mapQuad = false
>   m_direction = ApplyTransformDirection
> }
> (lldb) p wasFixed
> (bool *) $7 = 0x0000000000000000
> (lldb) 
> 
> In calling frame, RenderObject::localToAbsolute():
> 
> (lldb) p localPoint
> (const WebCore::FloatPoint) $8 = { x = 0.0, y = 0.0 }
> (lldb) p mode
> (WebCore::MapCoordinatesFlags) $9 = 1 { size = 1 } {
>   IsFixed = 1
> }

And the calling calling frame is WebCore::accumulateOffsetTowardsAncestor(WebCore::RenderLayer const*, WebCore::RenderLayer const*, WebCore::LayoutPoint&, WebCore::RenderLayer::ColumnOffsetAdjustment). And look what we have here a FIXME comment AND it only passes isFixed (see (*) line):

[[
    // FIXME: Positioning of out-of-flow(fixed, absolute) elements collected in a RenderFragmentedFlow
    // may need to be revisited in a future patch.
    // If the fixed renderer is inside a RenderFragmentedFlow, we should not compute location using localToAbsolute,
    // since localToAbsolute maps the coordinates from named flow to regions coordinates and regions can be
    // positioned in a completely different place in the viewport (RenderView).
    if (position == PositionType::Fixed && !fixedFragmentedFlowContainer && (!ancestorLayer || ancestorLayer == renderer.view().layer())) {
        // If the fixed layer's container is the root, just add in the offset of the view. We can obtain this by calling
        // localToAbsolute() on the RenderView.
(*)     FloatPoint absPos = renderer.localToAbsolute(FloatPoint(), IsFixed);
        location += LayoutSize(absPos.x(), absPos.y());
        return ancestorLayer;
    }
]]
<https://trac.webkit.org/browser/trunk/Source/WebCore/rendering/RenderLayer.cpp?rev=244742#L2070>

[Daniel Bates]

RenderSVGRoot::mapLocalToContainer() is the only implementation of mapLocalToContainer that asserts that UseTransforms is in mode. RenderSVGRoot::mapLocalToContainer() is too strict OR calling code is wrong. Calling code has FIXME that seems to acknowledge that it is wrong. So, it's probably wrong.

[Simon Fraser (smfr)]

*** Bug 202803 has been marked as a duplicate of this bug. ***

[Sergio Villar Senin]

Created attachment 430560 [details]
Minimal test

Trying to revive a bit an old bug. I'm easily hitting this with automatically generated test cases. See for example the one I'm attaching. We should figure out what to do.

I am no longer sure the ASSERT(mode & ^isFixed) is correct. Said mentioned that the comment is wrong but I think the ASSERT is what it's wrong here. I think it really wants to check !(mode & isFixed) because if I am not wrong mode was just a boolean and then it became an enum, so this is probably a leftover from that transition.

WRT to the 2nd condition, I am not sure. It looks like Chromium deleted it when they enabled layer flattering. Not sure if that makes any sense in the WebKit context.

[Ahmad Saleem]

Blink fixed similar assertion here - https://chromium.googlesource.com/chromium/src.git/+/1761ef2855918b3e54d974a7fa1a860d604d660f