New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 11 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2014
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug

Blocked on:
issue 245900
issue 314190



Sign in to add a comment
link

Issue 312462: Investigate using CARemoteLayer for Mac compositing

Reported by ccameron@chromium.org, Oct 28 2013 Project Member

Issue description

Our current architecture has the renderer draw to an IOSurface, then draw that to the screen. This causes a  minimum-factor-of-two overdraw per frame, which may avoided by using the CARemoteLayer API.

I've attached a self-contained example program that I used to familiarize myself with the API.

The hardest part of this from an architectural perspective is that we will need all drawing that we want to appear in the view (so, everything that we currently draw to the IOSurface) to be drawn in the [CAOpenGLLayer drawInCGLContext] function. This function is not called directly by us, but rather in the event loop in response to [CALayer setNeedsDisplay].

On way of doing this (suitable for prototyping) is to bracket the compositing draw calls in something like glBeginCompositeCHROMIUM/glEndCompositeCHROMIUM. The effect of these calls would be to 
- stall execution of the command buffer starting at glBeginCompositeCHROMIUM
- ensure that we don't run out of command buffer space between the begin and end (how?)
- call [CALayer setNeedsDisplay] when we hit glEndCompositeCHROMIUM
- execute the rest of the command buffer inside [CAOpenGLLayer drawInCGLContext]
- un-stall execution of the command buffer after [CAOpenGLLayer drawInCGLContext]

There are roughly equivalent ways of doing this without an explicit bracket (for instance, having the "begin" be drawing anything to framebuffer 0, and then "end" be the swap).

Another mechanism may be to add a GPU process compositor that outputs to real the real GL interface instead of the command buffer. The are probably yet-other mechanisms.
 
CARemoteLayerExample.mm
6.5 KB View Download

Comment 1 by thakis@chromium.org, Oct 28 2013

Nice!

(since I thought I saw this in emails before but a search for "CARemoteLayer" came up empty: keywords CARemoteLayerServer / CARemoteLayerClient)

Comment 2 by piman@chromium.org, Oct 28 2013

Cc: danakj@chromium.org
This is similar in concept to the Android WebView concept (draw functor), with roughly the same problems, but made worse because Mac supports more things.

Some things in the renderer currently need to glFinish() (or equivalent), think pepper or webgl or horrible things like printing an accelerated canvas. What happens in that case if we're within the bracket (on potentially a different context, but they have to run in-order)?

One possibility I'd like to investigate if we want to play with this is to have something along the lines of:
- make the renderer produce quads (delegating renderer), on its own schedule
- have the browser create a cc instance, or at least a GLRenderer to render the quads

Since the GLRenderer would run in the browser, we control it 100% and it's easier to bracket, because we can ensure we don't do finishing commands. It doesn't directly solve the out-of-command-buffer-space potential (though it's easier to e.g. pre-allocate the CB).

The side benefit is that it makes the renderer on Mac similar to Aura, without forcing Aura to the UI.


Other questions:
- do we know that drawInCGLContext is 1:1 with setNeedsDisplay? Can't the compositor send us more invalidates (e.g. because of damage caused by other layers)? Can't the compositor cull/delay the draw if the layer is invisible?
- can we use the context outside of drawInCGLContext? How much control do we have on that context, is it just given to us? Can we make it part of the share group?

Comment 3 by ccameron@chromium.org, Oct 28 2013

> - do we know that drawInCGLContext is 1:1 with setNeedsDisplay? Can't the
> compositor send us more invalidates (e.g. because of damage caused by other
> layers)? Can't the compositor cull/delay the draw if the layer is invisible?

It does appear to be closer to 1:1 than ordinary CALayers (understandably), but I don't have confidence in this.

This property impacts some architectural decisions (see later).

> - can we use the context outside of drawInCGLContext? How much control
> do we have on that context, is it just given to us? Can we make it part of the
> share group?

We have pretty much full control over the context -- we can even create a context on our own, and then tell CA to use it. IIUC we aren't allowed to change the context (though we can do the moral equivalent by creating a new layer with a different context and swapping them out).

> One possibility I'd like to investigate if we want to play with this is to have something along the lines of:
> - make the renderer produce quads (delegating renderer), on its own schedule
> - have the browser create a cc instance, or at least a GLRenderer to render the quads

IMO this is the right way to do things, with the slight change that we may want the cc instance to be running in the GPU process instead of the browser process. In particular, we'll want cc in the GPU process if we can be told to draw at any time. If the setNeedsDisplay:draw calls are actually-actually-actually to 1:1 (as you asked earlier), then having cc in the browser process is probably the way to go.

I think that the "bracketing" scheme may be the fastest way to get "sort-of-working" to further evaluate this.

Comment 4 by ccameron@chromium.org, Oct 28 2013

> I think that the "bracketing" scheme may be the fastest way to get "sort-of-working" to further evaluate this.

Not to check in, though (!)

Comment 5 by piman@chromium.org, Oct 29 2013

> we may want the cc instance to be running in the GPU process

I think that would need a lot more architectural pieces to be able to run (parts of) cc from the GPU process while sharing the contexts etc. Possible, but non-trivial.

Comment 6 by vangelis@chromium.org, Oct 29 2013

> One possibility I'd like to investigate if we want to play with this is to have something along the lines of:
> - make the renderer produce quads (delegating renderer), on its own schedule
> - have the browser create a cc instance, or at least a GLRenderer to render the quads

We could presumably do this today without using the new CA API? Essentially we would be replacing the part of the RWH that draws the IO surface with a gl_renderer that knows how to handle the quads that we got from the Renderer process. That would save us the extra blit.

Comment 7 by pi...@piman.org, Oct 29 2013

@#6: we don't share textures between the GPU process and the browser process. Quads coming from the renderer reference textures from the GPU process. So the GLRenderer in the browser process would have to use command buffers (like in aura) to access the textures referenced by the quads. That implies it renders to an IO surface like we do today (i.e. would not save a blit), or that we use CARemoteLayer as proposed.

That said, I think there is value in doing this either way from the architecture pov.

Comment 8 by vangelis@chromium.org, Oct 29 2013

Could we back the quads with IOsurfaces ?

Comment 9 by ccameron@chromium.org, Oct 29 2013

@#8, that's an option. That said, we've seen issues where we had to do bizarre workarounds to make sure IOSurfaces weren't leaked, etc. Leaning very heavily on that path may not be the way to go -- the CARemoteLayer seems to be the platform's preferred mechanism.

Comment 10 by vangelis@chromium.org, Oct 29 2013

Chris, what versions of OSX is CARemoteLayer supported in? I couldn't find any docs for it...

Comment 11 by ccameron@chromium.org, Oct 29 2013

My understanding is that it's 10.7+. So we'd need the IOSurface path for 10.6.

Comment 12 by piman@chromium.org, Oct 29 2013

@#8: not while keeping the full generality. My understanding IOSurfaces can only be put into TEXTURE_RECTANGLE_ARB, which don't support wrapping or filtering. It would result in a complete duplication of a bunch of code in cc (different shaders, different support code), for Mac-specific things.

Comment 13 by ccameron@chromium.org, Oct 30 2013

I played around with the example a bit and found that indeed the setNeedsDisplay/drawInContext calls are almost 1:1. The deviations are that
- there is an immediate drawInContext after sharing the layer (so that can be treated like an implicit setNeedsDisplay on creation)
- repeated calls to setNeedsDisplay don't get queued up (so if you call it N times in a row without an intervening drawInContext, you'll only get 1 drawInContext call)
I've attached the test app I used to verify this -- it lets you add/remove the layer from the view by pressing L and request new frames with F. So you do get the drawInContext calls even if you're not attached to the view hierarchy.

This means that we can just use a delegated renderer in the browser, like everyone else.

