[Piglit] [waffle] long-standing wgl pixel format issue

[Brian Paul]

On 06/20/2016 05:48 PM, Emil Velikov wrote:
> On 20 June 2016 at 15:46, Brian Paul <brianp at vmware.com> wrote:
>> On 06/17/2016 07:59 PM, Emil Velikov wrote:
>>>
>>> On 17 June 2016 at 16:53, Brian Paul <brianp at vmware.com> wrote:
>>>>
>>>>
>>>> I spent a few hours yesterday pulling out my hair trying to understand
>>>> why
>>>> the piglit fbo-mipmap-copypix test was failing on Windows.  But it was
>>>> only
>>>> failing when I ran it directly.  It passed when I ran it via
>>>> piglit-run.py
>>>>
>>>> The key difference was the -fbo option.  With -fbo the test used an RGBA8
>>>> framebuffer but without -fbo the test used an RGB565 framebuffer.
>>>>
>>>> So why is a 565 framebuffer being used?  It boils down to the fact that
>>>> wglChoosePixelFormatARB() does not work like glXChooseFBConfig().
>>>>
>>>>   From the glXChooseFBConfig man page:
>>>> """
>>>>     GLX_RED_SIZE, GLX_GREEN_SIZE, GLX_BLUE_SIZE, GLX_ALPHA_SIZE
>>>>
>>>>        Each attribute, if present, must be followed by a nonnegative
>>>> minimum
>>>> size
>>>>       specification or GLX_DONT_CARE.
>>>>       The largest available total RGBA color buffer size (sum of
>>>> GLX_RED_SIZE,
>>>>       GLX_GREEN_SIZE, GLX_BLUE_SIZE, and GLX_ALPHA_SIZE)
>>>>       of at least the minimum size specified for each color component is
>>>> preferred.
>>>> """
>>>>
>>>> So if you specify GLX_RED_SIZE, BLUE_SIZE, etc to be 1 and there are both
>>>> RGB565 and RGBA8 formats available, the _later_ (the largest) will be
>>>> chosen.
>>>>
>>>> But the wglChoosePixelFormatARB docs say:
>>>> """
>>>> Some attribute values must match the pixel format value exactly when
>>>>       the attribute is specified while others specify a minimum criteria,
>>>>       meaning that the pixel format value must meet or exceed the
>>>>       specified value.
>>>>
>>>>        Attribute                      Type        Match Criteria
>>>>        WGL_RED_BITS_ARB               integer     minimum
>>>>        WGL_GREEN_BITS_ARB             integer     minimum
>>>>        WGL_BLUE_BITS_ARB              integer     minimum
>>>>        WGL_ALPHA_BITS_ARB             integer     minimum
>>>> """
>>>>
>>>> So if you specify WGL_RED/GREEN/BLUE_BITS_ARB to be 1 and there are both
>>>> RGB565 and RGBA8 formats available, the _former_ may be chosen.  Note
>>>> that
>>>> some WGL apps use WGL_COLOR_BITS_ARB=24 and avoid this.
>>>>
>>>> Piglit's call to piglit_wfl_framework_init() uses an attribute list with
>>>> WAFFLE_RED/GREEN/BLUE_SIZE = 1 and that winds up going directly to
>>>> wglChoosePixelFormatARB and glXChooseFBConfig so this difference in
>>>> behavior
>>>> effects the window's pixel format.
>>>>
>>> Thanks for this Brian and apologies I did not spot these differences
>>> as I was writing the WGL backend.
>>>
>>> Here's a bit more comprehensive list, listing all the waffle backends
>>> and attributes.
>>>
>>> GLX/EGL:
>>> Largest - red, green, blue, alpha plus their accum counterparts + depth
>>> Smallest - buffer, stencil
>>>
>>> If requested size is zero - "largest" become "smallest" (but it's not
>>> said it will be zero), "smallest" become "zero".
>>>
>>> CGL
>>> One that "most closely matches the specified size is preferred"
>>>
>>> WGL/NaCL
>>> "At least", meaning that there's not definition if it's the "smallest"
>>> or "largest". Furthermore there's not mention that it will give you
>>> the smallest if you specify 0 :-\
>>>
>>>
>>>> The Waffle docs for waffle_config_choose() say:
>>>>
>>>> """
>>>> WAFFLE_RED_SIZE
>>>> WAFFLE_GREEN_SIZE
>>>> WAFFLE_BLUE_SIZE
>>>> WAFFLE_ALPHA_SIZE
>>>> WAFFLE_DEPTH_SIZE
>>>> WAFFLE_STENCIL_SIZE
>>>>
>>>>       The default value for each size attribute is 0. Valid values are the
>>>> non-negative integers and WAFFLE_DONT_CARE. If the requested size for a
>>>> channel is 0, then any surface created with the config will lack that
>>>> channel. If the requested size for a channel is positive, then the number
>>>> of
>>>> bits in that channel for any surface created with the config will be at
>>>> least the requested size.
>>>> """
>>>>
>>>> There's some ambiguity here because if several different pixel formats
>>>> (such
>>>> as RGB565 and RGBA8) both meet the WAFFLE_RED/GREEN/BLUE_SIZE minimums,
>>>> which should be preferred?
>>>>
>>>> I can fix my Windows Piglit issue by changing Piglit's
>>>> choose_config_attribs() function to specify WAFFLE_RED/GREEN/BLUE_SIZE=8
>>>> instead of 1, but that's not a final solution.
>>>>
>>>>
>>>> I propose:
>>>>
>>>> 1. The Waffle docs should be clarified to specify whether the largest or
>>>> smallest color format should be used when several meet the WAFFLE_*_SIZE
>>>> minimums.  My suggesting is "smallest", like WGL.
>>>>
>>>> 2. The Waffle code for either GLX or WGL should be modified to follow
>>>> that
>>>> part of the spec.  Following my suggestion, the GLX format chooser code
>>>> would need to be modified.
>>>>
>>>> 3. The Piglit code to specify the Waffle pixel format should be updated,
>>>> probably replacing '1' with '8' as above.  And maybe falling back to the
>>>> former if the later fails (though I doubt anyone runs piglit on less than
>>>> a
>>>> 24-bit display nowadays).
>>>>
>>>> 4. If Waffle wants to get fancy, we could consider new attributes like
>>>> WAFFLE_MIN_RED_SIZE, WAFFLE_MAX_RED_SIZE and WAFFLE_EXACT_RED_SIZE to
>>>> provide more control over format selection.  But I think my suggestion in
>>>> (1) would avoid this for now.
>>>>
>>>> Thoughts?
>>>>
>>> I'm somewhat inclined that the GLX/EGL behaviour might be the better
>>> choice. Then again I don't might if people choose another route -
>>> always smallest, always largest, a combination of the two, minimum or
>>> "most closely matches".
>>> In each case there's the issue (as each spec has a massive table) of
>>> how exactly to determine(sort) the config(s).
>>
>>
>> I think the "choose smallest that exceeds the user-specified minimum" is the
>> way to go.
>>
> Can I put a minor correction - s/that exceeds/that meets or exceeds/.
>
> Should that rule hold true, even if the user have explicitly set some
> sizes to zero ... for example if they want an RGB8 (A=0) format ?
>
>> Suppose you have a system that has 3 color formats: RGB10_A2, RGBA8 and
>> R5G6B5.  An app could specify WAFFLE_RED/GREEN/BLUE_SIZE=10 to get the first
>> one,
> I hope for cases like this user will set A=2 ;-)

If you really need A=2, yes.  The default for WAFFLE_ALPHA_SIZE is zero 
so the "smallest" config with R,G,B>=10 and A>=0 would be returned.  So 
if there were a RGB10_X2 config you'd get that if WAFFLE_ALPHA_SIZE was 
omitted.


>
>> or WAFFLE_RED/GREEN/BLUE_SIZE=8 to get the second, etc.
>>
>>
>>> Now that I think of it I do recall seeing some heuristics on the topic
>>> in SDL and Xwin. Perhaps it's worth checking what they do and using
>>> something identical/close to it ?
>>
>>
>> Sounds good. Do you want to do that?
>>
> Seems like I was a bit off - Xwin does not do anything fancy, just
> like most of SDL. The latter uses an algorithm [1] only for EGL, while
> we'll need to use one for GLX/EGL/WGL/NaCl.

OK, I'll see if I can make time to work on this.

I haven't seen feedback from anyone else so I'll assume there's no 
objections to fixing this issue.

Thanks, Emil.

-Brian