[Mesa-dev] [PATCH 05/11] [RFC] mesa: allow binding framebuffer without depth&stencil

[Miklós Máté]

On 02/01/2016 03:51 PM, Miklós Máté wrote:
> On 12/19/2015 12:24 AM, Marek Olšák wrote:
>> On Fri, Dec 18, 2015 at 11:45 PM, Miklós Máté<mtmkls at gmail.com>  
>> wrote:
>>> On 12/17/2015 01:06 PM, Marek Olšák wrote:
>>>> On Wed, Dec 16, 2015 at 11:30 PM, Miklós Máté<mtmkls at gmail.com>  
>>>> wrote:
>>>>> On 12/16/2015 05:27 PM, Marek Olšák wrote:
>>>>>> What is this good for?
>>>>>>
>>>>>> Marek
>>>>> KotOR uses a series of scratch framebuffers for drawing the 
>>>>> framebuffer
>>>>> effects. These have no depth and no stencil, so check_compatible()
>>>>> rejects
>>>>> them, subsequent GL calls are no-op, and the screen becomes 
>>>>> garbage. I
>>>>> also
>>>>> experimented successfully with disabling the visuals that have no 
>>>>> depth
>>>>> or
>>>>> no stencil in gallium/state_trackers/dri/dri_screen.c, but I 
>>>>> concluded
>>>>> that
>>>>> this one was a smaller hack.
>>>> What kind of scratch buffer are we talking about? How is it created?
>>>>
>>>> Marek
>>> They are pbuffers, created like this:
>>>
>>> glXChooseFBConfig(dpy = 0x7cbe2280, screen = 0, attribList = 
>>> [GLX_RED_SIZE,
>>> 8, GLX_GREEN_SIZE, 8, GLX_BLUE_SIZE, 8, GLX_ALPHA_SIZE, 8, 
>>> GLX_DOUBLEBUFFER,
>>> False, GLX_DRAWABLE_TYPE, GLX_PBUFFER_BIT, GLX_RENDER_TYPE, 
>>> GLX_RGBA_BIT |
>>> GLX_COLOR_INDEX_BIT | 0xfffffffffffffffc, 0], nitems = [80])
>>> glXGetFBConfigAttrib(dpy = 0x7cbe2280, config = 0x7ccf08a0, attribute =
>>> GLX_FBCONFIG_ID, value = [104])
>>> glXCreatePbuffer(dpy = 0x7cbe2280, config = 0x7ccf08a0, attribList =
>>> [GLX_PBUFFER_WIDTH, 64, GLX_PBUFFER_HEIGHT, 64, 0])
>>>
>>> Since depth is unspecified in the attrib list (I checked that if the 
>>> game
>>> had supplied WGL_DEPTH_BITS_ARB to wglChoosePixelFormatARB, Wine 
>>> would have
>>> added GLX_DEPTH_SIZE to the attrib list), the first fbconfig is 
>>> chosen that
>>> has 8 bits of r,g,b, which happens to be the very first config in 
>>> the list,
>>> and it has no depth or stencil.
>>>
>>> I noticed that the list of visuals that glxinfo prints has two elements
>>> prepended that look like a bugfix for a similar problem. Maybe that 
>>> would be
>>> the optimal solution in this case as well?
>> Yes. If reordering the FBConfigs fixes the issue, it would be an
>> acceptable workaround I think.
>>
>> Marek
> I'm ready to post v2 of my patch series, except this one (well, I 
> haven't been able to tackle any of the 3 problems mentioned in 00/11 
> either, but those are a different story).
>
> Quick recap: the problem is that KotOR uses a series of pbuffers to 
> draw its post-process effects, but their visuals (depth=0, stencil=0 
> because they are not specified in the attrib list) are incompatible 
> with the gl context, so the gl calls are no-op, and it blits back 
> uninitialized data onto the final image.
>
> I found 3 ways to work around this issue (tested with radeonsi), but I 
> can't decide which one is the smallest/acceptable hack:
>
> 1. remove the compatibility check in main/context.c (this is what 
> patch 05/11 does) -- honestly I don't know why ctx needs a visual, it 
> should be sufficient to use the visual of the currently bound buffer; 
> moreover, I couldn't find these compatibility criteria in the glX 
> specification
>
> 2. in src/glx/glxcmds.c fbconfig_compare(): use PREFER_LARGER() for 
> depth and stencil -- this violates the glX specification, but it makes 
> glXChooseFBConfig() return a list that starts with a config that is 
> compatible with the ctx (KotOR uses the first config from the list)
>
> 3. in gallium/state_trackers/dri/dri_screen.c dri_fill_in_modes(): 
> disable all the modes that are not z24s8 -- this is probably the worst 
> option, because z32 must also be disabled or it gets chosen over z24s8 
> (BTW is it really necessary to advertise 600 fbconfigs? fglrx only has 
> about 50)
>
> I just filed bugreport #93955 with a test code about the first issue 
> mentioned in 00/11; that test code also demonstrates this issue (see 
> the comments in the code).
>
> MM

I started working on option 1, as I think that one is a progressive 
approach, and it doesn't violate specs. I found that the only legit user 
of gl_context::Visual was the compatibility check, and it was marked for 
removal by Brian Paul in 2007 (the last time it was touched). How does 
the attached patch look like? I'd like to replace the current 05/11 with 
this one in v2 of my series.

MM
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mesa_remove_ctxvisual.patch
Type: text/x-patch
Size: 9213 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20160203/aaa537c3/attachment-0001.bin>