I also tested this on 10.7, and it works there. It doesn't compile on 10.6 (didn't try any trickery to make it materialize).

After some discussion, it looks like the way to go is to
1. Enable delegated rendering using the IOSurface path (this will have just 1 IOSurface per window, instead of 1 per tab)
2. Use the CARemoteLayer API where available to avoid the extra blit
CARemoteLayerExample.mm
8.9 KB View Download

Comment 14 by danakj@chromium.org, Oct 30 2013

Labels: Cr-Internals-Compositing-Ubercomp

Comment 15 by ccameron@chromium.org, Nov 1 2013

Blockedon: chromium:314190

Comment 16 by ccameron@chromium.org, Nov 1 2013

Blockedon: chromium:245900

Comment 17 by ccameron@chromium.org, Nov 7 2013

The CARemoteLayerExample.mm is broken by upgrading to 10.9. If compiled against the 10.8 SDK it runs correctly in 10.9, so it shouldn't be too hard to fix up.

Comment 18 by ccameron@google.com, Nov 7 2013

Attaching a very simplified example that shows the break with the 10.9 SDK. The example is single-process, so it's less likely to be an issue with the mach port (though that can't be rule out).

The headers of Cocoa and Quartz have basically no differences in the SDKs. Beyond that, I haven't looked very deeply into what has changed between the SDK versions.

Comment 19 by ccameron@chromium.org, Nov 7 2013

Attaching the file for real this time.
CARemoteLayerBrokenOnMavericks.mm
3.1 KB Download

Comment 20 by wiltzius@chromium.org, Nov 12 2013

Chris is your conclusion this is a Mavericks bug or just a change in the
API that you haven't accounted for? We can attempt to file a bug with Apple.

Comment 21 by a...@chromium.org, Nov 12 2013

There are times when Apple changes the runtime behavior of code when you compile it against different SDKs. This might be the case where, when you compile against the 10.9 SDK rather than the 10.8 SDK, it enforces a more "strict" or "correct" behavior on you and disallows you from "getting away" with something you used to do wrong. Is there anything in the console?

Comment 22 by a...@chromium.org, Nov 12 2013

There are a few suspicious times. If you breakpoint on '_CFExecutableLinkedOnOrAfter' you hit at:

    frame #0: 0x00007fff8481e890 CoreFoundation`_CFExecutableLinkedOnOrAfter
    frame #1: 0x00007fff89076513 QuartzCore`CA::Context::connect_remote() + 219
    frame #2: 0x00007fff890f16c3 QuartzCore`-[CAContextImpl initRemoteWithOptions:] + 49
    frame #3: 0x00007fff890f1644 QuartzCore`+[CAContext remoteContextWithOptions:] + 48
    frame #4: 0x00007fff891135d6 QuartzCore`-[CARemoteLayerClient initWithServerPort:] + 148

    frame #0: 0x00007fff8481e890 CoreFoundation`_CFExecutableLinkedOnOrAfter
    frame #1: 0x00007fff88fc0235 QuartzCore`CA::Context::connect_local() + 59
    frame #2: 0x00007fff88fbff1a QuartzCore`-[CAContextImpl initWithOptions:localContext:] + 144
    frame #3: 0x00007fff88fbfe71 QuartzCore`+[CAContext localContextWithOptions:] + 53
    frame #4: 0x00007fff88fc38d5 QuartzCore`view_ensure_context(_CAView*) + 418
    frame #5: 0x00007fff88fc32cc QuartzCore`CAViewUpdate + 3384
    frame #6: 0x00007fff9033f99c AppKit`-[NSView(NSInternal) _createLayerTreeRenderer] + 474
    frame #7: 0x00007fff9033cbd1 AppKit`-[NSView(NSLayerKitGlue) _setUpLayerTreeRendererAndSurface] + 86
    frame #8: 0x00007fff9027261c AppKit`-[NSView _setUpLayerTreeRendererAndSurfaceIfNecessary] + 56
    frame #9: 0x00007fff902d14c0 AppKit`-[NSView _doSetWantsLayerYES] + 408
    frame #10: 0x00007fff902d0ffc AppKit`-[NSView setWantsLayer:] + 310

My suspicion is that this is intentional on Apple's part, that we're doing something "wrong", and we should make sure it works when compiled against the 10.9 SDK. I haven't the slightest idea of what that "wrong" thing is.

Someone should write a script that runs the two different binaries in GDB and breaks when their execution diverges... :)

Comment 23 by ccameron@chromium.org, Nov 12 2013

Yeah, I suspect it's intentional -- I'll follow up with them. I get the following in the console, but I get it even when I'm not using the remote layer API and when it works (so it's almost certainly unrelated)
    11/12/13 12:15:53.079 PM Terminal[5857]: CGSCopyDisplayUUID: Invalid display 0x3c0a5501
    11/12/13 12:15:56.492 PM taskgated[17]: no system signature for unsigned /Users/ccameron/Downloads/CARemoteLayerBrokenOnMavericks[73492]

Comment 24 by ccameron@chromium.org, Dec 2 2013

Attaching a version of the app that tries to add instrumentation with NSLogUnusualAppConfig. I didn't get any output in Console, but I may be doing it wrong.
CARemoteLayer-link-instrumentation.tar.gz
2.4 KB Download

Comment 25 by a...@chromium.org, Dec 2 2013

Re comment 24:

- Your scripts don't make sense. Why are you dumping the executable in a pseudo-bundle? Your compile instructions at the top of the file work fine.
- clientId should be uint32_t, not a uint64.

Here's my (slightly cleaned) terminal output:

> g++ CARemoteLayerBrokenOnMavericks.mm -o CARemoteLayerBrokenOnMavericks8 -framework Cocoa -framework QuartzCore -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.8.sdk
> g++ CARemoteLayerBrokenOnMavericks.mm -o CARemoteLayerBrokenOnMavericks9 -framework Cocoa -framework QuartzCore -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.9.sdk
> ./CARemoteLayerBrokenOnMavericks8 -NSLogUnusualAppConfig YES
CARemoteLayerBrokenOnMavericks8[30892:507] NSLogUnusualAppConfig=YES
CARemoteLayerBrokenOnMavericks8[30892:507] NSWindowHostsLayersInWindowServer=NO
CARemoteLayerBrokenOnMavericks8[30892:507] NSSpacePerDisplay=YES
CARemoteLayerBrokenOnMavericks8[30892:507] NSViewLinkedOnZin=YES
CARemoteLayerBrokenOnMavericks8[30892:507] NSViewLayerBackWindowFrame=NO
CARemoteLayerBrokenOnMavericks8[30892:507] NSWindowAdjustSecondaryScreenFillingWindows=YES
CARemoteLayerBrokenOnMavericks8[30892:507] If you see green, it worked. If you see red, it's broken.
> ./CARemoteLayerBrokenOnMavericks9 -NSLogUnusualAppConfig YES
CARemoteLayerBrokenOnMavericks9[30897:507] NSLogUnusualAppConfig=YES
CARemoteLayerBrokenOnMavericks9[30897:507] NSSpacePerDisplay=YES
CARemoteLayerBrokenOnMavericks9[30897:507] NSViewLinkedOnZin=YES
CARemoteLayerBrokenOnMavericks9[30897:507] If you see green, it worked. If you see red, it's broken.

What's different that looks suspicious? NSWindowHostsLayersInWindowServer and NSViewLayerBackWindowFrame. Do either of those trigger any aha?

Comment 26 by ccameron@chromium.org, Dec 2 2013

The NSWindowHostsLayersInWindowServer sounds relevant, and this patch "Remotely hosted plugins don’t work in window-server-hosted WebKit1 views" looks like it may be solving the same problem.

https://bug-119709-attachments.webkit.org/attachment.cgi?id=208667

