687868 - GfxInfo::Init() shouldn't put us in Discrete (CGLQueryRendererInfo)

Status: REOPENED

(Core :: Graphics, defect)

Description

[Benoit Girard (:BenWa)]

Currently we call CGLQueryRendererInfo in GfxInfo::Init() which puts us in Discrete mode. We need to get renderer info without having it put Firefox in Discrete.

Comment 1

[Jeff Muizelaar [:jrmuizel]]

Attachment: Redo blacklisting to use device ids instead of render info

This removes the work from 611292. We explicitly ask for a hardware renderer (http://mxr.mozilla.org/mozilla-central/source/gfx/thebes/GLContextProviderCGL.mm#88) so it shouldn't be needed.

Comment 2

[Jeff Muizelaar [:jrmuizel]]

Attachment: Redo blacklisting to use device ids instead of render info v2

Add some more comment

Comment 3

[Benoit Girard (:BenWa)]

This fix is no longer required to fix bug 687864. Apple looks at our bundle ID and make it such that CGLQueryRendererInfo does not put us into Discrete for builds with the bundle id 'org.mozilla.firefox'

Comment 4

[Jeff Muizelaar [:jrmuizel]]

It does prevent us from testing it though, so I'd like to keep the dependency around.

Comment 5

[Benoit Jacob [:bjacob] (mostly away)]

Comment on attachment 561605 [details] [diff] [review]
Redo blacklisting to use device ids instead of render info v2

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

r+ with following comments (changes needed):

::: widget/src/cocoa/GfxInfo.mm
@@ +106,5 @@
> +IsATIRadeonX1000(PRUint32 aVendorID, PRUint32 aDeviceID)
> +{
> +  if (aVendorID == 0x1002) {
> +    // this list is from the ATIRadeonX1000.kext Info.plist
> +    PRUint32 devices[] = {0x7187, 0x72101, 0x71DE, 0x7146, 0x7142, 0x7109, 0x71C5, 0x71C0, 0x7240, 0x7249 0x7291};

the 0x72101 here is bogus; i'm ok to r+ the rest if you're confident in this list

@@ +111,5 @@
> +    for (size_t i; i<NS_ARRAY_LENGTH(devices); i++) {
> +      if (aDeviceID == devices[i])
> +        return true;
> +    }
> +  }

return missing

@@ +358,5 @@
> +    if (!IsATIRadeonX1000(mAdapterVendorID, mAdapterDeviceId)) {
> +      foundGoodDevice = PR_TRUE;
> +    }
> +
> +#if 0

OK. if the slowness in bug 611292 was caused by software fallbacks then we might take a performance regression from this so it's worth replying to bug 611292 to ask people to test, and let this bake in channels. However Jeff points out that we're already requiring NSOpenGLPFAAccelerated so in principle this shouldn't happen.

Comment 6

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/9ba53832faf2

Comment 7

[Reuben Morais [:reuben]]

GfxInfo::Init() still triggers discrete graphics mode for me, on

  Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7.2; rv:10.0a1) Gecko/20111028 Firefox/10.0a1 ID:20111028031044

To reproduce, simply open about:support.

Comment 8

[Benoit Girard (:BenWa)]

Confirmed, it still happens and I get a nasty black flash on 10.6.

Comment 9

[Reuben Morais [:reuben]]

After changing the build identifier to org.mozilla.firefox, we go back to integrated graphics right after causing the switch.