676248 - Make the window GL context transparent

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Markus Stange [:mstange]]

We did this in bug 596303 but backed it out for performance reasons.
However, we'll need it for bug 675410 and bug 676241.

I think I've tracked down the real reasons why making the GL context transparent regressed performance:
 - it seems to force on V-Sync
 - we start repainting window background behind our GL context

Every time we touch the window buffer (like when repainting the window background), something tries to sync the window buffer surface with the GL context surface and blocks on GL flush completion.

We can work around that by moving our GL repaints away from setNeedsDisplay and drawRect. Then we'll no longer redraw the window background unnecessarily and we'll no longer block on anything.

But until we can activate V-Sync (bug 555834), we can't make the context transparent.

Comment 1

[Markus Stange [:mstange]]

Attachment: make it transparent

No idea who to ask for review but since I'm making Josh review the rest of the work for bug 675410, he gets this one too.

Comment 2

[Markus Stange [:mstange]]

OpenGL V-Sync has already been enabled in bug 748816, so this shouldn't regress anything as soon as we have bug 676250.

Comment 3

[Benoit Girard (:BenWa)]

Comment on attachment 675320 [details] [diff] [review]
make it transparent

I'm ok with this patch if it doesn't regress performance. If it does regress performance we should quantify it first.

Have you had a chance to compare some profiles before or after for operations like 60Hz compositing and resizing? I find them very informative when we're changing a significant code paths.

Comment 4

[Mark Rejhon]

(In reply to Benoit Girard (:BenWa) from comment #3)
> Have you had a chance to compare some profiles before or after for
> operations like 60Hz compositing and resizing? I find them very informative
> when we're changing a significant code paths.

Don't neglect us 120Hz computer monitor owners please.  Asus VG236H, Asus VG278H, Samsung S23A700D, Benq XL2420T, Acer GD235Hz.

Perhaps a window.mozMaxFrameRate = 120 setting, to allow us JavaScript programmers to let everything in the browser take advantage of the rate.  Ideally, I prefer the Chrome behaviour (automatically allow requestAnimFrame to run at 120Hz), but I'll settle for Internet Explorer behaviour (sync to VSYNC but only at 60Hz, skip every other VSYNC at 120Hz), as long as I can access a JavaScript properties to control the VSYNC and the internal framelimit (window.mozMaxFrameRate = 120 and window.mozSyncOnVsync = true)

Comment 5

[Markus Stange [:mstange]]

(In reply to Benoit Girard (:BenWa) from comment #3)
> Have you had a chance to compare some profiles before or after for
> operations like 60Hz compositing and resizing? I find them very informative
> when we're changing a significant code paths.

Yes, and I haven't found any regressions.

Comment 6

[Benoit Girard (:BenWa)]

(In reply to Mark Rejhon from comment #4)
> (In reply to Benoit Girard (:BenWa) from comment #3)
> > Have you had a chance to compare some profiles before or after for
> > operations like 60Hz compositing and resizing? I find them very informative
> > when we're changing a significant code paths.
> 
> Don't neglect us 120Hz computer monitor owners please.  Asus VG236H, Asus
> VG278H, Samsung S23A700D, Benq XL2420T, Acer GD235Hz.

Sorry I missed your comment. 60Hz in this context was not in any ways an upper bound. Proper vsync is very important to us but we need to solve some other bugs first.

Comment 7

[Mark Rejhon]

(In reply to Benoit Girard (:BenWa) from comment #6)
> > Don't neglect us 120Hz computer monitor owners please.  Asus VG236H, Asus
> > VG278H, Samsung S23A700D, Benq XL2420T, Acer GD235Hz.
> 
> Sorry I missed your comment. 60Hz in this context was not in any ways an
> upper bound. Proper vsync is very important to us but we need to solve some
> other bugs first.

Understood.  For some context, Webkit browsers such as Chrome synchronizes with VSYNC pretty well at all refresh rates, from 50 Hz all the way to beyond 144 Hz; yielding flawless-looking 144fps@144Hz operation.   From what I know, FireFox has an artifical 60fps framelimit for all rendering (not just Canvas2D or WebGL) that affects fluidity on non-60Hz displays.

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Attachment: Patch v1.1 (r+'d by bgirard)

I've de-bitrotted this patch, and I ran some profiles with and without the patch.

I ran this demo: https://developer.mozilla.org/en-US/demos/detail/kai-opua
And rotated the camera around a bit, and then resized the window.

Here's the profile without the patch:

http://people.mozilla.com/~bgirard/cleopatra/#report=712ceb292adfe496b85233e2b88d1a1773083593

and here's the profile with the patch:

http://people.mozilla.com/~bgirard/cleopatra/#report=dc05af436747e583332d9dcb5cfa33245ab9e2da

Does anything in there strike you as particularly worrying in terms of performance?

Comment 9

[Kelsey Gilbert [:jgilbert]]

(In reply to Mark Rejhon from comment #7)
> (In reply to Benoit Girard (:BenWa) from comment #6)
> > > Don't neglect us 120Hz computer monitor owners please.  Asus VG236H, Asus
> > > VG278H, Samsung S23A700D, Benq XL2420T, Acer GD235Hz.
> > 
> > Sorry I missed your comment. 60Hz in this context was not in any ways an
> > upper bound. Proper vsync is very important to us but we need to solve some
> > other bugs first.
> 
> Understood.  For some context, Webkit browsers such as Chrome synchronizes
> with VSYNC pretty well at all refresh rates, from 50 Hz all the way to
> beyond 144 Hz; yielding flawless-looking 144fps@144Hz operation.   From what
> I know, FireFox has an artifical 60fps framelimit for all rendering (not
> just Canvas2D or WebGL) that affects fluidity on non-60Hz displays.

Yes, our 'refresh driver' is not actually what it says it is, and it gives us jittery 60Hz timings, regardless of display refresh rate. If we enable vsync on non-60Hz displays, the effects could be especially nasty.

Comment 10

[Benoit Girard (:BenWa)]

(In reply to Mike Conley (:mconley) from comment #8)
> Does anything in there strike you as particularly worrying in terms of
> performance?

Sadly the profile are JS bound and don't have full stacks. I did notice that both builds are spending 6% getting timestamp under LayerManagerOGL::Render but I can't find the caller without full stacks. This patch doesn't appear to make these profiles worse but it's hard to tell for sure.

Comment 11

[Mike Conley (:mconley) (:⚙️)]

(In reply to Benoit Girard (:BenWa) from comment #10)
> (In reply to Mike Conley (:mconley) from comment #8)
> > Does anything in there strike you as particularly worrying in terms of
> > performance?
> 
> Sadly the profile are JS bound and don't have full stacks.

Is there any way I can get you full stacks?

Comment 12

[Benoit Girard (:BenWa)]

(In reply to Mike Conley (:mconley) from comment #11)
> (In reply to Benoit Girard (:BenWa) from comment #10)
> > (In reply to Mike Conley (:mconley) from comment #8)
> > > Does anything in there strike you as particularly worrying in terms of
> > > performance?
> > 
> > Sadly the profile are JS bound and don't have full stacks.
> 
> Is there any way I can get you full stacks?

Lets land this and I'll keep an eye for changes in profiles. In the long run this will fix the titlebar drawing which will help performance.

Comment 13

[Mike Conley (:mconley) (:⚙️)]

Done. Landed on mozilla-inbound as https://hg.mozilla.org/integration/mozilla-inbound/rev/635e926d9e4f

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/635e926d9e4f