The default bug view has changed. See this FAQ.

GMail inbox get rendered as component alpha layer

RESOLVED FIXED in Firefox 16

Status

()

Core
Layout: View Rendering
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: bas, Assigned: roc)

Tracking

(Blocks: 1 bug, {regression})

16 Branch
mozilla18
x86_64
Windows 7
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox16 fixed, firefox17 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

5 years ago
When playing around with my new 2D gfx recording and replaying tools I noticed gmail is rendered as a single, large, component alpha layer. This will be very expensive particularly when software rendering with D3D9 acceleration (or on Mac for example). What seems to be the transparent part of the page in the recording is the folder list on the left and the headers on the top. The rest appears to actually be opaque.
Component: Layout → Layout: View Rendering
We can probably fix this quite easily by using a heuristic to make the GMail page's root scroll frame non-active, since it can never be scrolled.
Maybe treat root scroll frames that have overflow: hidden like any other scroll frame? Active only when scrolled.
Yeah.
Blocks: 789573
FWIW I don't see this on D3D10 in my GMail configuration. I see just one component alpha layer, and it's not very big:
D3D10ThebesLayer (0x1b1bec00) [transform=[ 1 0; 0 1; 1 123; ]] [visible=< (x=0, y=0, w=1229, h=15); (x=0, y=15, w=1229, h=87); (x=0, y=116, w=1229, h=567); >] [componentAlpha] [isFixedPosition] [valid=< (x=0, y=0, w=1229, h=15); (x=0, y=15, w=1229, h=87); (x=0, y=116, w=1229, h=567); >]
Created attachment 660705 [details] [diff] [review]
fix

This should help, at least when you're not scrolling.
Assignee: nobody → roc
Attachment #660705 - Flags: review?(matspal)
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #4)
> FWIW I don't see this on D3D10 in my GMail configuration. I see just one
> component alpha layer, and it's not very big:
> D3D10ThebesLayer (0x1b1bec00) [transform=[ 1 0; 0 1; 1 123; ]] [visible=<
> (x=0, y=0, w=1229, h=15); (x=0, y=15, w=1229, h=87); (x=0, y=116, w=1229,
> h=567); >] [componentAlpha] [isFixedPosition] [valid=< (x=0, y=0, w=1229,
> h=15); (x=0, y=15, w=1229, h=87); (x=0, y=116, w=1229, h=567); >]

This comment is, of course, completely wrong. That layer is plenty big.
Comment on attachment 660705 [details] [diff] [review]
fix

Oops, this doesn't help.
Attachment #660705 - Attachment is obsolete: true
Attachment #660705 - Flags: review?(matspal)
GMail's inner IFRAME has a white background on the body, which isn't a problem, but it also has a DIV (class="wl" in my instance) which is position:fixed; height:100%; width:100%; background:white ... and no content. I'm not sure what it's supposed to be for, but we give it its own layer and then we have to draw the rest of the content on top with component alpha.
Created attachment 660714 [details] [diff] [review]
fix

This fixes it.

Note that the root scroll frame for the root content document normally is active. So this only affects subframes that aren't actively being scrolled. In such a case the page content's root active scrollframe is the viewport (or higher) anyway, so putting fixed position content in its own layer is pointless.
Attachment #660714 - Flags: review?(chrislord.net)
The good news is, with this patch, even when we scroll (in the main view anyway, I didn't test the others), we don't get component-alpha layers. In fact, the layer setup is entirely sane:

    D3D10ContainerLayer (0x8523540) [clip=(x=1, y=123, w=1229, h=683)] [visible=< (x=1, y=123, w=1229, h=683); >] [opaqueContent] [isFixedPosition]
      D3D10ThebesLayer (0x8523700) [transform=[ 1 0; 0 1; 1 123; ]] [visible=< (x=0, y=0, w=1229, h=683); >] [opaqueContent] [isFixedPosition] [valid=< (x=0, y=0, w=1229, h=683); >]
      D3D10ThebesLayer (0x18e45480) [transform=[ 1 0; 0 1; 205 -85; ]] [visible=< (x=0, y=368, w=991, h=523); >] [opaqueContent] [isFixedPosition] [valid=< (x=0, y=368, w=991, h=523); >]

This is because the scrollable pane has its own background:white so the scrollable content is not transparent.
Looks good to me, but I want to try it out on mobile first... Doing so now.
Comment on attachment 660714 [details] [diff] [review]
fix

Review of attachment 660714 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/generic/nsFrame.cpp
@@ +2160,5 @@
> +  // Don't build an nsDisplayFixedPosition if our root scroll frame is not
> +  // active, that's pointless and the extra layer(s) created may be wasteful.
> +  bool buildFixedPositionItem = disp->mPosition == NS_STYLE_POSITION_FIXED &&
> +    !child->GetParent()->GetParent() && !isSVG &&
> +    IsRootScrollFrameActive(PresContext()->PresShell());

Why is the IsInFixedPosition() check removed? If that's purposeful, the comment above needs to be altered. If it's not purposeful, I'd suggest moving the !isSVG check to be last, as it's likely to always be true.

Will r+ with this clarified.
Created attachment 661053 [details] [diff] [review]
fix v2

It was not intentional, good catch!
Attachment #660714 - Attachment is obsolete: true
Attachment #660714 - Flags: review?(chrislord.net)
Attachment #661053 - Flags: review?(chrislord.net)
Comment on attachment 661053 [details] [diff] [review]
fix v2

Review of attachment 661053 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me.
Attachment #661053 - Flags: review?(chrislord.net) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/5c68c842a1fc
Comment on attachment 661053 [details] [diff] [review]
fix v2

Review of attachment 661053 [details] [diff] [review]:
-----------------------------------------------------------------

We need this patch to fix a regression in GMail rendering performance in aurora/beta. The patch is simple and safe; it just turns off nsDisplayFixedPosition in more cases.
Attachment #661053 - Flags: approval-mozilla-beta?
Attachment #661053 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/5c68c842a1fc
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Triage drive-by, will wait until this has been on Nightly a bit longer before approving.
Comment on attachment 661053 [details] [diff] [review]
fix v2

[Triage Comment]
Approving for Aurora 17 regardless.

This a+ for Beta 16 is *only* if this is a new regression (FF15 or 16) given where we are in the cycle (please include a regressing bug # if so). Decided to approve instead of ask the question to speed the process up.
Attachment #661053 - Flags: approval-mozilla-beta?
Attachment #661053 - Flags: approval-mozilla-beta+
Attachment #661053 - Flags: approval-mozilla-aurora?
Attachment #661053 - Flags: approval-mozilla-aurora+

Updated

5 years ago
Keywords: regression
This is a regression from bug 758620.
Blocks: 758620
https://hg.mozilla.org/releases/mozilla-aurora/rev/a06f7b317f78

https://hg.mozilla.org/releases/mozilla-beta/rev/a0213afa1514

Updated

5 years ago
status-firefox16: --- → fixed
status-firefox17: --- → fixed
Version: unspecified → 16 Branch

Updated

3 years ago
Depends on: 934705
You need to log in before you can comment on or make changes to this bug.