1255277 - Avoid using glass theme when we don't need it

Status: NEW

(Core :: Widget: Win32, defect, P5)

Description

[Jeff Muizelaar [:jrmuizel]]

We end up making transparent windows with aeroglass. This can hurt performance. It looks like we're using glass on Windows 8+ which I don't think is necessary.

Comment 1

[Sotaro Ikeda [:sotaro]]

Attachment: patch - Avoid using glass theme when we don't need it

Comment 2

[Sotaro Ikeda [:sotaro]]

Attachment: MozReview Request: Bug 1255277 - Avoid using glass theme when we don't need it

Review commit: https://reviewboard.mozilla.org/r/40009/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/40009/

Comment 3

[Sotaro Ikeda [:sotaro]]

attachment 8730565 [details] seems to address the problem. Though my understanding to browser-aero.css is not good enough.

Comment 4

[:Gijs (he/him)]

Comment on attachment 8730565 [details]
MozReview Request: Bug 1255277 - Avoid using glass theme when we don't need it

https://reviewboard.mozilla.org/r/40009/#review36539

Where did you test this? Why do you think this is the right thing to do?

As it is, this uses the grey background and titlebar button styling from Windows 10 on Windows 8, which is definitely wrong, and makes me think you didn't test this on either win10 or win8. There might be more wrong (in fact, I don't think you can get rid of the -moz-appearance: glass stuff like this either) but tbh I think this needs a total rethink and more testing, and it's not particularly useful for me to go through this line by line.

What are we actually trying to achieve here? Jim?

Comment 5

[Sotaro Ikeda [:sotaro]]

Thanks for the comment. attachment 8730565 [details] was tried to remove glass on windows 8. "transparent windows with aeroglass" was disabled since windows 8. Therefore we does not get transparent window even when the glass is set.

Widget code expects transparent window when the glass is set. Then it allocates BGRA surface for screen compositing. It slowed down the compositing performance when BasicCompositor was used. Removing the glass could improve BasicCompositor's compositing performance a lot.

Can you give an advice about how to disable transparent window setting?

Comment 6

[:Gijs (he/him)]

(In reply to Sotaro Ikeda [:sotaro] from comment #5)
> "transparent windows with aeroglass" was disabled since
> windows 8. 

I don't know what this statement means, specifically. Are you saying we made a change in widget/windows somewhere? What bug?

If you're simply saying that "aero" styles are no longer present on Windows 8, that is more or less true (you can get some interesting results if the dwm crashes) but the "transparent window" part is still present.

> Therefore we does not get transparent window even when the glass
> is set.

This is not true. The Windows 8 accent color and the caption buttons are shown at the top of the window because the window is rendered transparently. If you remove the -moz-appearance for glass and borderless-glass, which your patch does, we will probably use -moz-appearance:window, which is not transparent. Practically speaking, I just tried this from the browser toolbox and it turns the formerly-transparent top of the window completely black on my Windows 8 machine.

Did you not see any of these effects when you wrote the patch?

> Widget code expects transparent window when the glass is set. Then it
> allocates BGRA surface for screen compositing. It slowed down the
> compositing performance when BasicCompositor was used. Removing the glass
> could improve BasicCompositor's compositing performance a lot.
> 
> Can you give an advice about how to disable transparent window setting?

No, because I have no idea about the graphics side of this. We clearly need the transparent rendering, otherwise the window background is not shown. Even on Windows 10 we need some transparency in order to show the (1 device pixel wide) window border in the correct color. Is there any performance impact besides the transparency that we're incurring through the use of glass and/or borderless-glass? If not, could we use the transparency for a smaller portion of the window, especially on Windows 10? Either way it would take some subtantial amount of work in widget/windows/ code, potentially some new CSS moz-appearance settings or whatever, and only after that could we fix this with just some CSS.

Your patch basically:

1) removes the "borderless-glass" moz-appearance setting and associated styling for windows 8;
2) extends the windows 10 styling to windows 8
3) breaks that styling by removing the 'glass' moz-appearance setting there as well.
4) leaves some of the other changes relating to glass (ie the exclude-glass properties that are being set)

So I'm still just as confused as to what you are trying to accomplish. Maybe :jrmuizel or :jimm can help?