688844 - Stop using PBuffers for plugins on OS X

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

CGLCreatePBuffer and friends are deprecated on 10.7 there may also be some bad interaction with offline renderers.

Comment 1

[Benoit Girard (:BenWa)]

Still used here:
http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/nsCoreAnimationSupport.mm#461

Comment 2

[Benoit Girard (:BenWa)]

Attachment: Remove PBuffers for in-process plugins

Comment 3

[Benoit Girard (:BenWa)]

Attachment: Remove PBuffers for in-process plugins

Changes after self-review. Tested this change for IP and OOP plugins youtube+flash.

Comment 4

[Benoit Girard (:BenWa)]

I forgot to mention, we we're only using PBuffers for in process plugin when rendering to a CGImage. We we're alread using FBOs to draw to a shared texture (IOSurface).

Comment 5

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 565749 [details] [diff] [review]
Remove PBuffers for in-process plugins

Review of attachment 565749 [details] [diff] [review]:
-----------------------------------------------------------------

There's some white space changes mixed in here, but overall it looks decent. Have you tested both mIOSurface and !mIOSurface paths?

::: gfx/thebes/nsCoreAnimationSupport.mm
@@ +560,5 @@
>                                             mIOSurface->mIOSurfacePtr, 0);
>      ::glBindTexture(GL_TEXTURE_RECTANGLE_ARB, 0);
> +  } else {
> +    ::glGenTextures(1, &mFBOTexture);
> +    ::glBindTexture(GL_TEXTURE_RECTANGLE_ARB, mIOTexture);

Should this be mFBOTexture?

Comment 6

[Benoit Girard (:BenWa)]

Yes good catch. I had tested this for IP/OOP so I'm not sure why this wasn't caught.

Comment 7

[Benoit Girard (:BenWa)]

Attachment: patch v2

Comment 8

[Benoit Girard (:BenWa)]

Pushed to try: https://hg.mozilla.org/try/rev/a6bd54d83f63

Comment 9

[Mozilla RelEng Bot]

Try run for a6bd54d83f63 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=a6bd54d83f63
Results (out of 2 total builds):
    failure: 2
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-a6bd54d83f63

Comment 10

[Benoit Girard (:BenWa)]

It doesn't like that I removed the _EXT bits although that worked fine on my machine. Some of the apple documentation drops it but I'm guessing the 10.5 SDKs still requires it.

Comment 11

[Benoit Girard (:BenWa)]

Attachment: patch v3 (Re-add EXT postfix)

Carrying forward review since there are no functional changes.

Comment 12

[Mozilla RelEng Bot]

Try run for 24a3443d2449 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=24a3443d2449
Results (out of 13 total builds):
    success: 13
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-24a3443d2449

Comment 13

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/d0639183a957

Comment 14

[Marco Bonardo [:mak]]

and backed out
https://hg.mozilla.org/mozilla-central/rev/bd376ef5c7a8

Comment 15

[Benoit Girard (:BenWa)]

Attachment: patch (carry forward r=jrmuizel)

Waiting on tree closure.

Comment 16

[Benoit Girard (:BenWa)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/319d93476307

Comment 17

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/319d93476307