[Mesa-dev] [PATCH] st/mesa: Add R8G8B8A8_SRGB case to st_pipe_format_to_mesa_format.
Roland Scheidegger
sroland at vmware.com
Thu Mar 6 09:55:45 PST 2014
Am 06.03.2014 18:32, schrieb Jose Fonseca:
>
>
> ----- Original Message -----
>>
>>
>> ----- Original Message -----
>>> On 03/06/2014 09:59 AM, jfonseca at vmware.com wrote:
>>>> From: José Fonseca <jfonseca at vmware.com>
>>>>
>>>> With the recent SRGB changes all my automated OpenGL llvmpipe tests
>>>> (piglit, conform, glretrace) start asserting with the backtrace below.
>>>>
>>>> I'm hoping this change will fix it. I'm not entirely sure, as this
>>>> doesn't happen in my development machine (the bug probably depends on
>>>> the exact X visual).
>>>>
>>>> Anyway, it seems the sensible thing to do here.
>>>>
>>>> Program terminated with signal 5, Trace/breakpoint trap.
>>>> #0 _debug_assert_fail (expr=expr at entry=0x7fa324df2ed7 "0",
>>>> file=file at entry=0x7fa324e3fc30 "src/mesa/state_tracker/st_format.c",
>>>> line=line at entry=758, function=function at entry=0x7fa324e40160
>>>> <__func__.34798> "st_pipe_format_to_mesa_format") at
>>>> src/gallium/auxiliary/util/u_debug.c:281
>>>> #0 _debug_assert_fail (expr=expr at entry=0x7fa324df2ed7 "0",
>>>> file=file at entry=0x7fa324e3fc30 "src/mesa/state_tracker/st_format.c",
>>>> line=line at entry=758, function=function at entry=0x7fa324e40160
>>>> <__func__.34798> "st_pipe_format_to_mesa_format") at
>>>> src/gallium/auxiliary/util/u_debug.c:281
>>>> No locals.
>>>> #1 0x00007fa3241d22b3 in st_pipe_format_to_mesa_format
>>>> (format=format at entry=PIPE_FORMAT_R8G8B8A8_SRGB) at
>>>> src/mesa/state_tracker/st_format.c:758
>>>> __func__ = "st_pipe_format_to_mesa_format"
>>>> #2 0x00007fa3241c8ec5 in st_new_renderbuffer_fb
>>>> (format=format at entry=PIPE_FORMAT_R8G8B8A8_SRGB, samples=0,
>>>> sw=<optimised out>) at src/mesa/state_tracker/st_cb_fbo.c:295
>>>> strb = 0x19e8420
>>>> #3 0x00007fa32409d355 in st_framebuffer_add_renderbuffer
>>>> (stfb=stfb at entry=0x19e7fa0, idx=<optimised out>) at
>>>> src/mesa/state_tracker/st_manager.c:314
>>>> rb = <optimised out>
>>>> format = PIPE_FORMAT_R8G8B8A8_SRGB
>>>> sw = <optimised out>
>>>> #4 0x00007fa32409e635 in st_framebuffer_create (st=0x19e7fa0,
>>>> st=0x19e7fa0, stfbi=0x19e7a30) at
>>>> src/mesa/state_tracker/st_manager.c:458
>>>> stfb = 0x19e7fa0
>>>> mode = {rgbMode = 1 '\001', floatMode = 0 '\000',
>>>> colorIndexMode = 0 '\000', doubleBufferMode = 0, stereoMode =
>>>> 0, haveAccumBuffer = 0 '\000', haveDepthBuffer = 1 '\001',
>>>> haveStencilBuffer = 1 '\001', redBits = 8, greenBits = 8,
>>>> blueBits = 8, alphaBits = 8, redMask = 0, greenMask = 0,
>>>> blueMask = 0, alphaMask = 0, rgbBits = 32, indexBits = 0,
>>>> accumRedBits = 0, accumGreenBits = 0, accumBlueBits = 0,
>>>> accumAlphaBits = 0, depthBits = 24, stencilBits = 8,
>>>> numAuxBuffers = 0, level = 0, visualRating = 0,
>>>> transparentPixel = 0, transparentRed = 0, transparentGreen =
>>>> 0, transparentBlue = 0, transparentAlpha = 0,
>>>> transparentIndex
>>>> = 0, sampleBuffers = 0, samples = 0, maxPbufferWidth = 0,
>>>> maxPbufferHeight = 0, maxPbufferPixels = 0,
>>>> optimalPbufferWidth = 0, optimalPbufferHeight = 0, swapMethod
>>>> = 0, bindToTextureRgb = 0, bindToTextureRgba = 0,
>>>> bindToMipmapTexture = 0, bindToTextureTargets = 0, yInverted
>>>> =
>>>> 0, sRGBCapable = 1}
>>>> idx = <optimised out>
>>>> #5 st_framebuffer_reuse_or_create (st=st at entry=0x19dfce0,
>>>> fb=<optimised out>, stfbi=stfbi at entry=0x19e7a30) at
>>>> src/mesa/state_tracker/st_manager.c:728
>>>> No locals.
>>>> #6 0x00007fa32409e8cc in st_api_make_current (stapi=<optimised out>,
>>>> stctxi=0x19dfce0, stdrawi=0x19e7a30, streadi=0x19e7a30) at
>>>> src/mesa/state_tracker/st_manager.c:747
>>>> st = 0x19dfce0
>>>> stdraw = 0x640064
>>>> stread = 0x1300000006
>>>> ret = <optimised out>
>>>> #7 0x00007fa324074a20 in XMesaMakeCurrent2 (c=c at entry=0x195bb00,
>>>> drawBuffer=0x19e7e90, readBuffer=0x19e7e90) at
>>>> src/gallium/state_trackers/glx/xlib/xm_api.c:1194
>>>> No locals.
>>>> #8 0x00007fa3240783c8 in glXMakeContextCurrent (dpy=0x194e900,
>>>> draw=8388610, read=8388610, ctx=0x195bac0) at
>>>> src/gallium/state_trackers/glx/xlib/glx_api.c:1177
>>>> drawBuffer = <optimised out>
>>>> readBuffer = <optimised out>
>>>> xmctx = 0x195bb00
>>>> glxCtx = 0x195bac0
>>>> firsttime = 0 '\000'
>>>> no_rast = 0 '\000'
>>>> #9 0x00007fa32407852f in glXMakeCurrent (dpy=<optimised out>,
>>>> drawable=<optimised out>, ctx=<optimised out>) at
>>>> src/gallium/state_trackers/glx/xlib/glx_api.c:1211
>>>> No locals.
>>>> ---
>>>> src/mesa/state_tracker/st_format.c | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/src/mesa/state_tracker/st_format.c
>>>> b/src/mesa/state_tracker/st_format.c
>>>> index 25577ac..0311a2b 100644
>>>> --- a/src/mesa/state_tracker/st_format.c
>>>> +++ b/src/mesa/state_tracker/st_format.c
>>>> @@ -753,6 +753,8 @@ st_pipe_format_to_mesa_format(enum pipe_format
>>>> format)
>>>>
>>>> case PIPE_FORMAT_B8G8R8X8_SRGB:
>>>> return MESA_FORMAT_B8G8R8X8_SRGB;
>>>> + case PIPE_FORMAT_R8G8B8A8_SRGB:
>>>> + return MESA_FORMAT_B8G8R8A8_SRGB;
>>>
>>>
>>> Hmm, we don't have a MESA_FORMAT_R8G8B8A8_SRGB to match that pipe format
>>> so I'm not sure this is correct.
>>
>>
>>> But if this fixes the crash, it's
>>> better than nothing. Acked-by: Brian Paul <brianp at vmware.com>
>>
>> I'm hoping it does. I'm actually not sure as I couln't repro. I hope it
>> doesn't only moves the symptom away...
>
> I think you're right, the problem won't go completely away so soon.
>
> I think the problem is the st_manager.c changes in commit 4c68c6dcffe6c738d563eb0e0650bb865a5457b2 . In particular these lines:
>
> if (_mesa_is_desktop_gl(st->ctx)) {
> struct pipe_screen *screen = st->pipe->screen;
> const enum pipe_format srgb_format =
> util_format_srgb(stfbi->visual->color_format);
>
> if (srgb_format != PIPE_FORMAT_NONE &&
> screen->is_format_supported(screen, srgb_format,
> PIPE_TEXTURE_2D, stfbi->visual->samples,
> PIPE_BIND_RENDER_TARGET))
> mode.sRGBCapable = GL_TRUE;
> }
>
> We are adding the sRGBCapable visual and using SRGB format if the pipe drivers supports the SRGB variant, but we are not checking that the SRGB format variant actually exists in Mesa.
>
> I think we'll need to comment out this `mode.sRGBCapable = GL_TRUE` assignment until this is addressed.
>
Hmm yes that looks messy. I guess an option (other than adding this
format to mesa which probably would be a good idea) could be to ditch
the assert from st_pipe_format_to_mesa_format() (the reverse function
doesn't have it neither), then run that format through this function
here and don't set the sRGBCapable bit if the return value was
PIPE_FORMAT_NONE.
Though I still don't understand why conform suddenly needs srgb formats.
Roland
More information about the mesa-dev
mailing list