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 User image 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 User image 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 User image Benoit Girard (:BenWa) 2011-10-08 11:22:15 PDT
Created attachment 565748 [details] [diff] [review]
Remove PBuffers for in-process plugins
Comment 3 User image 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 User image 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 User image 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 User image 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 User image Benoit Girard (:BenWa) 2011-10-13 07:38:00 PDT
Created attachment 566811 [details] [diff] [review]
patch v2
Comment 8 User image Benoit Girard (:BenWa) 2011-10-17 09:30:09 PDT
Pushed to try: https://hg.mozilla.org/try/rev/a6bd54d83f63
Comment 9 User image 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 User image 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 User image 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 User image 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 User image Marco Bonardo [::mak] 2011-10-18 05:40:32 PDT
https://hg.mozilla.org/mozilla-central/rev/d0639183a957
Comment 14 User image Marco Bonardo [::mak] 2011-10-18 05:42:06 PDT
and backed out
https://hg.mozilla.org/mozilla-central/rev/bd376ef5c7a8
Comment 15 User image 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 User image 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.