Open Bug 1815397 Opened 2 years ago Updated 2 years ago

Some blob layers are way too large

Categories

(Core :: Graphics: WebRender, defect, P3)

defect

Tracking

()

People

(Reporter: nical, Unassigned, NeedInfo)

References

(Blocks 2 open bugs)

Details

Attachments

(3 files)

The rasterized area of a blob image should be clipped to the displayport, however in some situations it looks like we fail to maintain a tight clip. In these cases, combined with a large scaling transform we end up rasterizing enormous amounts of pixels, causing performance issues and OOMs.

It isn't something that I've seen reported a lot on real websites but fuzzers are very good at finding these cases, to the point that it gets in the way of finding other bugs.

The important parameter here is the blob image visible rect.
We need to better understand why we aren't maintaining a tight clip in all cases.

Attached file Test case from bug 1743190 (1) (deleted) —

In this test case we requests 5204364 tiles (most of whichare 256x256 pixels) typically causing the OOM when enumerating the rasterization requests before we even attempt to allocate storage for the tiles.

It looks like it is the combination of the scale and the rotation that for some reason causes us to fail to clip the bounds, and the enormous scale factors causes the huge visible rect.

This test case is similar: we attempt to rasterize 32768 tiles, the visible rect is 32768x32768.

This time looks like it is the cobination of a filter and a scale that prevents us to minimize the size of the layer.

This one looks similar to test case 1, with a rotation and a scale.

Test case (2) can be reduced down to:

<style>
mtext {
  -webkit-text-stroke: 1px green;
  filter: brightness(0.71);
  transform: scale(1048576);
}
</style>
<math mathsize="1px">
<munderover>
<mtext>.</mtext>
<mo>-</mo>

It appears that the issue comes from here: https://searchfox.org/mozilla-central/source/layout/generic/nsIFrame.cpp#3262

we untransform a rectangle and store the result in an nsRect. the problem is that the transform has an enormous scaling factor, resulting in a tiny rectangle that is then rounded out to 1x1 pixel (many orders of magnitude larger than the tiny rect) to store as an nsRect.

later this will turn into the nsDiplaItem's BuildingRect which will be scaled by that same transform into something gigantic which will become the blob's visible rect.

precision is lost at this stack trace:

#0  0x00007efe8133ce35 in mozilla::nsDisplayTransform::UntransformRect
    at layout/painting/nsDisplayList.cpp:7267
#1  0x00007efe80ff3160 in nsIFrame::BuildDisplayListForStackingContext(..)
    (this=0x7efe670b2ee8, aCreatedContainerItem=0x7fff37a362d7)
    at layout/generic/nsIFrame.cpp:3262
#2  0x00007efe80faebab in nsIFrame::BuildDisplayListForChild(..) (this=0x7efe670b2dd0, aChild=0x7efe670b2ee8, aFlags=...)
    at layout/generic/nsIFrame.cpp:4290
#3  0x00007efe80f23af0 in nsContainerFrame::BuildDisplayListForNonBlockChildren(..) (this=0x7efe670b2dd0, aFlags=...)
    at layout/generic/nsContainerFrame.cpp:385
#4  0x00007efe80f4f80d in nsContainerFrame::BuildDisplayListForInline(..)
    (this=0x7efe670b2dd0) at layout/generic/nsContainerFrame.h:554
#5  0x00007efe812aad5a in nsMathMLContainerFrame::BuildDisplayList(..)
    (this=0x7efe670b2dd0) at layout/mathml/nsMathMLContainerFrame.cpp:611
#6  0x00007efe80faef1c in nsIFrame::BuildDisplayListForChild(..) (this=0x7efe670b2cd0, aChild=0x7efe670b2dd0, aFlags=...)
    at layout/generic/nsIFrame.cpp:4340
#7  0x00007efe80f23af0 in nsContainerFrame::BuildDisplayListForNonBlockChildren(..) (this=0x7efe670b2cd0, aFlags=...)
    at layout/generic/nsContainerFrame.cpp:385
#8  0x00007efe80f4f80d in nsContainerFrame::BuildDisplayListForInline(..)
    (this=0x7efe670b2cd0) at layout/generic/nsContainerFrame.h:554
#9  0x00007efe81028709 in nsInlineFrame::BuildDisplayList(..)
    (this=0x7efe670b2cd0) at layout/generic/nsInlineFrame.cpp:213
