Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

With --no-native-titlebar, requestAnimationFrame runs at 500 FPS #9431

Closed
paulrouget opened this issue Jan 26, 2016 · 12 comments · Fixed by servo/glutin#68
Closed

With --no-native-titlebar, requestAnimationFrame runs at 500 FPS #9431

paulrouget opened this issue Jan 26, 2016 · 12 comments · Fixed by servo/glutin#68
Assignees
Labels
P-mac Any version of MacOS

Comments

@paulrouget
Copy link
Contributor

Probably a glutin/cocoa issue.

I'll investigate.

@paulrouget
Copy link
Contributor Author

/System/Library/Frameworks/AppKit.framework/Versions/C/Headers/NSWindow.h about NSFullSizeContentViewWindowMask:

If set, the contentView will consume the full size of the window; it can be combined with other window style masks, but is only respected for windows with a titlebar.
Utilizing this mask opts-in to layer-backing. Utilize the contentLayoutRect or auto-layout contentLayoutGuide to layout views underneath the titlebar/toolbar area.

@jdm jdm added the P-mac Any version of MacOS label Jan 26, 2016
@paulrouget
Copy link
Contributor Author

NSFullSizeContentViewWindowMask is used when the -b option is provided.

Doc says:

Utilizing this mask opts-in to layer-backing

I think this is why it's not working. But I don't know enough about Cocoa to understand what needs to be done here.

Not sure if that helps in anyway, but calling view.setLayer(YES) in Glutin when NSFullSizeContentViewWindowMask when is not used makes FPS go up to 500 as well.

@glennw - any idea?

@pcwalton
Copy link
Contributor

Cocoa or Core Animation might be setting the swap interval to zero when layers are being used. (When setLayer: is used, Core Animation takes over.) This can be tested using the APIs here (which also contains an explanation of what the swap interval is): https://developer.apple.com/library/mac/documentation/GraphicsImaging/Conceptual/OpenGL-MacProgGuide/opengl_contexts/opengl_contexts.html#//apple_ref/doc/uid/TP40001987-CH216-SW24

If this is the case, I don't know to get it to stop doing that. We could try to reset the swap interval back to 1 ourselves, but that might cause Core Animation to break somehow, or CA might try to reset it back to 0. Failing that, it might be best to use some other method to disable the title bar that doesn't get Core Animation involved.

@pcwalton
Copy link
Contributor

The problem here seems to be that, when a window is layer-backed, OpenGL contexts created for the window's content view are offscreen contexts displayed by drawing to an off-screen buffer and swapping that buffer onto the screen via Core Animation. This is a problem because, since the contexts are offscreen, swap_buffers() doesn't have a vertical blanking interval to swap to and therefore returns immediately regardless of the value of kCGLCPSwapInterval. I experimented with a hack that enabled NSFullSizeContentViewWindowMask without creating a backing layer (by forcibly clearing the the bit in the _NSViewAuxiliary._vFlags2 bitfield) and the result was that the window indeed loses its title bar—but the traffic light buttons don't show up. I suspect this is because the traffic light buttons are Core Animation layers composited on top of the window's view, and that's why NSFullSizeContentViewWindowMask requires that the window has a layer.

There are two realistic options here that I can see: (1) keep the current state of affairs and use a CVDisplayLink to try to figure out what the vertical blanking interval is; (2) create a title-bar-less window and implement our own close/minimize/full-screen buttons. I'm inclined toward (2), because (a) CVDisplayLink is often a lie on desktop, last I checked, and will cause tearing; (b) if we go with the Core Animation approach I think we'll have our compositor sitting on top of Core Animation's compositor for no real purpose other than to draw traffic light buttons, which seems like overkill.

@paulrouget, what do you think? How do you feel about implementing the traffic light buttons in browser.html?

@paulrouget
Copy link
Contributor Author

How do you feel about implementing the traffic light buttons in browser.html?

We can do that (we actually already have such HTML buttons).

What should be done in Glutin to have a titlebar-less window with an on-screen context? Do you think we should use your NSViewAuxiliary hack?

@pcwalton
Copy link
Contributor

NSBorderlessWindowMask oughta do the trick.

@paulrouget
Copy link
Contributor Author

I tried that in the past. But without NSFullSizeContentViewWindowMask, I never managed to get a proper looking window with borders, rounded corners and shadow.

@mstange
Copy link
Contributor

mstange commented Jan 27, 2016

Here's how we do this in Firefox: We don't mess with the window mask at all - we don't add NSBorderlessWindowMask (because that affects the shadow, and the rounded corners in the shadow), and we also don't set NSFullSizeContentViewWindowMask because I didn't even know it existed. Instead, we get our content view to cover the whole window by overriding the contentRectForFrameRect:styleMask: method (and a few others) in our NSWindow subclass.

