Last Comment Bug 688844 - Stop using PBuffers for plugins on OS X
: Stop using PBuffers for plugins on OS X
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Graphics (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla10
Assigned To: Benoit Girard (:BenWa)
:
: Milan Sreckovic [:milan]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-09-23 12:55 PDT by Jeff Muizelaar [:jrmuizel]
Modified: 2011-10-21 02:17 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Remove PBuffers for in-process plugins (10.90 KB, patch)
2011-10-08 11:22 PDT, Benoit Girard (:BenWa)
no flags Details | Diff | Splinter Review
Remove PBuffers for in-process plugins (10.94 KB, patch)
2011-10-08 11:25 PDT, Benoit Girard (:BenWa)
no flags Details | Diff | Splinter Review
patch v2 (11.13 KB, patch)
2011-10-13 07:38 PDT, Benoit Girard (:BenWa)
jmuizelaar: review+
Details | Diff | Splinter Review
patch v3 (Re-add EXT postfix) (11.20 KB, patch)
2011-10-17 11:33 PDT, Benoit Girard (:BenWa)
no flags Details | Diff | Splinter Review
patch (carry forward r=jrmuizel) (11.12 KB, patch)
2011-10-19 13:29 PDT, Benoit Girard (:BenWa)
no flags Details | Diff | Splinter Review

Description Jeff Muizelaar [:jrmuizel] 2011-09-23 12:55:45 PDT
CGLCreatePBuffer and friends are deprecated on 10.7 there may also be some bad interaction with offline renderers.
Comment 1 Benoit Girard (:BenWa) 2011-10-07 16:16:27 PDT
Still used here:
http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/nsCoreAnimationSupport.mm#461
Comment 2 Benoit Girard (:BenWa) 2011-10-08 11:22:15 PDT
Created attachment 565748 [details] [diff] [review]
Remove PBuffers for in-process plugins
Comment 3 Benoit Girard (:BenWa) 2011-10-08 11:25:51 PDT
Created attachment 565749 [details] [diff] [review]
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) 2011-10-08 11:26:48 PDT
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] 2011-10-13 06:40:58 PDT
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) 2011-10-13 06:55:56 PDT
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) 2011-10-13 07:38:00 PDT
Created attachment 566811 [details] [diff] [review]
patch v2
Comment 8 Benoit Girard (:BenWa) 2011-10-17 09:30:09 PDT
Pushed to try: https://hg.mozilla.org/try/rev/a6bd54d83f63
Comment 9 Mozilla RelEng Bot 2011-10-17 10:30:51 PDT
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/b56girard@gmail.com-a6bd54d83f63
Comment 10 Benoit Girard (:BenWa) 2011-10-17 11:23:51 PDT
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) 2011-10-17 11:33:30 PDT
Created attachment 567505 [details] [diff] [review]
patch v3 (Re-add EXT postfix)

Carrying forward review since there are no functional changes.
Comment 12 Mozilla RelEng Bot 2011-10-17 14:10:53 PDT
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/b56girard@gmail.com-24a3443d2449
Comment 13 Marco Bonardo [::mak] 2011-10-18 05:40:32 PDT
https://hg.mozilla.org/mozilla-central/rev/d0639183a957
Comment 14 Marco Bonardo [::mak] 2011-10-18 05:42:06 PDT
and backed out
https://hg.mozilla.org/mozilla-central/rev/bd376ef5c7a8
Comment 15 Benoit Girard (:BenWa) 2011-10-19 13:29:30 PDT
Created attachment 568169 [details] [diff] [review]
patch (carry forward r=jrmuizel)

Waiting on tree closure.
Comment 17 Marco Bonardo [::mak] 2011-10-21 02:17:57 PDT
https://hg.mozilla.org/mozilla-central/rev/319d93476307

Note You need to log in before you can comment on or make changes to this bug.