Closed Bug 620608 Opened 9 years ago Closed 9 years ago

Firefox Aero theme requires unnecessarily large ThebesLayer

Categories

(Firefox :: Theme, defect)

x86
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 4.0b9
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: roc, Assigned: dao)

References

Details

Attachments

(3 files)

Here's what our layer tree looks like currently when viewing about:blank:

BasicLayerManager (0xadbf130)
  BasicContainerLayer (0xb14d488) [visible=< (x=0, y=0, w=1144, h=962); >] [metrics={ viewport=(w=1144, h=962) viewportScroll=(x=0, y=0) displayport=(x=0, y=0, w=0, h=0) }]
    BasicThebesLayer (0xb798688) [visible=< (x=0, y=0, w=1144, h=961); >] [valid=< (x=0, y=0, w=1144, h=961); >]
    BasicContainerLayer (0xb14d5f0) [clip=(x=0, y=0, w=1144, h=962)] [visible=< (x=0, y=98, w=1, h=863); >]
      BasicThebesLayer (0xb798840) [clip=(x=0, y=0, w=0, h=0)]
      BasicColorLayer (0xb238e20) [clip=(x=0, y=98, w=1, h=863)] [visible=< (x=0, y=98, w=1, h=863); >] [color=rgba(26, 26, 26, 0.4)]
    BasicContainerLayer (0xb14d758) [clip=(x=1, y=98, w=1142, h=863)] [visible=< (x=1, y=98, w=1142, h=863); >] [opaqueContent]
      BasicThebesLayer (0xb7989f8) [clip=(x=0, y=0, w=0, h=0)] [transform=[ 1 0; 0 1; 1 98; ]]
      BasicColorLayer (0xb239630) [clip=(x=1, y=98, w=1142, h=863)] [transform=[ 1 0; 0 1; 1 98; ]] [visible=< (x=0, y=0, w=1142, h=863); >] [opaqueContent] [color=rgba(255, 255, 255, 1)]
    BasicContainerLayer (0x4da2c38) [clip=(x=0, y=0, w=1144, h=962)] [visible=< (x=1143, y=98, w=1, h=863); >]
      BasicThebesLayer (0xb798bb0) [clip=(x=0, y=0, w=0, h=0)]
      BasicColorLayer (0xb239788) [clip=(x=1143, y=98, w=1, h=863)] [visible=< (x=1143, y=98, w=1, h=863); >] [color=rgba(26, 26, 26, 0.4)]
    BasicContainerLayer (0xb918ed8) [clip=(x=0, y=0, w=1144, h=962)] [visible=< (x=0, y=961, w=1144, h=1); >] [opaqueContent]
      BasicThebesLayer (0xb798d68) [visible=< (x=0, y=961, w=1144, h=1); >] [opaqueContent] [valid=< (x=0, y=961, w=1144, h=1); >]

The first BasicThebesLayer is way too large.

The problem is that the #browser element has background:-moz-dialog due to this rule in browser-aero.css:
http://mxr.mozilla.org/mozilla-central/source/browser/themes/winstripe/browser/browser-aero.css#161

Since bug 588764 was fixed, the #browser element sticks out by one pixel to the left and right of the content area. Those vertical strips are covered by #browser-border-start and #browser-border-end, but the backgrounds of those elements are not opaque. Therefore the #browser element is visible on both sides of the content area, so its visible rect contains the entire content area. The visible region of the chrome ThebesLayer has to include that rect.

This means we'll be keeping around a large buffer in VRAM that's mostly not visible, and compositing that buffer every time we paint. This will especially be hurting people who are using Aero Glass but who we've blacklisted for D3D; it forces us to always take a slower "manual backbuffer" path.

I think the best fix would be to change the color of the #browser-border-start and #browser-border-end elements to be solid. This could address bug 618399 at the same time.
By "solid" I mean opaque.

For example, this patch fixes it:

   #main-window[sizemode="normal"] #browser-border-start,
   #main-window[sizemode="normal"] #browser-border-end {
     display: -moz-box;
-    background-color: @glassToolbarBorderColor@;
+    background-color: rgb(10%, 10%, 10%);
     width: 1px;
   }

I guess you'd need to change the following rule for the bottom-border too.

I'm sure that's not the right color, someone will have to figure out what the right color is.
blocking2.0: --- → ?
If it helps, right now what ends up in those vertical strips is exactly @glassToolbarBorderColor@ over -moz-dialog.
(In reply to comment #0)
> The problem is that the #browser element has background:-moz-dialog due to this
> rule in browser-aero.css:
> http://mxr.mozilla.org/mozilla-central/source/browser/themes/winstripe/browser/browser-aero.css#161

Would removing this be a solution?

I'm pretty sure we don't want an opaque border, as it would be too dark or too light depending on the glass background.
(In reply to comment #3)
> (In reply to comment #0)
> > The problem is that the #browser element has background:-moz-dialog due to this
> > rule in browser-aero.css:
> > http://mxr.mozilla.org/mozilla-central/source/browser/themes/winstripe/browser/browser-aero.css#161
> 
> Would removing this be a solution?

Yes.

> I'm pretty sure we don't want an opaque border, as it would be too dark or too
> light depending on the glass background.

OK.
blocking2.0: ? → betaN+
Assignee: nobody → dao
Blocks: 618399
Attached patch patchSplinter Review
I gave #browser a background to work around some weirdness in the widget code determining which areas should be on glass (transparent ones) and which shouldn't (those with a solid background). I think it's not needed anymore. As roc mentioned, this fixes bug 618399 as far as #browser is concerned. Adding background-clip: padding-box fixes it for #browser-bottombox too.
Attachment #499123 - Flags: review?(gavin.sharp)
Comment on attachment 499123 [details] [diff] [review]
patch

rs=me
Attachment #499123 - Flags: review?(gavin.sharp) → review+
http://hg.mozilla.org/mozilla-central/rev/d37e24f10e8d
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 4.0b9
Depends on: 622328
Depends on: 621222
Dão, also after the land of this patch the color of the border doesn't mach perfectly the color of the toolbar, so I did some tests and  if you change this line:

#main-window[sizemode="normal"] #browser-border-start,
#main-window[sizemode="normal"] #browser-border-end {
 display: -moz-box;
 background-color: @glassToolbarBorderColor@;
 width: 1px;
}

with this the color will match perfectly:

#main-window[sizemode="normal"] #browser-border-start,
#main-window[sizemode="normal"] #browser-border-end {
 display: -moz-box;
 background: @glassToolbarBorderColor@ padding-box !important;
 width: 1px;
}
(In reply to comment #8)
Thanks for your suggestion, but I don't see why this would make a difference. #browser-border-start and #browser-border-end don't have any border or padding.
Attached image Border
I don't know why this make a difference but I made a test disabling the transparency in windows 7 and I have seen that without this correction there is a difference between the color of the toolbar and the color of the content area. I will made you a screenshot (you will see better the difference zooming the image because it's not in high resolution)
Attached image better image
This image show better the difference
You need to log in before you can comment on or make changes to this bug.