Closed Bug 1155137 Opened 5 years ago Closed 10 months ago

what is causing this warning: "Transparent content with displayports can be expensive"

Categories

(Core :: Graphics: Layers, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: paul, Assigned: botond)

References

Details

(Whiteboard: [gfx-noted])

Attachments

(1 file)

B2G on Desktop, the terminal is spammed with:
[Child 63376] WARNING: Transparent content with displayports can be expensive.: file /Users/paul/mozilla/gecko-projects/layout/base/nsDisplayList.cpp, line 1737

The setup:
- B2G desktop. Multi-process enabled. Debug build
- app in an iframe
- app itself includes several remote mozbrowser iframes (it's a browser)

In the top level document (shell.html), the app and the content of the iframe, there is no transparent element (afaik). iframes are opaque, and content of iframes are opaque too.

If the app is taking a slow path, I'd like to know what is the cause of it.

Is there any way to dump relevant information? I can attach a debugger and dump the stack if that helps in anyway.
This only happens if APZC is enabled.
Botond,

Can you help out here or ping relevant gfx person?
Flags: needinfo?(botond)
Whiteboard: [gfx-noted]
I was just talking to Timothy about this last week. In that case, we were looking at a simple a clearly non-transparent page, like http://people.mozilla.org/~bballo/grid.html, in the desktop browser with APZ enabled.

It looks like Layout is incorrectly failing to set the Layer::CONTENT_OPAQUE flag on some opaque content. We should definitely investigate this.

I'll park this bug with me, and hopefully look at it in the next week or so.
Assignee: nobody → botond
Flags: needinfo?(botond)
Anything I can do to help investigate what's going on?
Flags: needinfo?(botond)
http://mxr.mozilla.org/mozilla-central/source/layout/base/FrameLayerBuilder.cpp?rev=12e87a6861d7#4760

is the only place that the opaque flag could get set to not trigger that assertion. So I guess find out why it's not getting set there.
I'm really not familiar with this code, so don't trust me too much on this, but apparently, in ContainerState::ComputeOpaqueRect, aAnimatedGeometryRoot is never equal to mContainerAnimatedGeometryRoot, so aList->SetIsOpaque() is never called.

Not sure if that helps is any way.
(In reply to Paul Rouget [:paul] from comment #6)
> I'm really not familiar with this code, so don't trust me too much on this,
> but apparently, in ContainerState::ComputeOpaqueRect, aAnimatedGeometryRoot
> is never equal to mContainerAnimatedGeometryRoot, so aList->SetIsOpaque() is
> never called.
> 
> Not sure if that helps is any way.

Well, removing the test doesn't help. So that's not it.
I think this is because of the scrollbars on osx.
In content.css, setting scrollbars to `display:none` will greatly reduce the amount of warnings.

So do scrollbars actually slow down rendering?
If so, how do we fix that?
If not, can we make the warning a bit less aggressive?


Also, for something as simple as this:
<iframe remote="true" mozbrowser="true" src="data:text/html,<div style='height:300px'></div>"></iframe>
I still get 2 warning on scroll, even with the scrollbar display:none, and I haven't figured out yet what's going on.
Flags: needinfo?(botond) → needinfo?(tnikkel)
I was hoping to get around to investigating this bug this week, but that didn't work out, sorry. I'll give it a shot next week.
Blocks: graphene
This looks to be caused by rounding error. At this check [1] for the root layer, the opaque region contains a single rectangle (0,0,1920,1062), while the container bounds are (0,0,1921,1063). The former does not contain the latter, so the opaque flag is never set.

[1] http://mxr.mozilla.org/mozilla-central/source/layout/base/FrameLayerBuilder.cpp?rev=31459ec4ef93#4391
A rounding error like this likely has several root causes, and tracking them down is fairly tedious (though rr helps considerably).

I tracked down one of the root causes. It was related to full-zoom. I was running with a full-zoom of 133% (that is, 1.33, exactly, with no trailing 3s). nsDocument::GetViewportInfo() uses this exact amount [1] to compute the scale that is then used [2] to calculate the CSS viewport size; the CSS viewport size thus becomes precisely (1/1.33)th of the screen size.

TabChild uses the ratio of the screen size to the viewport size to compute its initial zoom [3], so it recovers the 1.33 exactly.

To compute the pres shell resolution, TabChild divides the zoom by the device scale [4]. The device scale, however, is calculated based on the number of app units per device pixel [5], and *this* quantity is an integer. With 60 app units per CSS pixel, the exact number of app units per device pixel at 1.33 full-zoom is 45.11, but that gets rounded to 45, and as a result the scale that is then recovered from this calculation is 1.333333333.

As a result, we calculate a pres shell resolution of 1.33 / 1.33333333 = 0.9975, even though there has been no pinch-zooming (or other things that we'd expect to set a pres shell resolution). This then trickles down through Layout to eventually give us the 1-pixel difference described in comment 10.

--------

It's not immediately clear how this mismatch should be resolved: should things like nsDocument::GetViewportInfo() be adjusting the full zoom to the nearest value that gives a whole number of app units per device pixel?

--------

Note that even if we fix this root cause, the warning doesn't go away, it just becomes slightly less frequent, suggesting that there are other root causes too. Those remain to be investigated.


[1] http://mxr.mozilla.org/mozilla-central/source/dom/base/nsDocument.cpp?rev=f0312c7f7e88#7893
[2] http://mxr.mozilla.org/mozilla-central/source/dom/base/nsDocument.cpp?rev=f0312c7f7e88#8081
[3] http://mxr.mozilla.org/mozilla-central/source/dom/ipc/TabChild.cpp?rev=a69094e0f2a4#375
[4] http://mxr.mozilla.org/mozilla-central/source/dom/ipc/TabChild.cpp?rev=a69094e0f2a4#393
[5] http://mxr.mozilla.org/mozilla-central/source/dom/ipc/TabChild.cpp?rev=a69094e0f2a4#389
> Note that even if we fix this root cause, the warning doesn't go away, it just becomes slightly less frequent, suggesting that there are other root causes too. Those remain to be investigated.

Not sure if that helps or not, but on OSX, most of the warnings come from the scrollbars. See comment 8.
Scrollbars are placed at a specific spot by layout, and then transform again by the compositor before drawing to the screen. So the location of the scrollbars (as far as layout sees) isn't relevant to the final composited image. So we might need something special to handle scrollbars.
I would like to understand if scrolling performance is actually impacted by the fact that scrollbars have a transparent background.

Here, I get around 20 warnings every time I swipe up and down to scroll.
re comment 12: today, even without scrollbars I keep getting the warnings.
I can't even move my mouse without getting a ton of these, and I could care less about transparent content. This warning is too aggressive or too broad or both. Please fix warning asap even if root cause is harder.
Let's disable the warning while we continue investigating its root cause.
Bug 1155137 - Disable spammy layout warnings by default. r=roc

The rationale is that they fire way too frequently to be useful to anyone
who's not specifically investigating them.

The warning about transparent content with displayports being expensive
is the only such warning currently.
Attachment #8657215 - Flags: review?(roc)
Comment on attachment 8657215 [details]
MozReview Request: Bug 1155137 - Disable spammy layout warnings by default. r=roc

https://reviewboard.mozilla.org/r/18333/#review16447
Attachment #8657215 - Flags: review?(roc) → review+
Let's group-land this workaround. There's no try push because it's just a pref flip, and the pref just controls a single warning.
Hi, this patch doesn't apply cleanly to m-i

applying 264ed23109b0
patching file gfx/layers/apz/src/APZCTreeManager.cpp
Hunk #1 FAILED at 1542
1 out of 1 hunks FAILED -- saving rejects to file gfx/layers/apz/src/APZCTreeManager.cpp.rej

could you take a look ? Thanks!
Flags: needinfo?(botond)
I'm not sure what patch you tried to apply - the one in the MozReview request attached to this bug, which is the only attachment to this bug, doesn't touch APZCTreeManager.cpp.

Anyways, I'll just go ahead and land this myself.
Flags: needinfo?(botond)
I don't see myself getting to doing further investigation of this in the next month or so, I've got higher priority things on my plate. Unassigning in case someone else wants to give it a shot.
Assignee: botond → nobody
The leave-open keyword is there and there is no activity for 6 months.
:davidb, maybe it's time to close this bug?
Flags: needinfo?(dbolter)
Botond want to close this?
Flags: needinfo?(dbolter) → needinfo?(botond)
I enabled the warning on debug builds of desktop and fennec, and browsed around a bit. I'm still getting spammed with the warning on the desktop build (though not fennec).

I still think it would be good to investigate the underlying causes, but it's not a high priority.

Also, it would make sense to do it in a different bug, as a patch has landed in this bug.
Status: NEW → RESOLVED
Closed: 10 months ago
Flags: needinfo?(tnikkel)
Flags: needinfo?(botond)
Resolution: --- → FIXED
See Also: → 1509909
(In reply to Botond Ballo [:botond] from comment #28)
> Also, it would make sense to do it in a different bug, as a patch has landed
> in this bug.

Filed bug 1509909 for this.
Assignee: nobody → botond
You need to log in before you can comment on or make changes to this bug.