[Mesa-dev] [PATCH] st/mesa: Add R8G8B8A8_SRGB case to st_pipe_format_to_mesa_format.
Chia-I Wu
olvaffe at gmail.com
Thu Mar 6 20:47:26 PST 2014
On Fri, Mar 7, 2014 at 11:56 AM, Chia-I Wu <olvaffe at gmail.com> wrote:
> On Fri, Mar 7, 2014 at 2:04 AM, Jose Fonseca <jfonseca at vmware.com> wrote:
>>
>>
>> ----- Original Message -----
>>> 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.
>>
>> I wonder if it is a matter of luck: some visuals have srgb, others don't, conform doesn't specify, and gets one at random?
> Promoting a linear format to the corresponding sRGB format gives the
> framebuffer the capability for sRGB writes. Whether sRGB writes are
> enabled still depends on GL_FRAMEBUFFER_SRGB. But yeah, you will get
> a random value when querying the winsys fbo for
> GL_FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING (instead of always
> GL_LINEAR).
>
> How about the attached patch? It gives sRGB capability only to
> framebuffers with formats recognized by mesa core.
It looks like there is a workaround/fix for this specific format. To
avoid other breakages, there are more to fix.
Going over the possible return values of util_format_srgb(), there are
three formats
PIPE_FORMAT_A8R8G8B8_SRGB
PIPE_FORMAT_X8B8G8R8_SRGB
PIPE_FORMAT_X8R8G8B8_SRGB
that have no corresponding mesa formats. One solution is to add them,
and in st_framebuffer_create(), also add
assert(st_pipe_format_to_mesa_format(srgb_format) != MESA_FORMAT_NONE);
to make debugging simpler in the future.
Having the state tracker support all possible sRGB formats has the
benefit that GLX_ARB_framebuffer_sRGB or EGL_KHR_gl_colorspace will be
easier to implement. When the pipe driver supports
util_format_srgb(format), st/dri (or other state tracker managers) can
mark visuals with the format sRGB-capable without consulting st/mesa.
>
>>
>> Jose
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
>
> --
> olv at LunarG.com
--
olv at LunarG.com
More information about the mesa-dev
mailing list