Stop using PBuffers for plugins on OS X

RESOLVED FIXED in mozilla10

Status

()

Core
Graphics
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jrmuizel, Assigned: BenWa)

Tracking

unspecified
mozilla10
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

6 years ago
CGLCreatePBuffer and friends are deprecated on 10.7 there may also be some bad interaction with offline renderers.
(Assignee)

Comment 1

6 years ago
Still used here:
http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/nsCoreAnimationSupport.mm#461
(Assignee)

Comment 2

6 years ago
Created attachment 565748 [details] [diff] [review]
Remove PBuffers for in-process plugins
Assignee: nobody → bgirard
Status: NEW → ASSIGNED
(Assignee)

Comment 3

6 years ago
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.
Attachment #565748 - Attachment is obsolete: true
Attachment #565749 - Flags: review?(jmuizelaar)
(Assignee)

Comment 4

6 years ago
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).
(Reporter)

Comment 5

6 years ago
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?
(Assignee)

Comment 6

6 years ago
Yes good catch. I had tested this for IP/OOP so I'm not sure why this wasn't caught.
(Assignee)

Updated

6 years ago
Attachment #565749 - Flags: review?(jmuizelaar)
(Assignee)

Comment 7

6 years ago
Created attachment 566811 [details] [diff] [review]
patch v2
(Assignee)

Updated

6 years ago
Attachment #566811 - Flags: review?(jmuizelaar)
(Reporter)

Updated

6 years ago
Attachment #566811 - Flags: review?(jmuizelaar) → review+
(Assignee)

Comment 8

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

Comment 9

6 years ago
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
(Assignee)

Comment 10

6 years ago
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.
(Assignee)

Comment 11

6 years ago
Created attachment 567505 [details] [diff] [review]
patch v3 (Re-add EXT postfix)

Carrying forward review since there are no functional changes.
Attachment #566811 - Attachment is obsolete: true

Comment 12

6 years ago
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
Last Resolved: 6 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 → ---
(Assignee)

Updated

6 years ago
Attachment #565749 - Attachment is obsolete: true
(Assignee)

Comment 15

6 years ago
Created attachment 568169 [details] [diff] [review]
patch (carry forward r=jrmuizel)

Waiting on tree closure.
Attachment #567505 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
(Assignee)

Comment 16

6 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/319d93476307
https://hg.mozilla.org/mozilla-central/rev/319d93476307
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 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.