Of note is that [NSWindow _hostsLayersInWindowServer] is false when linking against 10.8's SDK and true when linking against 10.9's SDK.

A guess is that the CALayer which the GPU process is creating is actually being "hosted" by the window server, so asking for it from the GPU process' CARemoteLayerServer is asking from the wrong place. Maybe we can force that the CALayer we create in the GPU process be hosted in the "old place".

Comment 27 by a...@chromium.org, Dec 2 2013

You're out of my area of expertise again, so I can't say. It's weird that you'd have to go through so much trouble to make it work; why wouldn't a straightforward use of the API be correct enough to work?

Comment 28 by ccameron@chromium.org, Dec 2 2013

Okay, sorted out the 10.9 issue (with lots of help from avi). I'll still try to get some confirmation from Apple that the story that we've strung together based on variable names is correct and supported.

It looks like the CALayers that we share out need to be hosted by the app instead of the window server (otherwise the app can't share them out). The API fails silently if this isn't done.

Running any of the above examples with "-NSWindowHostsLayersInWindowServer NO" will fix the issue.

That argument is only needed by the GPU process. I've attached another version of the two-process CARemoteLayerExample.mm which dynamically adds the setting to the GPU process (server process), but not the client process, and everything works fine.

Comment 29 by ccameron@chromium.org, Dec 2 2013

CARemoteLayerExample.mm
9.1 KB Download

Comment 30 by a...@chromium.org, Dec 3 2013

I don't know which is worse: fiddling with the private API [NSWindow _hostsLayersInWindowServer] via the call _WKPHPluginShouldHostLayersInWindowServerChanged (which I haven't disassembled yet), or using the private setting NSWindowHostsLayersInWindowServer.

Yes, I would like to see what Apple says. Why is this so hard?

Comment 31 by ccameron@chromium.org, Dec 3 2013

This API is a bit tough because it's living somewhere in the world between API and SPI. It went from SPI to API with 10.7, but it hasn't been documented on developer.apple.com -- it just appeared in the headers.

Comment 32 by ccameron@chromium.org, Dec 17 2013

Btw, the Apple bug for the the usage of the flag is Radar 15567388.

Comment 33 by a...@chromium.org, Jan 7 2014

Today I became aware of https://developer.apple.com/library/mac/documentation/Cocoa/Reference/ApplicationKit/Classes/NSView_Class/Reference/NSView.html#//apple_ref/occ/instm/NSView/setLayerUsesCoreImageFilters: :

> Sets whether the view’s layer uses Core Image filters and should therefore be rendered in process.
> - (void)setLayerUsesCoreImageFilters:(BOOL)usesFilters

and the interesting part:

> In OS X v10.9 and later, AppKit prefers to render layer trees out-of-process but cannot do so if any layers have Core Image filters attached to them. Specifying YES for the usesFilters parameter lets AppKit know that this view has Core Image filters and that AppKit should move rendering of the layer hierarchy back into your app’s process.

If you can make this work, this would probably be a better option than the NSUserDefaults fiddling you were doing in your last code sample.

Comment 34 by a...@chromium.org, Jan 8 2014

Have you tried your code in comment 29 lately? It seems to work for me when linked with 10.9, even without the line flipping NSWindowHostsLayersInWindowServer.

I can still repro the problem with the code from comment 19. It turns out to be fairly trivial to fix with setLayerUsesCoreImageFilters.
CARemoteLayerBrokenOnMavericks.mm
3.9 KB Download

Comment 35 by ccameron@chromium.org, Jan 8 2014

Yes -- that attachment works for me. That seems like a much better way to solve the problem, and more than borderline-supported (btw, I haven't heard back on the radars that I filed).

Comment 36 by ccameron@chromium.org, Jan 23 2014

Btw, back in  issue 245900 , we saw that setLayerUsesCoreImageFilters:YES and NSWindowHostsLayersInWindowServer:NO cause pretty terrible performance in 10.9. In order to use CoreAnimation at all, it is necessary that we set NSWindowHostsLayersInWindowServer:NO.

Looking at the WebKit implementation, it may be that CARemoteLayerServer/Client is the old way of doing this, and sharing the layer via the window server as an intermediary is the new way. I base this on

std::unique_ptr<LayerHostingContext> LayerHostingContext::createForPort(mach_port_t serverPort) ...
    layerHostingContext->m_layerHostingMode = LayerHostingModeDefault;
    layerHostingContext->m_context = WKCAContextMakeRemoteWithServerPort(serverPort);

...
#if HAVE(LAYER_HOSTING_IN_WINDOW_SERVER)
std::unique_ptr<LayerHostingContext> LayerHostingContext::createForWindowServer() ...
    auto layerHostingContext = std::make_unique<LayerHostingContext>();
    layerHostingContext->m_layerHostingMode = LayerHostingModeInWindowServer;
    layerHostingContext->m_context = WKCAContextMakeRemoteForWindowServer();

That's where the trail goes cold, though.

Comment 37 by a...@chromium.org, Jan 23 2014

That's not a cold trail. Dig in with GDB into the WK* calls.

We need to know what's really going on behind the scenes to make the call as to whether we want to go this route or not.

Comment 38 by ccameron@chromium.org, Jan 29 2014

I've filed Radar 15896506 with a more-detailed description of the CARemoteLayer issues.

Comment 39 by dxie@google.com, Feb 6 2014

Status: Assigned

Comment 40 by ccameron@chromium.org, Apr 18 2014

This has been sorted out on Mavericks. Attaching a simple application (one solid-color layer) and a representative (10 OpenGL windows). No performance issues founds so far.

I'm debating whether to do this before or after ÜC ( issue 314190 ). I think that this will have to wait until after ÜC because the tab capture code won't work with a CALayer (it needs the IOSurface).
CARemoteLayerOpenGL.mm
7.0 KB Download
CARemoteLayerMavericks.mm
5.6 KB Download

Comment 41 by kbr@chromium.org, Apr 19 2014

Cc: hubbe@chromium.org
+hubbe for potential comment

Depending on when the ubercompositor is expected to work, I wonder whether some intermediate solution could be found. Maybe CARemoteLayer could be used except when tab capture is actively being used, or maybe the tab capture implementation could be changed to grab the tab's contents before they're irrevocably given to Core Animation to render.

Comment 42 by hubbe@chromium.org, Apr 21 2014

Cc: hclam@chromium.org m...@chromium.org
Adding the people who actually know something about tab capture on mac.

Comment 43 by danakj@chromium.org, Apr 22 2014

> except when tab capture is actively being used

I think that would provide little benefit to switching, as then we'd still have to maintain the old codepath. I'd prefer going in a direction that doesn't require maintaining two display paths. If we can do this without ubercomp and accomplish that, it would be awesome as it would allow us to remove a lot more code from the browser/RenderView/etc stuff.

Comment 44 by ccameron@chromium.org, May 8 2014

I've done a few more experiments with this. I can get the browser to share a window with itself ... for some reason ti requires an extra retain call to the CAContext created with contextWithCGSConnection.

I haven't been able to get the GPU process and the browser process to share yet. I'm going to try having them share with a third party separately, to see where things are breaking down.

Comment 45 by ccameron@chromium.org, May 8 2014

Looks like we need to make the message loop in the GPU process be TYPE_UI (that is, it bottoms out to a NS message loop) for this to work (or, at least, we need to pull a lot of stuff in for that).

avi, thakis, rsesek, (others): Is this something we don't want to do?

With that hooked up, I can get CALayers in the GPU process to appear in the browser process. I may roll this out earlier than expected in order to help out with  issue 364808 , and to kick the tires. That said, rollout will be in two phases
1. Where we still do an extra copy -- we just use a GL texture in the GPU process instead of an IOSurface
2. Where we cut out that copy (we'll need to do some creative things to make this work ... nothing insurmountable ...).

Comment 46 by a...@chromium.org, May 8 2014

Is there any way around this? Can we get away with it being an NSRunLoop rather than the NSApplication loop?

Robert just spent a lot of time ripping the TYPE_UI MessageLoop out of the renderer. Using one for the GPU process would be a step backwards for security.

Comment 47 by ccameron@chromium.org, May 9 2014

Oh -- there very well may be a way to do this without the full [NSApp run] -- I just stopped debugging at the point where I found what was making the difference between the test programs and Chrome. I'll take a look into using NSRunLoop instead.

Comment 48 by a...@chromium.org, May 9 2014

Definitely spend some quality time with Robert discussing this. He is our new expert :)

Comment 49 by ccameron@chromium.org, May 9 2014

Sounds like a plan. As notes for the discussion, [[NSRunLoop currentRunLoop] run] seems to do it. Also, just a while(1) doing [NSApp nextEvent] and [NSApp sendEvent] seems to do it. So, whichever is better.

Comment 50 by a...@chromium.org, May 9 2014

Does whatever MessagePumpNSRunLoop does work? See r254175.

Comment 51 by rsesek@chromium.org, May 9 2014

For context, on OS X there are four options for the main thread MessagePump:

1. Default, which is nothing more than a condition variable and a MessageLoop. This doesn't process system work at all, so it is not a viable option here.
2. CFRunLoop, which is the lowest-level system loop. It can process system events from Mach ports, CFTimers, and CFRunLoopSources. 
3. NSRunLoop, which handles all of CFRunLoop's work as well as NSTimers and NSObject delayed-performs.
4. NSApplication, which is a NSRunLoop with a lot of extra machinery for actually consuming and processing CGEvents from the window server, processing of Apple Events, and handling connections to the TSM for advanced text input.

It would be very unfortunate to use (4) because it connects to several Mach servers besides just the window server (which needs to be accessible to the GPU process anyways). Using a CFRunLoop or NSRunLoop message pump would be best. You may be able to get away with (2) if nothing requires NSTimer or -[NSObject performSelector:afterDelay:withObject:]. This was not the case in the renderers (scrollbars use NSTimer), so that's why renderer_main.cc allocates (3).

Can you try both MessagePumpCFRunLoop and MessagePumpNSRunLoop() like done here: https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/renderer_main.cc&q=renderer_main.cc&sq=package:chromium&type=cs&l=161 and see which ones work?

Comment 52 by ccameron@chromium.org, May 9 2014

Awesome -- thanks!!

I tried the 4 of them, and CFRunLoop looks like it works (as does NSRunLoop, and NSApplication). I didn't kick the tires enough to make sure it's stable, but I'll start with the CFRunLoop.

Comment 53 by bugdroid1@chromium.org, May 14 2014

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/826aab00c6fdd9435e8cc65823e15df4d4cb9e38

commit 826aab00c6fdd9435e8cc65823e15df4d4cb9e38
Author: ccameron@chromium.org <ccameron@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Wed May 14 02:58:59 2014

Change the GPU process to use a CFRunLoop

This is necessary for CAContext sharing (determined empirically, as
that API is undocumented).

BUG= 312462 

Review URL: https://codereview.chromium.org/279163005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@270309 0039d316-1c4b-4281-b951-d872f2087c98

Comment 54 by bugdroid1@chromium.org, May 14 2014

Project Member
------------------------------------------------------------------
r270309 | ccameron@chromium.org | 2014-05-14T02:58:59.324044Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/base/message_loop/message_pump_mac.h?r1=270309&r2=270308&pathrev=270309
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/gpu/gpu_main.cc?r1=270309&r2=270308&pathrev=270309

Change the GPU process to use a CFRunLoop

This is necessary for CAContext sharing (determined empirically, as
that API is undocumented).

BUG= 312462 

Review URL: https://codereview.chromium.org/279163005
-----------------------------------------------------------------

Comment 55 by ccameron@chromium.org, Jun 17 2014

This is "working" end-to-end with the following patch: https://codereview.chromium.org/337303003

I'm going to introduce this in two steps -- first rendering to an FBO sharing most of the code with the IOSurface path (this version won't save us the extra blit) -- after that we can try to avoid the extra blit. The above patch uses the share-everything-with-IOSurface scheme.

