On Fri, Mar 7, 2014 at 2:04 AM, Jose Fonseca <jfons...@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, jfons...@vmware.com wrote:
>> >>>> From: José Fonseca <jfons...@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@entry=0x7fa324df2ed7 "0",
>> >>>> file=file@entry=0x7fa324e3fc30 "src/mesa/state_tracker/st_format.c",
>> >>>> line=line@entry=758, function=function@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@entry=0x7fa324df2ed7 "0",
>> >>>> file=file@entry=0x7fa324e3fc30 "src/mesa/state_tracker/st_format.c",
>> >>>> line=line@entry=758, function=function@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@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@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@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@entry=0x19dfce0,
>> >>>> fb=<optimised out>, stfbi=stfbi@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@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 <bri...@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.
>
> Jose
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
--
o...@lunarg.com
From a600bf561a9c3d5776fd60d3ae3d14bd4ab417e6 Mon Sep 17 00:00:00 2001
From: Chia-I Wu <olva...@gmail.com>
Date: Fri, 7 Mar 2014 11:39:24 +0800
Subject: [PATCH] st/mesa: avoid sRGB format without a matching mesa format
Calling util_format_srgb() blindly may return a pipe format that has no
matching mesa format. It will cause an assertion failure in
st_pipe_format_to_mesa_format(), and probably other mysterious behaviors in a
release build.
---
src/mesa/state_tracker/st_manager.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/src/mesa/state_tracker/st_manager.c b/src/mesa/state_tracker/st_manager.c
index 68cb5de..5219504 100644
--- a/src/mesa/state_tracker/st_manager.c
+++ b/src/mesa/state_tracker/st_manager.c
@@ -438,8 +438,22 @@ st_framebuffer_create(struct st_context *st,
*/
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);
+ enum pipe_format srgb_format;
+
+ /*
+ * We could not blindly call util_format_srgb() because the returned
+ * format may not be recognized by mesa core.
+ */
+ switch (stfbi->visual->color_format) {
+ case PIPE_FORMAT_B8G8R8A8_UNORM:
+ case PIPE_FORMAT_R8G8B8X8_UNORM:
+ case PIPE_FORMAT_B8G8R8X8_UNORM:
+ srgb_format = util_format_srgb(stfbi->visual->color_format);
+ break;
+ default:
+ srgb_format = PIPE_FORMAT_NONE;
+ break;
+ }
if (srgb_format != PIPE_FORMAT_NONE &&
screen->is_format_supported(screen, srgb_format,
--
1.8.5.3
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