As you already found out, this causes the window buttons to disappear. Moreover, our OpenGL drawing is not clipped to the rounded window corners. We fix both of those things up in a DrawWindowOverlay function that gets called by the compositor on every composite. There is a "titlebar" texture that contains the window buttons (and the titlebar highlight line, and in some cases even the window title text), and there's a "corner mask" texture that we use to mask out the four corners (using operator destination-in).
We get the window buttons into the titlebar texture by iterating over the NSViews in our window frame and drawing them at their correct location. We update the texture when necessary before we send a new layers transaction from the main thread to the compositor; and we know what parts of it to update by overriding [NSWindow _setNeedsDisplayInRect:] in our window subclass (really!? I had completely forgotten about that hack).

This is a fair bit of complexity. I'd try to avoid it if possible. Using a layer-backed view and having two compositors doesn't sound all that bad, as long as the performance is still good. (Is it?) There shouldn't be tearing because I'm pretty sure CoreAnimation has the swap interval enabled on its OpenGL context. So if you use CVDisplayLink to fix your compositor scheduling, all that could happen is that you might miss a frame every once in a while if CVDisplayLink doesn't completely sync up with CoreAnimation's frame scheduling mechanism. Though I'd expect CoreAnimation to use CVDisplayLink for scheduling. So I think that's definitely worth a try.

@pcwalton
Copy link
Contributor

It seems like a real shame to double our GPU ROP count just for this, though, and missing frames seems bad. (I was actually just in meetings with web developers who were complaining that they can't get good animation performance due to, ultimately, CVDisplayLink being a lie.) I'd like to see if we can't figure out some way to do what we want with window masks; my hack sounds potentially better than either of the options on the table, honestly.

@pcwalton
Copy link
Contributor

After doing some research, I think the corners may actually be something we have to do in WebRender. The reason is that the way Mac OS X draws corners is one of:

(1) Use Quartz clipping regions to clip out the corner areas when doing 2D vector drawing of our window; or

(2) Use Core Animation corners on a layer. I assume the way this works is that WebRender ultimately draws to a texture, then this texture gets either alpha masked or stenciled to draw the corners in Core Animation.

(2) is easier for us, but it's not the most efficient way to draw corners, and it'll cause us to have to fall back to CVDisplayLink, which I've had problems with in the past. WebRender already knows how to clip out corners, though; it has to be able to do this because of border-radius and overflow: hidden. So maybe we should just instruct WR to clip out the corners.

Shadows should be easy to get back; Apple has some sample code for this: https://developer.apple.com/library/mac/samplecode/RoundTransparentWindow/Introduction/Intro.html

@paulrouget
Copy link
Contributor Author

Related: #7659

pcwalton added a commit to pcwalton/glutin that referenced this issue Feb 3, 2016
because that disables the swap interval.

This hack is very simple but a little evil. We get the superview of the
content view, which is the `NSThemeFrame`, and install an OpenGL context
into it.  `NSThemeFrame` is a private class, but we only call public
`NSView` APIs on it here, so this seems OK in terms of being supported
by Apple going forward.

The reason for using this hack as opposed to
`NSFullSizeContentViewWindowMask` is that the latter forces the window
to be Core Animation-backed, which results in us rendering to an off
screen surface. Not only does this inject another compositor into the
system, but it also results in us rendering to an off-screen surface,
disabling the swap interval.

This depends on a `cocoa-rs` upgrade to add a binding to the `-[NSView
superview]` method.

Known issues:

* The traffic light buttons are not drawn but still function if you
  click on them. This can be worked around in browser.html.

* The top border is not rounded, although the shadow properly displays.
  This should be able to be worked around in browser.html.

* The title bar reappears if you go to full screen and then go back.
  This is the most serious issue, but I suspect it'll be fixable and
  it's better than what we have right now.

This should fix servo/servo#9431.
pcwalton added a commit to pcwalton/glutin that referenced this issue Feb 3, 2016
because that disables the swap interval.

This hack is very simple but a little evil. We get the superview of the
content view, which is the `NSThemeFrame`, and install an OpenGL context
into it.  `NSThemeFrame` is a private class, but we never speak its
name, we get to it with public APIs, and we only call public `NSView`
APIs on it, so this seems OK in terms of being supported by Apple going
forward.

Additionally, we remove the standard window buttons so that they
disappear and the user can't click them. This can also be done using
public APIs.

The reason for using this hack as opposed to
`NSFullSizeContentViewWindowMask` is that the latter forces the window
to be Core Animation-backed, which results in us rendering to an off
screen surface. Not only does this inject another compositor into the
system, but it also results in us rendering to an off-screen surface,
disabling the swap interval.

This depends on a `cocoa-rs` upgrade to add a binding to the `-[NSView
superview]` method.

Note that the top edge of the window is not rounded, although the shadow
is. Applications that wish to use this mode will need to round the top
edge themselves if they wish. (This is how Cocoa internally works:
rounding is done by the app, not the window manager.)

This should fix servo/servo#9431.
@pcwalton
Copy link
Contributor

pcwalton commented Feb 3, 2016

I've got a proposed fix for this in servo/glutin#68. It uses only public APIs and is only ~30 lines of code.

bors-servo pushed a commit to servo/glutin that referenced this issue Feb 26, 2016
Stop using `NSFullSizeContentViewWindowMask` to get borderless windows,

because that disables the swap interval.

This hack is very simple but a little evil. We get the superview of the
content view, which is the `NSThemeFrame`, and install an OpenGL context
into it.  `NSThemeFrame` is a private class, but we only call public
`NSView` APIs on it here, so this seems OK in terms of being supported
by Apple going forward.

The reason for using this hack as opposed to
`NSFullSizeContentViewWindowMask` is that the latter forces the window
to be Core Animation-backed, which results in us rendering to an off
screen surface. Not only does this inject another compositor into the
system, but it also results in us rendering to an off-screen surface,
disabling the swap interval.

This depends on a `cocoa-rs` upgrade to add a binding to the `-[NSView
superview]` method.

Known issues:

* The traffic light buttons are not drawn but still function if you
  click on them. This can be worked around in browser.html.

* The top border is not rounded, although the shadow properly displays.
  This should be able to be worked around in browser.html.

* The title bar reappears if you go to full screen and then go back.
  This is the most serious issue, but I suspect it'll be fixable and
  it's better than what we have right now.

This should fix servo/servo#9431.

r? @jdm

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/glutin/68)
<!-- Reviewable:end -->
pcwalton added a commit to pcwalton/glutin that referenced this issue Feb 26, 2016
because that disables the swap interval.