Comment 56 by bugdroid1@chromium.org, Jun 18 2014

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a187cf51deb0f5c8dd345d8ee3352adc6e36711f

commit a187cf51deb0f5c8dd345d8ee3352adc6e36711f
Author: ccameron@chromium.org <ccameron@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Wed Jun 18 07:30:37 2014

Clean up GLSurfaceCGL

This is in preparation for support for using remote CALayers.

The big change is to separate out ImageTransportSurfaceIOSurface into
ImageTransportSurfaceFBO and its StorageProvider. The IOSurface-
specific bits of ImageTransportSurfaceIOSurface are moved into the
StorageProvider. A separate CALayer-specific version will be added
later.

While in the neighborhood, don't make everything inherit from
GLSurfaceCGL (that class doesn't actually do anything). Move the helper
functions and classes from its file to more appropriate locations.

Make image_transport_surface_mac.cc a .mm file, because it will need
to include Objective C code in the near future. Move the
ImageTransportSurfaceFBO into its own file.

Also make the IOSurface handles in renderer_host use the 32-bit
IOSurfaceID handle type instead of uint64, since that is the type
actually used by the interface, and those other bits may find use in
disambiguating handle types.

BUG= 312462 

Review URL: https://codereview.chromium.org/334173006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@277970 0039d316-1c4b-4281-b951-d872f2087c98

Comment 57 by bugdroid1@chromium.org, Jun 18 2014

Project Member
------------------------------------------------------------------
r277970 | ccameron@chromium.org | 2014-06-18T07:30:37.744058Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/compositor/browser_compositor_view_mac.mm?r1=277970&r2=277969&pathrev=277970
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/render_widget_host_view_mac.h?r1=277970&r2=277969&pathrev=277970
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/gl/gl.gyp?r1=277970&r2=277969&pathrev=277970
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/gl/BUILD.gn?r1=277970&r2=277969&pathrev=277970
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/compositing_iosurface_mac.mm?r1=277970&r2=277969&pathrev=277970
   A http://src.chromium.org/viewvc/chrome/trunk/src/content/common/gpu/image_transport_surface_mac.mm?r1=277970&r2=277969&pathrev=277970
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/compositor/browser_compositor_view_mac.h?r1=277970&r2=277969&pathrev=277970
   A http://src.chromium.org/viewvc/chrome/trunk/src/content/common/gpu/image_transport_surface_fbo_mac.cc?r1=277970&r2=277969&pathrev=277970
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/gl/gl_surface_mac.cc?r1=277970&r2=277969&pathrev=277970
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/gl/gl_context_cgl.cc?r1=277970&r2=277969&pathrev=277970
   A http://src.chromium.org/viewvc/chrome/trunk/src/content/common/gpu/image_transport_surface_fbo_mac.h?r1=277970&r2=277969&pathrev=277970
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/render_widget_host_view_mac.mm?r1=277970&r2=277969&pathrev=277970
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/compositing_iosurface_mac.h?r1=277970&r2=277969&pathrev=277970
   D http://src.chromium.org/viewvc/chrome/trunk/src/ui/gl/gl_surface_cgl.cc?r1=277970&r2=277969&pathrev=277970
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/content_common.gypi?r1=277970&r2=277969&pathrev=277970
   D http://src.chromium.org/viewvc/chrome/trunk/src/ui/gl/gl_surface_cgl.h?r1=277970&r2=277969&pathrev=277970
   D http://src.chromium.org/viewvc/chrome/trunk/src/content/common/gpu/image_transport_surface_mac.cc?r1=277970&r2=277969&pathrev=277970
   A http://src.chromium.org/viewvc/chrome/trunk/src/content/common/gpu/image_transport_surface_iosurface_mac.cc?r1=277970&r2=277969&pathrev=277970
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/render_widget_helper_mac.mm?r1=277970&r2=277969&pathrev=277970
   A http://src.chromium.org/viewvc/chrome/trunk/src/content/common/gpu/image_transport_surface_iosurface_mac.h?r1=277970&r2=277969&pathrev=277970

Clean up GLSurfaceCGL

This is in preparation for support for using remote CALayers.

The big change is to separate out ImageTransportSurfaceIOSurface into
ImageTransportSurfaceFBO and its StorageProvider. The IOSurface-
specific bits of ImageTransportSurfaceIOSurface are moved into the
StorageProvider. A separate CALayer-specific version will be added
later.

While in the neighborhood, don't make everything inherit from
GLSurfaceCGL (that class doesn't actually do anything). Move the helper
functions and classes from its file to more appropriate locations.

Make image_transport_surface_mac.cc a .mm file, because it will need
to include Objective C code in the near future. Move the
ImageTransportSurfaceFBO into its own file.

Also make the IOSurface handles in renderer_host use the 32-bit
IOSurfaceID handle type instead of uint64, since that is the type
actually used by the interface, and those other bits may find use in
disambiguating handle types.

BUG= 312462 

Review URL: https://codereview.chromium.org/334173006
-----------------------------------------------------------------

Comment 58 by bugdroid1@chromium.org, Jun 23 2014

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ba2a3366f2f3029913177d9e6526eb0181b36402

commit ba2a3366f2f3029913177d9e6526eb0181b36402
Author: ccameron@chromium.org <ccameron@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Mon Jun 23 22:26:47 2014

Make cross-process CALayers work on Mac

Add a header file for the remote layer API and a helper function to
determine if it is supported at runtime.

Add a mechanism for disambiguating if a "surface handle" refers to an
IOSurfaceID or a CAContextID (the thing that is passed across processes
to do cross-process CALayer drawing).

Add support in RenderWidgetHostViewMac to put in a CALayerHost from
another process's CAContextID. In the process of this, make the root
CALayer hanging off of the RenderWidgetHostViewMac have flipped
geometry, and update LayoutLayers to take this into account.

This is working surprisingly well, but needs a bit more work before it
can be turned on. We still need to:
* exert GPU back-pressure from the GPU process'
  -[ImageTransportLayer drawInCGLContext] function (it's an open loop
  now).
* use the isAsynchronous property to make 60fps animation not jerky.
* ensure that these resources' lifetimes are being managed reasonably
  and aren't eating tons of GPU
* ensure that not rounding texture sizes to the nearest 64 pixels (as is
  done for the IOSurface scheme) isn't causing fragmentation.

BUG= 312462 

Review URL: https://codereview.chromium.org/347653005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@279200 0039d316-1c4b-4281-b951-d872f2087c98

Comment 59 by bugdroid1@chromium.org, Jun 23 2014

Project Member
------------------------------------------------------------------
r279200 | ccameron@chromium.org | 2014-06-23T22:26:47.915120Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/common/gpu/image_transport_surface_fbo_mac.h?r1=279200&r2=279199&pathrev=279200
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/render_widget_host_view_mac.mm?r1=279200&r2=279199&pathrev=279200
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/content_common.gypi?r1=279200&r2=279199&pathrev=279200
   A http://src.chromium.org/viewvc/chrome/trunk/src/ui/base/cocoa/remote_layer_api.mm?r1=279200&r2=279199&pathrev=279200
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/common/gpu/image_transport_surface_iosurface_mac.cc?r1=279200&r2=279199&pathrev=279200
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/render_widget_helper_mac.mm?r1=279200&r2=279199&pathrev=279200
   A http://src.chromium.org/viewvc/chrome/trunk/src/content/common/gpu/image_transport_surface_calayer_mac.h?r1=279200&r2=279199&pathrev=279200
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/common/gpu/image_transport_surface_iosurface_mac.h?r1=279200&r2=279199&pathrev=279200
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/render_widget_host_view_mac.h?r1=279200&r2=279199&pathrev=279200
   A http://src.chromium.org/viewvc/chrome/trunk/src/ui/base/cocoa/remote_layer_api.h?r1=279200&r2=279199&pathrev=279200
   A http://src.chromium.org/viewvc/chrome/trunk/src/content/common/gpu/surface_handle_types_mac.cc?r1=279200&r2=279199&pathrev=279200
   A http://src.chromium.org/viewvc/chrome/trunk/src/content/common/gpu/surface_handle_types_mac.h?r1=279200&r2=279199&pathrev=279200
   A http://src.chromium.org/viewvc/chrome/trunk/src/content/common/gpu/image_transport_surface_calayer_mac.mm?r1=279200&r2=279199&pathrev=279200
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/common/gpu/image_transport_surface_mac.mm?r1=279200&r2=279199&pathrev=279200
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/base/ui_base.gyp?r1=279200&r2=279199&pathrev=279200
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/common/gpu/image_transport_surface_fbo_mac.cc?r1=279200&r2=279199&pathrev=279200

Make cross-process CALayers work on Mac

Add a header file for the remote layer API and a helper function to
determine if it is supported at runtime.

Add a mechanism for disambiguating if a "surface handle" refers to an
IOSurfaceID or a CAContextID (the thing that is passed across processes
to do cross-process CALayer drawing).

Add support in RenderWidgetHostViewMac to put in a CALayerHost from
another process's CAContextID. In the process of this, make the root
CALayer hanging off of the RenderWidgetHostViewMac have flipped
geometry, and update LayoutLayers to take this into account.

This is working surprisingly well, but needs a bit more work before it
can be turned on. We still need to:
* exert GPU back-pressure from the GPU process'
  -[ImageTransportLayer drawInCGLContext] function (it's an open loop
  now).
* use the isAsynchronous property to make 60fps animation not jerky.
* ensure that these resources' lifetimes are being managed reasonably
  and aren't eating tons of GPU
* ensure that not rounding texture sizes to the nearest 64 pixels (as is
  done for the IOSurface scheme) isn't causing fragmentation.

BUG= 312462 

Review URL: https://codereview.chromium.org/347653005
-----------------------------------------------------------------

Comment 60 by bugdroid1@chromium.org, Jun 24 2014

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/633eb8b4a43bd49550d25caba070601dcd16f59b

commit 633eb8b4a43bd49550d25caba070601dcd16f59b
Author: ccameron@chromium.org <ccameron@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Tue Jun 24 00:35:08 2014

Remove stray setAutoresizingMask call.

This turned out not to be necessary, and also turned out ot be causing
issues with tab dragging and CALayer reparenting.

BUG= 312462 
TBR=kbr@chromium.org
NOTRY=true

Review URL: https://codereview.chromium.org/347423004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@279247 0039d316-1c4b-4281-b951-d872f2087c98

Comment 61 by bugdroid1@chromium.org, Jun 24 2014

Project Member
------------------------------------------------------------------
r279247 | ccameron@chromium.org | 2014-06-24T00:35:08.377617Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/render_widget_host_view_mac.mm?r1=279247&r2=279246&pathrev=279247

Remove stray setAutoresizingMask call.

This turned out not to be necessary, and also turned out ot be causing
issues with tab dragging and CALayer reparenting.

BUG= 312462 
TBR=kbr@chromium.org
NOTRY=true

Review URL: https://codereview.chromium.org/347423004
-----------------------------------------------------------------

Comment 62 by bugdroid1@chromium.org, Aug 5 2014

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b64d44d87f89563abc520b0efa3826be96be5286

commit b64d44d87f89563abc520b0efa3826be96be5286
Author: ccameron@chromium.org <ccameron@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Tue Aug 05 09:40:31 2014

Make remote CALayers work with browser compositor on Mac

This adds the equivalent remote layer code from RenderWidgetHostViewMac
over to the BrowserCompositorViewMac.

This code still isn't activated yet -- backpressure is still not appropriately
applied via ImageTransportLayer.

BUG= 312462 

Review URL: https://codereview.chromium.org/441743003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@287502 0039d316-1c4b-4281-b951-d872f2087c98

Comment 63 by bugdroid1@chromium.org, Aug 5 2014

Project Member
------------------------------------------------------------------
r287502 | ccameron@chromium.org | 2014-08-05T09:40:31.877622Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/compositor/browser_compositor_view_mac.mm?r1=287502&r2=287501&pathrev=287502
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/compositor/browser_compositor_view_private_mac.h?r1=287502&r2=287501&pathrev=287502
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/compositor/browser_compositor_view_private_mac.mm?r1=287502&r2=287501&pathrev=287502

Make remote CALayers work with browser compositor on Mac

This adds the equivalent remote layer code from RenderWidgetHostViewMac
over to the BrowserCompositorViewMac.

This code still isn't activated yet -- backpressure is still not appropriately
applied via ImageTransportLayer.

BUG= 312462 

Review URL: https://codereview.chromium.org/441743003
-----------------------------------------------------------------

Comment 64 by bugdroid1@chromium.org, Aug 13 2014

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3da74d14e73e63f723d0d030783140d6ffd8e782

commit 3da74d14e73e63f723d0d030783140d6ffd8e782
Author: ccameron@chromium.org <ccameron@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Wed Aug 13 09:37:27 2014

Make GPU back-pressure work with remote CALayers

Prior to this change, ImageTransportSurfaceFBO had the property
that it would un-schedule the GPU channel at a swap, and then
re-schedule the GPU channel when the swap was acknowledged by
the browser process.

Separate out the re-scheduling of the channel into the function
ImageTransportSurfaceFBO::UnblockContextAfterPendingSwap.
Previously, this re-scheduling was done after receiving an ack in
the form of the AcceleratedSurfaceMsg_BufferPresented IPC.

Because the re-scheduling of the GPU channel is no longer blocked
on the AcceleratedSurfaceMsg_BufferPresented IPC, issue that
IPC from the UI thread in the browser when the SwapBuffers IPC
is processed (instead of doing so on the IO thread immediately).
Get rid of the hacks being used prevent the IOSurface from being
freed while the SwapBuffers IPC was bouncing from the IO thread to
the UI thread.

For IOSurface-based ImageTransportSurfaces, re-schedule the GPU
channel immediately, because the ui::Compositor in the browser
process is responsible for "feeling" the GPU back-pressure in its
CompositingIOSurfaceLayer. Prevent the IOSurface from being freed
while it is in-flight by keeping around an extra reference to all
in-flight IOSurfaces (the reference is taken at SwapBuffers and is
released at AcceleratedSurfaceMsg_BufferPresented).

For CAContext/CALayer-based ImageTransportSurfaces, re-schedule
the GPU channel when the ImageTransportLayer in the GPU process is
displayed (the back-pressure is "felt" within the same process).
Because the CAContext used for this ImageTransportSurface is static
for the lifetime of the ImageTransportSurface (unlike IOSurfaces
where re-allocation at resize is common), there is no need to keep
around references to in-flight surfaces.

BUG= 312462 
R=jbauman
TBR=kbr

Review URL: https://codereview.chromium.org/454243002

Cr-Commit-Position: refs/heads/master@{#289232}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@289232 0039d316-1c4b-4281-b951-d872f2087c98

Comment 65 by bugdroid1@chromium.org, Aug 13 2014

Project Member
------------------------------------------------------------------
r289232 | ccameron@chromium.org | 2014-08-13T09:37:27.872894Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/compositor/browser_compositor_view_private_mac.h?r1=289232&r2=289231&pathrev=289232
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/common/gpu/image_transport_surface_iosurface_mac.cc?r1=289232&r2=289231&pathrev=289232
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/compositor/browser_compositor_view_private_mac.mm?r1=289232&r2=289231&pathrev=289232
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/common/gpu/image_transport_surface_calayer_mac.h?r1=289232&r2=289231&pathrev=289232
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/common/gpu/image_transport_surface_iosurface_mac.h?r1=289232&r2=289231&pathrev=289232
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/common/gpu/image_transport_surface_calayer_mac.mm?r1=289232&r2=289231&pathrev=289232
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/common/gpu/image_transport_surface_mac.mm?r1=289232&r2=289231&pathrev=289232
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/compositor/browser_compositor_view_mac.h?r1=289232&r2=289231&pathrev=289232
   D http://src.chromium.org/viewvc/chrome/trunk/src/content/common/gpu/image_transport_surface_fbo_mac.cc?r1=289232&r2=289231&pathrev=289232
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/compositor/browser_compositor_view_mac.mm?r1=289232&r2=289231&pathrev=289232
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/gpu/gpu_process_host_ui_shim.cc?r1=289232&r2=289231&pathrev=289232
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/common/gpu/image_transport_surface_fbo_mac.h?r1=289232&r2=289231&pathrev=289232
   A http://src.chromium.org/viewvc/chrome/trunk/src/content/common/gpu/image_transport_surface_fbo_mac.mm?r1=289232&r2=289231&pathrev=289232
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/gpu/gpu_process_host.cc?r1=289232&r2=289231&pathrev=289232
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/content_common.gypi?r1=289232&r2=289231&pathrev=289232

Make GPU back-pressure work with remote CALayers

Prior to this change, ImageTransportSurfaceFBO had the property
that it would un-schedule the GPU channel at a swap, and then
re-schedule the GPU channel when the swap was acknowledged by
the browser process.

Separate out the re-scheduling of the channel into the function
ImageTransportSurfaceFBO::UnblockContextAfterPendingSwap.
Previously, this re-scheduling was done after receiving an ack in
the form of the AcceleratedSurfaceMsg_BufferPresented IPC.

Because the re-scheduling of the GPU channel is no longer blocked
on the AcceleratedSurfaceMsg_BufferPresented IPC, issue that
IPC from the UI thread in the browser when the SwapBuffers IPC
is processed (instead of doing so on the IO thread immediately).
Get rid of the hacks being used prevent the IOSurface from being
freed while the SwapBuffers IPC was bouncing from the IO thread to
the UI thread.

For IOSurface-based ImageTransportSurfaces, re-schedule the GPU
channel immediately, because the ui::Compositor in the browser
process is responsible for "feeling" the GPU back-pressure in its
CompositingIOSurfaceLayer. Prevent the IOSurface from being freed
while it is in-flight by keeping around an extra reference to all
in-flight IOSurfaces (the reference is taken at SwapBuffers and is
released at AcceleratedSurfaceMsg_BufferPresented).

For CAContext/CALayer-based ImageTransportSurfaces, re-schedule
the GPU channel when the ImageTransportLayer in the GPU process is
displayed (the back-pressure is "felt" within the same process).
Because the CAContext used for this ImageTransportSurface is static
for the lifetime of the ImageTransportSurface (unlike IOSurfaces
where re-allocation at resize is common), there is no need to keep
around references to in-flight surfaces.

BUG= 312462 
R=jbauman
TBR=kbr

Review URL: https://codereview.chromium.org/454243002
-----------------------------------------------------------------

Comment 66 by bugdroid1@chromium.org, Aug 15 2014

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5693a6cbe83b850db090971d8e77e8d2506e5a7c

commit 5693a6cbe83b850db090971d8e77e8d2506e5a7c
Author: ccameron@chromium.org <ccameron@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Fri Aug 15 09:41:13 2014

Add a flag to enable remote CoreAnimation API.

A follow-on patch will switch this flag to be enabled by default.

BUG= 312462 
TBR=kbr
R=sky

Review URL: https://codereview.chromium.org/464063005

Cr-Commit-Position: refs/heads/master@{#289835}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@289835 0039d316-1c4b-4281-b951-d872f2087c98

Comment 68 by ccameron@chromium.org, Aug 19 2014

This is available for anyone who wants to try it by using --enable-remote-core-animation.

I've been running with this flag on Canary all day. The only issue I've seen so far is flashing of stale frames when switching tabs when using the dGPU on the local retina display (with no external monitors). I think I have a fix for that.

I also want to clean up the CompositingIOSurfaceLayer (for 10.6-10.8), since most of its complexity isn't needed anymore.

Comment 69 by bugdroid1@chromium.org, Aug 28 2014

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/388cc928a95dc1a921d3b1671985046c3c626c22

commit 388cc928a95dc1a921d3b1671985046c3c626c22
Author: ccameron <ccameron@chromium.org>
Date: Thu Aug 28 02:09:14 2014

Fix assorted issues with remote CoreAnimation

These issues came up while running for a few days with the
flag --enable-remote-core-animation.

Fix flashes of old frames by hooking up the DiscardBackbuffer (which
happens when being made non-visible) to re-set the CAContext and
CALayer (so the browser gets a new one with new content for the next
frame).

Add support for disabling vsync by using setNeedsDisplay to draw.

Change the backpressure mechanism to rely on the browser to apply
backpressure in its compositor via the cc:: output surface.

Add a ScopedSetGLToRealGLApi structure to ensure that we are talking
to the real GL API while in the CoreAnimation callback.

BUG= 312462 

Review URL: https://codereview.chromium.org/516643002

Cr-Commit-Position: refs/heads/master@{#292293}

[modify] https://chromium.googlesource.com/chromium/src.git/+/388cc928a95dc1a921d3b1671985046c3c626c22/content/browser/compositor/browser_compositor_view_private_mac.mm
[modify] https://chromium.googlesource.com/chromium/src.git/+/388cc928a95dc1a921d3b1671985046c3c626c22/content/common/gpu/image_transport_surface_calayer_mac.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/388cc928a95dc1a921d3b1671985046c3c626c22/content/common/gpu/image_transport_surface_calayer_mac.mm
[modify] https://chromium.googlesource.com/chromium/src.git/+/388cc928a95dc1a921d3b1671985046c3c626c22/content/common/gpu/image_transport_surface_fbo_mac.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/388cc928a95dc1a921d3b1671985046c3c626c22/content/common/gpu/image_transport_surface_fbo_mac.mm
[modify] https://chromium.googlesource.com/chromium/src.git/+/388cc928a95dc1a921d3b1671985046c3c626c22/content/common/gpu/image_transport_surface_iosurface_mac.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/388cc928a95dc1a921d3b1671985046c3c626c22/content/common/gpu/image_transport_surface_iosurface_mac.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/388cc928a95dc1a921d3b1671985046c3c626c22/ui/compositor/compositor.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/388cc928a95dc1a921d3b1671985046c3c626c22/ui/compositor/compositor.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/388cc928a95dc1a921d3b1671985046c3c626c22/ui/gl/gl_gl_api_implementation.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/388cc928a95dc1a921d3b1671985046c3c626c22/ui/gl/gl_gl_api_implementation.h

Comment 70 by bugdroid1@chromium.org, Aug 28 2014

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f7fd083dbff47a60bd6da12006db1db7b1b4cb43

commit f7fd083dbff47a60bd6da12006db1db7b1b4cb43
Author: epenner <epenner@chromium.org>
Date: Thu Aug 28 19:12:58 2014

Revert of Fix assorted issues with remote CoreAnimation (patchset #2 of https://codereview.chromium.org/516643002/)

Reason for revert:
Speculatively reverting to see if this helps the mac perf bots:
https://code.google.com/p/chromium/issues/detail?id=408673

Original issue's description:
> Fix assorted issues with remote CoreAnimation
>
> These issues came up while running for a few days with the
> flag --enable-remote-core-animation.
>
> Fix flashes of old frames by hooking up the DiscardBackbuffer (which
> happens when being made non-visible) to re-set the CAContext and
> CALayer (so the browser gets a new one with new content for the next
> frame).
>
> Add support for disabling vsync by using setNeedsDisplay to draw.
>
> Change the backpressure mechanism to rely on the browser to apply
> backpressure in its compositor via the cc:: output surface.
>
> Add a ScopedSetGLToRealGLApi structure to ensure that we are talking
> to the real GL API while in the CoreAnimation callback.
>
> BUG= 312462 
>
> Committed: https://chromium.googlesource.com/chromium/src/+/3b6aee8ed0393d852ed21fae78f539ffffe3e8f8

TBR=piman@chromium.org,ccameron@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG= 312462 

Review URL: https://codereview.chromium.org/517733002

Cr-Commit-Position: refs/heads/master@{#292438}

[modify] https://chromium.googlesource.com/chromium/src.git/+/f7fd083dbff47a60bd6da12006db1db7b1b4cb43/content/browser/compositor/browser_compositor_view_private_mac.mm
[modify] https://chromium.googlesource.com/chromium/src.git/+/f7fd083dbff47a60bd6da12006db1db7b1b4cb43/content/common/gpu/image_transport_surface_calayer_mac.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/f7fd083dbff47a60bd6da12006db1db7b1b4cb43/content/common/gpu/image_transport_surface_calayer_mac.mm
[modify] https://chromium.googlesource.com/chromium/src.git/+/f7fd083dbff47a60bd6da12006db1db7b1b4cb43/content/common/gpu/image_transport_surface_fbo_mac.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/f7fd083dbff47a60bd6da12006db1db7b1b4cb43/content/common/gpu/image_transport_surface_fbo_mac.mm
[modify] https://chromium.googlesource.com/chromium/src.git/+/f7fd083dbff47a60bd6da12006db1db7b1b4cb43/content/common/gpu/image_transport_surface_iosurface_mac.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/f7fd083dbff47a60bd6da12006db1db7b1b4cb43/content/common/gpu/image_transport_surface_iosurface_mac.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/f7fd083dbff47a60bd6da12006db1db7b1b4cb43/ui/compositor/compositor.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/f7fd083dbff47a60bd6da12006db1db7b1b4cb43/ui/compositor/compositor.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/f7fd083dbff47a60bd6da12006db1db7b1b4cb43/ui/gl/gl_gl_api_implementation.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/f7fd083dbff47a60bd6da12006db1db7b1b4cb43/ui/gl/gl_gl_api_implementation.h

Comment 71 by bugdroid1@chromium.org, Aug 28 2014

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/18bbc2ab2660ec4ac9ddbd0d2367d9e3886ce136

commit 18bbc2ab2660ec4ac9ddbd0d2367d9e3886ce136
Author: ccameron <ccameron@chromium.org>
Date: Thu Aug 28 22:36:16 2014

Revert of Revert of Fix assorted issues with remote CoreAnimation (patchset #1 of https://codereview.chromium.org/517733002/)

Reason for revert:
Patch was reverted in error

Original issue's description:
> Revert of Fix assorted issues with remote CoreAnimation (patchset #2 of https://codereview.chromium.org/516643002/)
>
> Reason for revert:
> Speculatively reverting to see if this helps the mac perf bots:
> https://code.google.com/p/chromium/issues/detail?id=408673
>
> Original issue's description:
> > Fix assorted issues with remote CoreAnimation
> >
> > These issues came up while running for a few days with the
> > flag --enable-remote-core-animation.
> >
> > Fix flashes of old frames by hooking up the DiscardBackbuffer (which
> > happens when being made non-visible) to re-set the CAContext and
> > CALayer (so the browser gets a new one with new content for the next
> > frame).
> >
> > Add support for disabling vsync by using setNeedsDisplay to draw.
> >
> > Change the backpressure mechanism to rely on the browser to apply
> > backpressure in its compositor via the cc:: output surface.
> >
> > Add a ScopedSetGLToRealGLApi structure to ensure that we are talking
> > to the real GL API while in the CoreAnimation callback.
> >
> > BUG= 312462 
> >
> > Committed: https://chromium.googlesource.com/chromium/src/+/3b6aee8ed0393d852ed21fae78f539ffffe3e8f8
>
> TBR=piman@chromium.org,ccameron@chromium.org
> NOTREECHECKS=true
> NOTRY=true
> BUG= 312462 
>
> Committed: https://chromium.googlesource.com/chromium/src/+/07539c445c9fa7161b3da9031cf440a2b657dd18

TBR=piman@chromium.org,epenner@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG= 312462 

Review URL: https://codereview.chromium.org/517143002

Cr-Commit-Position: refs/heads/master@{#292481}

[modify] https://chromium.googlesource.com/chromium/src.git/+/18bbc2ab2660ec4ac9ddbd0d2367d9e3886ce136/content/browser/compositor/browser_compositor_view_private_mac.mm
[modify] https://chromium.googlesource.com/chromium/src.git/+/18bbc2ab2660ec4ac9ddbd0d2367d9e3886ce136/content/common/gpu/image_transport_surface_calayer_mac.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/18bbc2ab2660ec4ac9ddbd0d2367d9e3886ce136/content/common/gpu/image_transport_surface_calayer_mac.mm
[modify] https://chromium.googlesource.com/chromium/src.git/+/18bbc2ab2660ec4ac9ddbd0d2367d9e3886ce136/content/common/gpu/image_transport_surface_fbo_mac.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/18bbc2ab2660ec4ac9ddbd0d2367d9e3886ce136/content/common/gpu/image_transport_surface_fbo_mac.mm
[modify] https://chromium.googlesource.com/chromium/src.git/+/18bbc2ab2660ec4ac9ddbd0d2367d9e3886ce136/content/common/gpu/image_transport_surface_iosurface_mac.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/18bbc2ab2660ec4ac9ddbd0d2367d9e3886ce136/content/common/gpu/image_transport_surface_iosurface_mac.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/18bbc2ab2660ec4ac9ddbd0d2367d9e3886ce136/ui/compositor/compositor.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/18bbc2ab2660ec4ac9ddbd0d2367d9e3886ce136/ui/compositor/compositor.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/18bbc2ab2660ec4ac9ddbd0d2367d9e3886ce136/ui/gl/gl_gl_api_implementation.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/18bbc2ab2660ec4ac9ddbd0d2367d9e3886ce136/ui/gl/gl_gl_api_implementation.h

Comment 72 by ccameron@chromium.org, Sep 3 2014

Progress here is coming extremely slowly. I still need to fix a back-pressure issue related to tab capture (if tab capture is enabled. Also, the clean-up I was doing before this in https://codereview.chromium.org/490393002/ took forever to get working, and was just reverted.

Comment 73 by bugdroid1@chromium.org, Oct 9 2014

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/95e3616034fcf2ef6d1a7caf8b289ad87ad8b6d0

commit 95e3616034fcf2ef6d1a7caf8b289ad87ad8b6d0
Author: ccameron <ccameron@chromium.org>
Date: Thu Oct 09 09:06:42 2014

Clean up GPU back-pressure with remote CALayers

If a CAOpenGLLayer is attached to a NSView that is completely occluded
or isn't in the window hierarchy, then when we ask that layer to draw, it
will not draw, or will draw after a long delay. This optimization can be a
problem if we need to renderer to continue to produce frames, e.g, for
tab capture.

Fix this issue by having the frame acknowledgement from the browser
to the GPU process (which currently informs the GPU process of the
CGL renderer ID) inform the GPU process of the occlusion status of the
NSView being drawn to. Use this information in the GPU process to force
the CAOpenGLLayer to draw as soon as a new frame arrives.

Add the same timeout mechanism as is in the IOSurface drawing path
to limit CoreAnimation's throttling (based on GPU back-pressure) to
6 fps, to avoid having the renderer block indefinitely. This is something
of a hack, but is an unfortunate necessity.

Note that this change assumes that calling setNeedsDisplay and then
displayIfNeeded will result in the CALayer being draw except in the
case where the CALayer is backing a NSView that is not in the window
hierarchy. This exception only happens during tab capture of a
backgrounded tab, and the contents not drawn are actually never seen,
so it is safe to skip the draw entirely.

Also, wrap some Mac-only IPCs and structures #if defined(OS_MACOSX).

BUG= 312462 

Review URL: https://codereview.chromium.org/636003002

Cr-Commit-Position: refs/heads/master@{#298828}

[modify] https://chromium.googlesource.com/chromium/src.git/+/95e3616034fcf2ef6d1a7caf8b289ad87ad8b6d0/content/browser/compositor/browser_compositor_ca_layer_tree_mac.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/95e3616034fcf2ef6d1a7caf8b289ad87ad8b6d0/content/browser/compositor/browser_compositor_ca_layer_tree_mac.mm
[modify] https://chromium.googlesource.com/chromium/src.git/+/95e3616034fcf2ef6d1a7caf8b289ad87ad8b6d0/content/browser/compositor/browser_compositor_view_mac.mm
[modify] https://chromium.googlesource.com/chromium/src.git/+/95e3616034fcf2ef6d1a7caf8b289ad87ad8b6d0/content/common/gpu/gpu_messages.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/95e3616034fcf2ef6d1a7caf8b289ad87ad8b6d0/content/common/gpu/image_transport_surface.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/95e3616034fcf2ef6d1a7caf8b289ad87ad8b6d0/content/common/gpu/image_transport_surface.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/95e3616034fcf2ef6d1a7caf8b289ad87ad8b6d0/content/common/gpu/image_transport_surface_calayer_mac.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/95e3616034fcf2ef6d1a7caf8b289ad87ad8b6d0/content/common/gpu/image_transport_surface_calayer_mac.mm
[modify] https://chromium.googlesource.com/chromium/src.git/+/95e3616034fcf2ef6d1a7caf8b289ad87ad8b6d0/content/common/gpu/image_transport_surface_fbo_mac.h
[modify] https://chromium.googlesource.com/chromium/src.git/+/95e3616034fcf2ef6d1a7caf8b289ad87ad8b6d0/content/common/gpu/image_transport_surface_fbo_mac.mm
[modify] https://chromium.googlesource.com/chromium/src.git/+/95e3616034fcf2ef6d1a7caf8b289ad87ad8b6d0/content/common/gpu/image_transport_surface_iosurface_mac.cc
[modify] https://chromium.googlesource.com/chromium/src.git/+/95e3616034fcf2ef6d1a7caf8b289ad87ad8b6d0/content/common/gpu/image_transport_surface_iosurface_mac.h

Comment 74 by ccameron@chromium.org, Oct 10 2014

Back to trying to get this enabled now. Handful of failures on the GPU bots that still need looking at
https://codereview.chromium.org/474723002/

Comment 77 by ccameron@chromium.org, Oct 13 2014

So far the only fallout from this is  issue 422803 , which has a fix out for review.

Of note is the work so far has *not* eliminated the extra copy done -- it has just moved it to the GPU process. I've separated that step out into  issue 423163 .

Comment 78 by ccameron@chromium.org, Oct 17 2014

Status: Fixed
Two more issues that appear to have come up at the same time as this, and are almost certainly related --  issue 424713  and  issue 424433  -- probably not hard to fix.

Marking this as fixed -- the rest will go into bugfixing.

Sign in to add a comment