Closed Bug 688844 Opened 8 years ago Closed 8 years ago

Stop using PBuffers for plugins on OS X

Categories

(Core :: Graphics, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla10

People

(Reporter: jrmuizel, Assigned: BenWa)

Details

Attachments

(1 file, 4 obsolete files)

CGLCreatePBuffer and friends are deprecated on 10.7 there may also be some bad interaction with offline renderers.
Assignee: nobody → bgirard
Status: NEW → ASSIGNED
Changes after self-review. Tested this change for IP and OOP plugins youtube+flash.
Attachment #565748 - Attachment is obsolete: true
Attachment #565749 - Flags: review?(jmuizelaar)
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 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?
Yes good catch. I had tested this for IP/OOP so I'm not sure why this wasn't caught.
Attachment #565749 - Flags: review?(jmuizelaar)
Attached patch patch v2 (obsolete) — Splinter Review
Attachment #566811 - Flags: review?(jmuizelaar)
Attachment #566811 - Flags: review?(jmuizelaar) → review+
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
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.
Attached patch patch v3 (Re-add EXT postfix) (obsolete) — Splinter Review
Carrying forward review since there are no functional changes.
Attachment #566811 - Attachment is obsolete: true
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
https://hg.mozilla.org/mozilla-central/rev/d0639183a957
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
and backed out
https://hg.mozilla.org/mozilla-central/rev/bd376ef5c7a8
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla10 → ---
Attachment #565749 - Attachment is obsolete: true
Waiting on tree closure.
Attachment #567505 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/319d93476307
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla10
You need to log in before you can comment on or make changes to this bug.