#10 0x00007efe80faee2e in nsIFrame::BuildDisplayListForChild(..) (this=0x7efe670b2bb8, aChild=0x7efe670b2cd0, aFlags=...)
    at layout/generic/nsIFrame.cpp:4325
#11 0x00007efe80f16c3a in DisplayLine(mozilla::nsDisplayListBuilder*, nsLineList_iterator&, bool, mozilla::nsDisplayListSet const&, nsBlockFrame*, mozilla::css::TextOverflow*, unsigned int, int, int&)
    (aBuilder=0x7efe609fb000, aLine=..., aLineInLine=true, aFrame=0x7efe670b2bb8, aTextOverflow=0x0, aLineNumberForTextOverflow=0, aDepth=0, aDrawnLines=@0x7fff37a37a34: -1431655766) at layout/generic/nsBlockFrame.cpp:7059
#12 0x00007efe80f15ae3 in nsBlockFrame::BuildDisplayList(..)
    (this=0x7efe670b2bb8) at layout/generic/nsBlockFrame.cpp:7214
#13 0x00007efe80faee2e in nsIFrame::BuildDisplayListForChild(..) (this=0x7efe670b2af0, aChild=0x7efe670b2bb8, aFlags=...)
    at layout/generic/nsIFrame.cpp:4325
#14 0x00007efe80f16c3a in DisplayLine(mozilla::nsDisplayListBuilder*, nsLineList_iterator&, bool, mozilla::nsDisplayListSet const&, nsBlockFrame*, mozilla::css::TextOverflow*, unsigned int, int, int&)
    (aBuilder=0x7efe609fb000, aLine=..., aLineInLine=false, aFrame=0x7efe670b2af0, aTextOverflow=0x0, aLineNumberForTextOverflow=0, aDepth=0, aDrawnLines=@0x7fff37a38774: -1431655766) at layout/generic/nsBlockFrame.cpp:7059
#15 0x00007efe80f15ae3 in nsBlockFrame::BuildDisplayList(..)
    (this=0x7efe670b2af0) at layout/generic/nsBlockFrame.cpp:7214
#16 0x00007efe80faee2e in nsIFrame::BuildDisplayListForChild(..) (this=0x7efe670b20c8, aChild=0x7efe670b2af0, aFlags=...)
    at layout/generic/nsIFrame.cpp:4325
#17 0x00007efe80f1c729 in nsCanvasFrame::BuildDisplayList(..)
    (this=0x7efe670b20c8) at layout/generic/nsCanvasFrame.cpp:584
#18 0x00007efe80faee2e in nsIFrame::BuildDisplayListForChild(..) (this=0x7efe670b2198, aChild=0x7efe670b20c8, aFlags=...)
    at layout/generic/nsIFrame.cpp:4325
#19 0x00007efe80fb19f3 in mozilla::ScrollFrameHelper::BuildDisplayList(..)
    (this=0x7efe670b2250) at layout/generic/nsGfxScrollFrame.cpp:4332
#20 0x00007efe810551dc in nsHTMLScrollFrame::BuildDisplayList(..)
    (this=0x7efe670b2198) at layout/generic/nsGfxScrollFrame.h:937
#21 0x00007efe80faee2e in nsIFrame::BuildDisplayListForChild(..) (this=0x7efe670b2020, aChild=0x7efe670b2198, aFlags=...)
    at layout/generic/nsIFrame.cpp:4325
#22 0x00007efe80eead19 in mozilla::ViewportFrame::BuildDisplayList(..)
    (this=0x7efe670b2020) at layout/generic/ViewportFrame.cpp:66
#23 0x00007efe80ff3bfa in nsIFrame::BuildDisplayListForStackingContext(mozilla::nsDisplayListBuilder*, mozilla::nsDisplayList*, bool*)
    (this=0x7efe670b2020, aList=0x7efe609fcfd8, aCreatedContainerItem=0x0)
    at layout/generic/nsIFrame.cpp:3462
#24 0x00007efe80e5af59 in nsLayoutUtils::PaintFrame(gfxContext*, nsIFrame*, nsRegion const&, unsigned int, mozilla::nsDisplayListBuilderMode, nsLayoutUtils::PaintFrameFlags) (aRenderingContext=0x0, aFrame=0x7efe670b2020, aDirtyRegion=..., aBackstop=0, aBuilderMode=mozilla::nsDisplayListBuilderMode::Painting, aFlags=(nsLayoutUtils::PaintFrameFlags::WidgetLayers | nsLayoutUtils::PaintFrameFlags::ExistingTransaction | nsLayoutUtils::PaintFrameFlags::ForWebRender))
    at layout/base/nsLayoutUtils.cpp:3349