This hack is very simple but a little evil. We get the superview of the
content view, which is the `NSThemeFrame`, and install an OpenGL context
into it.  `NSThemeFrame` is a private class, but we never speak its
name, we get to it with public APIs, and we only call public `NSView`
APIs on it, so this seems OK in terms of being supported by Apple going
forward.

Additionally, we remove the standard window buttons so that they
disappear and the user can't click them. This can also be done using
public APIs.

The reason for using this hack as opposed to
`NSFullSizeContentViewWindowMask` is that the latter forces the window
to be Core Animation-backed, which results in us rendering to an off
screen surface. Not only does this inject another compositor into the
system, but it also results in us rendering to an off-screen surface,
disabling the swap interval.

This depends on a `cocoa-rs` upgrade to add a binding to the `-[NSView
superview]` method.

Note that the top edge of the window is not rounded, although the shadow
is. Applications that wish to use this mode will need to round the top
edge themselves if they wish. (This is how Cocoa internally works:
rounding is done by the app, not the window manager.)

This should fix servo/servo#9431.
pcwalton added a commit to pcwalton/glutin that referenced this issue Feb 26, 2016
because that disables the swap interval.

This hack is very simple but a little evil. We get the superview of the
content view, which is the `NSThemeFrame`, and install an OpenGL context
into it.  `NSThemeFrame` is a private class, but we never speak its
name, we get to it with public APIs, and we only call public `NSView`
APIs on it, so this seems OK in terms of being supported by Apple going
forward.

Additionally, we remove the standard window buttons so that they
disappear and the user can't click them. This can also be done using
public APIs.

The reason for using this hack as opposed to
`NSFullSizeContentViewWindowMask` is that the latter forces the window
to be Core Animation-backed, which results in us rendering to an off
screen surface. Not only does this inject another compositor into the
system, but it also results in us rendering to an off-screen surface,
disabling the swap interval.

This depends on a `cocoa-rs` upgrade to add a binding to the `-[NSView
superview]` method.

Note that the top edge of the window is not rounded, although the shadow
is. Applications that wish to use this mode will need to round the top
edge themselves if they wish. (This is how Cocoa internally works:
rounding is done by the app, not the window manager.)

This should fix servo/servo#9431.
emilio pushed a commit to emilio/glutin that referenced this issue Jul 6, 2016
because that disables the swap interval.

This hack is very simple but a little evil. We get the superview of the
content view, which is the `NSThemeFrame`, and install an OpenGL context
into it.  `NSThemeFrame` is a private class, but we never speak its
name, we get to it with public APIs, and we only call public `NSView`
APIs on it, so this seems OK in terms of being supported by Apple going
forward.

Additionally, we remove the standard window buttons so that they
disappear and the user can't click them. This can also be done using
public APIs.

The reason for using this hack as opposed to
`NSFullSizeContentViewWindowMask` is that the latter forces the window
to be Core Animation-backed, which results in us rendering to an off
screen surface. Not only does this inject another compositor into the
system, but it also results in us rendering to an off-screen surface,
disabling the swap interval.

This depends on a `cocoa-rs` upgrade to add a binding to the `-[NSView
superview]` method.

Note that the top edge of the window is not rounded, although the shadow
is. Applications that wish to use this mode will need to round the top
edge themselves if they wish. (This is how Cocoa internally works:
rounding is done by the app, not the window manager.)

This should fix servo/servo#9431.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment