411475 - Canvas3D not working on Radeon 9250

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Andrew Smith]

Attachment: add maxNumFormats to wglChoosePixelFormatARB()

The http://people.mozilla.com/~vladimir/canvas3d/examples/simple/simple.html example doesn't display anything when using a Radeon 9250.

Fix attached.

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 296113 [details] [diff] [review]
add maxNumFormats to wglChoosePixelFormatARB()

>     float fattribs[] = { 0.0f };
>     UINT numFormats = 0;
>-
>-    //fprintf (stderr, "EXT: %p ARB: %p rest: %s\n", wglewGetContext()->__wglewChoosePixelFormatEXT, wglewGetContext()->__wglewChoosePixelFormatARB, wglGetExtensionsStringARB(mGlewDC));
>-
>+    const int maxNumFormats = 10;
>+    int unusedFormatsList[maxNumFormats]; /* need this or else the function below will fail at times */
>+    
>     if (!wglChoosePixelFormatARB(mGlewDC,
>                                  attribs,
>                                  fattribs,
>-                                 0,
>-                                 NULL,
>-                                 &numFormats) ||
>-        numFormats == 0)
>+                                 maxNumFormats,
>+                                 unusedFormatsList,
>+                                 &numFormats))

So if ATI is failing on this, they're dumb and not following the spec.  However, the workaround would be to do something like:

int dummy;
wglChoosePixelFormatARB(mGlewDC, attribs, fattribs, 1, &dummy, &numFormats)

instead of trying to guess some maxNumFormats.

Comment 2

[Andrew Smith]

Well with the Radeon 9250 wglChoosePixelFormatARB() the way I call it returns 2 via numFormats, and chooses the second. When I change it to maxNumFormats 1, it returns 1, and chooses 1. It still works, but I'm guessing it's not doing the best it can.

I suggest the best thing to do is to figure out (or estimate/guesstimate) the real maximum number of formats concievably available, and use that. It's not like it's a performance issue :)

Comment 3

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Er, again, you generally have to call this function twice.  Is the value returned in numFormats real?  If so, then everything should be fine -- you'll note that wglChoosePixelFormatARB isn't making the final decision over which format to use; it's just returning a list, which is then queried in the loop later on.

What exactly is the problem on the radeon?  Is it picking a bogus pixel format, or what?

Comment 4

[Andrew Smith]

The problems seems to be that the value returned in numFormats is limited by maxNumFormats.

For example with maxNumFormats 10, numFormats is 2. If I change maxNumFormats to 1, numFormats is 1.

I don't know whether returning more than 1 via numFormats is useful, but I assume it is, since if it returns 2, on the Radeon9250 chosenFormat is chosen to be 2. And if it returns 1, chosenFormat is chosen to be 1.

Comment 5

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Dumb question, which version of the ATI driver do you have installed?  This is sounding like a crappy driver bug, but I guess we should try to work around it.  I would do something like:

#define STACK_FORMATS 64
int stackFormats[STACK_FORMATS];
int *formats = stackFormats;
int maxFormats = STACK_FORMATS;
int numFormats;

... wglChoosePixelFormat (... formats, maxFormats, &numFormats);

if (numFormats > maxFormats) {
   formats = new int[numFormats];
   maxFormats = numFormats;
   wglChoosePixelFormat (... formats, maxFormats, &numFormats);
}

/* do the stuff with formats array */

if (formats != stackFormats)
   delete [] formats;

Comment 6

[Andrew Smith]

Attachment: v2 patch

Sounds good to me, new patch attached.

I'm not sure how to check the driver version. I think I downloaded the latest one from ATI when I started, but I can't remember for sure.

Comment 7

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 298179 [details] [diff] [review]
v2 patch

looks good!

Comment 8

[José Jeria]

Was this ever checked in?

Comment 9

[Andrew Smith]

I think all of canvas3d has been moved outside the mozilla tree. And I suspect it was checked in, though I haven't actually looked at the code recently.

Comment 10

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

This bug is super old. But our current WGL code looks like it has the fix:

#define MAX_NUM_FORMATS 256
    UINT numFormats = MAX_NUM_FORMATS;
    int formats[MAX_NUM_FORMATS];

    if (!sWGLLibrary.fChoosePixelFormat(gDummyWindowDC,
                                        attribs.Elements(), NULL,
                                        numFormats, formats, &numFormats)
        || numFormats == 0)
    {
        return nsnull;
    }


So, marking as fixed.