nsDisplayItem::mBuildingRect is set from nsDisplayListBuilder::mVisibleRect here:

#0  0x00007efe8115d67f in mozilla::nsDisplayItem::SetBuildingRect(nsRect const&) (this=0x7efe607246e0, aBuildingRect=...)
    at layout/painting/nsDisplayList.h:2665
#1  0x00007efe813251f9 in mozilla::nsDisplayItem::nsDisplayItem(mozilla::nsDisplayListBuilder*, nsIFrame*, mozilla::ActiveScrolledRoot const*)
    (this=0x7efe607246e0, aFrame=0x7efe670b2fe8, aActiveScrolledRoot=0x7efe609f26c0)
    at layout/painting/nsDisplayList.cpp:2620
#2  0x00007efe80dca0d1 in mozilla::nsPaintedDisplayItem::nsPaintedDisplayItem(mozilla::nsDisplayListBuilder*, nsIFrame*, mozilla::ActiveScrolledRoot const*)
    (this=0x7efe607246e0, aFrame=0x7efe670b2fe8, aActiveScrolledRoot=0x7efe609f26c0)
    at layout/painting/nsDisplayList.h:2950
#3  0x00007efe80dc95e9 in mozilla::nsPaintedDisplayItem::nsPaintedDisplayItem(mozilla::nsDisplayListBuilder*, nsIFrame*)
    (this=0x7efe607246e0, aFrame=0x7efe670b2fe8) at layout/painting/nsDisplayList.h:2945
#4  0x00007efe8133dd19 in mozilla::nsDisplayText::nsDisplayText(mozilla::nsDisplayListBuilder*, nsTextFrame*)
    (this=0x7efe607246e0, aFrame=0x7efe670b2fe8) at layout/painting/nsDisplayList.cpp:7457
#5  0x00007efe8111b820 in mozilla::MakeDisplayItemWithIndex<mozilla::nsDisplayText, nsTextFrame>(mozilla::nsDisplayListBuilder*, nsTextFrame*, unsigned short)
    (aBuilder=0x7efe609fb000, aFrame=0x7efe670b2fe8, aIndex=0) at layout/painting/nsDisplayList.h:1990
#6  0x00007efe8111b772 in mozilla::nsDisplayList::AppendNewToTopWithIndex<mozilla::nsDisplayText, nsTextFrame>(mozilla::nsDisplayListBuilder*, nsTextFrame*, unsigned short)
    (this=0x7fff37a34798, aFrame=0x7efe670b2fe8, aIndex=0) at layout/painting/nsDisplayList.h:3103
#7  0x00007efe8110a297 in mozilla::nsDisplayList::AppendNewToTop<mozilla::nsDisplayText, nsTextFrame>(mozilla::nsDisplayListBuilder*, nsTextFrame*)
    (this=0x7fff37a34798, aFrame=0x7efe670b2fe8) at layout/painting/nsDisplayList.h:3096
#8  0x00007efe810e862c in nsTextFrame::BuildDisplayList(..)
    (this=0x7efe670b2fe8) at layout/generic/nsTextFrame.cpp:5192
#9  0x00007efe80faee2e in nsIFrame::BuildDisplayListForChild(..) (this=0x7efe670b3088, aChild=0x7efe670b2fe8, aFlags=...)
    at layout/generic/nsIFrame.cpp:4325
#10 0x00007efe80f16c3a in DisplayLine(mozilla::nsDisplayListBuilder*, nsLineList_iterator&, bool, mozilla::nsDisplayListSet const&, nsBlockFrame*, mozilla::css::TextOverflow*, unsigned int, int, int&)
    (aBuilder=0x7efe609fb000, aLine=..., aLineInLine=true, aFrame=0x7efe670b3088, aTextOverflow=0x0, aLineNumberForTextOverflow=0, aDepth=0, aDrawnLines=@0x7fff37a34a64: -1431655766) at layout/generic/nsBlockFrame.cpp:7059
#11 0x00007efe80f15ae3 in nsBlockFrame::BuildDisplayList(..)
    (this=0x7efe670b3088) at layout/generic/nsBlockFrame.cpp:7214
#12 0x00007efe80faef1c in nsIFrame::BuildDisplayListForChild(..) (this=0x7efe670b2ee8, aChild=0x7efe670b3088, aFlags=...)
    at layout/generic/nsIFrame.cpp:4340
#13 0x00007efe80f23af0 in nsContainerFrame::BuildDisplayListForNonBlockChildren(.., mozilla::EnumSet<nsIFrame::DisplayChildFlag, unsigned int>) (this=0x7efe670b2ee8, aFlags=...)
    at layout/generic/nsContainerFrame.cpp:385
#14 0x00007efe80f4f80d in nsContainerFrame::BuildDisplayListForInline(..)
    (this=0x7efe670b2ee8) at layout/generic/nsContainerFrame.h:554
#15 0x00007efe812aad5a in nsMathMLContainerFrame::BuildDisplayList(..)
    (this=0x7efe670b2ee8) at layout/mathml/nsMathMLContainerFrame.cpp:611
#16 0x00007efe80ff3bfa in nsIFrame::BuildDisplayListForStackingContext(mozilla::nsDisplayListBuilder*, mozilla::nsDisplayList*, bool*)
    (this=0x7efe670b2ee8, aCreatedContainerItem=0x7fff37a362d7)
    at layout/generic/nsIFrame.cpp:3462
#17 0x00007efe80faebab in nsIFrame::BuildDisplayListForChild(..) (this=0x7efe670b2dd0, aChild=0x7efe670b2ee8, aFlags=...)
    at layout/generic/nsIFrame.cpp:4290
#18 0x00007efe80f23af0 in nsContainerFrame::BuildDisplayListForNonBlockChildren(.., mozilla::EnumSet<nsIFrame::DisplayChildFlag, unsigned int>) (this=0x7efe670b2dd0, aFlags=...)
    at layout/generic/nsContainerFrame.cpp:385
#19 0x00007efe80f4f80d in nsContainerFrame::BuildDisplayListForInline(..)
    (this=0x7efe670b2dd0) at layout/generic/nsContainerFrame.h:554
#20 0x00007efe812aad5a in nsMathMLContainerFrame::BuildDisplayList(..)
    (this=0x7efe670b2dd0) at layout/mathml/nsMathMLContainerFrame.cpp:611
#21 0x00007efe80faef1c in nsIFrame::BuildDisplayListForChild(..) (this=0x7efe670b2cd0, aChild=0x7efe670b2dd0, aFlags=...)
    at layout/generic/nsIFrame.cpp:4340

Jnicol noted on matrix that it looks similar to an issue he dealt with a while back in bug 1594446.

Had a discussion with Jnicol and jrmuizel:

The visible rect and building rects

  • Same representation and coordinate space as the display items:
    • they are in local layout space (relative to the closest transform),
    • stored as nsRects which implies very few bits for the fractional parts.

The building rect is not a property of the display item, it is rather "the state of the visible rect during the gecko displaylist building traversal traversal, when we saw that particular item, that we use in the WR displaylist building traversal (aka painting)" in other words it is conceptually more like cached information about a state at particular points of the first traversal of the display list we reuse for convenience in the next traversal, than a property of the display items themselves.

Ideally we'd prefer for the building rect to not exist. We don't actually use it for many things but it occupies 4 x 32bits per display items. Instead it would be better if we either:

  • build it during the WR DL building traversal, similar to how we build the visible rect during gecko DL building.
  • somehow lazily reconstruct at points of the traversal where we need it.

The root of the problem

The problem boils down to the fact that we compute the visible rect in local space AND store it in nscoord which cannot represent anything smaller than 1/60.

As a result is is very imprecise (conservatively bigger than it needs to be) when large scales area applied since we have to apply the inverse of the transform to put the parent visible rect in the local space of the transformed child items. This means that we have to either:

  • represent it with floating point numbers
  • not use it for things that can't afford to be orders of magnitude wrong (such as deciding allocation sizes).

later that same imprecise rect is scaled back up, which is where things blow up but really the information loss when scaling down is the source of trouble.

What can we do

In practice we can't represent the visible rect with floats, because it interacts with a lot of things that are in nscoords and for most of these things it's OK for the visible rect to be very conservatively big. More generally we don't want to change the visible rect because of how much code interacts with it.

We want to change either the building rect or the code that uses it in the webrender fallback path. Right now the building rect is born out of the Gecko DL building's visible rect which is too imprecise (and can't be easily fixed), so we want to generate this information differently: probably as state of the WR DL building descent using floating point numbers (and possibly more lazily if that's not too hard to do).

This is growing into a bigger task than I can tackle before I go on leave. Tim is that something you could look into?

Flags: needinfo?(tnikkel)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: