[Mesa-dev] [PATCH 1/2] gallium/st: Clean up Haiku depth mapping, fix colorspace errors

Roland Scheidegger sroland at vmware.com
Mon Dec 29 14:55:23 PST 2014


Am 27.12.2014 um 18:41 schrieb Ilia Mirkin:
> On Sat, Dec 27, 2014 at 1:13 AM, Alexander von Gluck IV
> <kallisti5 at unixzen.com> wrote:
>> ---
>>  src/gallium/state_trackers/hgl/hgl.c |   48 +++++++++++++--------------------
>>  1 files changed, 19 insertions(+), 29 deletions(-)
>>
>> diff --git a/src/gallium/state_trackers/hgl/hgl.c b/src/gallium/state_trackers/hgl/hgl.c
>> index 4d7c479..0b30290 100644
>> --- a/src/gallium/state_trackers/hgl/hgl.c
>> +++ b/src/gallium/state_trackers/hgl/hgl.c
>> @@ -232,9 +232,10 @@ hgl_create_st_visual(ulong options)
>>         const GLboolean alphaFlag   = ((options & BGL_ALPHA) == BGL_ALPHA);
>>         const GLboolean dblFlag     = ((options & BGL_DOUBLE) == BGL_DOUBLE);
>>         const GLboolean stereoFlag  = false;
>> -       const GLint depth           = (options & BGL_DEPTH) ? 24 : 0;
>> -       const GLint stencil         = (options & BGL_STENCIL) ? 8 : 0;
>> -       const GLint accum           = (options & BGL_ACCUM) ? 16 : 0;
>> +       const GLboolean depthFlag   = ((options & BGL_DEPTH) == BGL_DEPTH);
>> +       const GLboolean stencilFlag = ((options & BGL_STENCIL) == BGL_STENCIL);
>> +       const GLboolean accumFlag   = ((options & BGL_ACCUM) == BGL_ACCUM);
>> +
>>         const GLint red             = rgbFlag ? 8 : 5;
>>         const GLint green           = rgbFlag ? 8 : 5;
>>         const GLint blue            = rgbFlag ? 8 : 5;
>> @@ -244,9 +245,9 @@ hgl_create_st_visual(ulong options)
>>         TRACE("alpha    :\t%d\n", (bool)alphaFlag);
>>         TRACE("dbl      :\t%d\n", (bool)dblFlag);
>>         TRACE("stereo   :\t%d\n", (bool)stereoFlag);
>> -       TRACE("depth    :\t%d\n", depth);
>> -       TRACE("stencil  :\t%d\n", stencil);
>> -       TRACE("accum    :\t%d\n", accum);
>> +       TRACE("depth    :\t%d\n", (bool)depthFlag);
>> +       TRACE("stencil  :\t%d\n", (bool)stencilFlag);
>> +       TRACE("accum    :\t%d\n", (bool)accumFlag);
>>         TRACE("red      :\t%d\n", red);
>>         TRACE("green    :\t%d\n", green);
>>         TRACE("blue     :\t%d\n", blue);
>> @@ -254,34 +255,23 @@ hgl_create_st_visual(ulong options)
>>
>>         // Determine color format
>>         if (red == 8) {
>> +               // Color format
>>                 if (alpha == 8)
>> -                       visual->color_format = PIPE_FORMAT_A8R8G8B8_UNORM;
>> +                       visual->color_format = PIPE_FORMAT_B8G8R8A8_UNORM;
>>                 else
>> -                       visual->color_format = PIPE_FORMAT_X8R8G8B8_UNORM;
>> +                       visual->color_format = PIPE_FORMAT_B8G8R8X8_UNORM;
>> +
>> +               // Depth buffer
>> +               if (depthFlag)
>> +                       visual->depth_stencil_format = PIPE_FORMAT_Z32_UNORM;
> 
> I guess you only work with llvmpipe which supports whatever, but I
> don't think a lot of hw drivers support Z32_UNORM. Z24 is much more
> common. Some hardware also supports Z16 and Z32_FLOAT (and
> Z32_FLOAT_S8X24_UNORM for depth/stencil combined version).
FWIW llvmpipe (and softpipe) do not really support z32_unorm neither, it
should
never be used. (The reason is just like hw most things are done with
floats, so you've got only 24bit mantissa bits to work with really). So,
while it may work, the precision is probably not what you expected and
if you rely on some specific accuracy (for instance for depth offsets)
the results may be somewhat bogus. We actually wanted to drop the format
entirely at some point, could still do it at some point. Similar things
can be said about the other 32bit snorm/unorm formats though IIRC these
are sort of necessary for supporting gl vertex attribs which can be
32bit normalized, but they should not be needed as render target /
texture support.
So indeed z24 (with or without stencil), z16_unorm if that's good enough
or z32_float should be used.

> Further you appear to have dropped the stencil format here entirely.
> If that's expected, perhaps get rid of the stencilFlag above?
> 
>> +               else
>> +                       visual->depth_stencil_format = PIPE_FORMAT_NONE;
>>         } else {
>> -               // TODO: I think this should be RGB vs BGR
>>                 visual->color_format = PIPE_FORMAT_B5G6R5_UNORM;
>> -    }
>>
>> -       // Determine depth stencil format
>> -       switch (depth) {
>> -               default:
>> -               case 0:
>> -                       visual->depth_stencil_format = PIPE_FORMAT_NONE;
>> -                       break;
>> -               case 16:
>> -                       visual->depth_stencil_format = PIPE_FORMAT_Z16_UNORM;
>> -                       break;
>> -               case 24:
>> -                       if ((options & BGL_STENCIL) != 0)
>> -                               visual->depth_stencil_format = PIPE_FORMAT_S8_UINT_Z24_UNORM;
>> -                       else
>> -                               visual->depth_stencil_format = PIPE_FORMAT_X8Z24_UNORM;
>> -                       break;
>> -               case 32:
>> -                       visual->depth_stencil_format = PIPE_FORMAT_Z32_UNORM;
>> -                       break;
>> -       }
>> +               // TODO: Indexed color depth buffer?
>> +               visual->depth_stencil_format = PIPE_FORMAT_NONE;
>> +    }
>>
>>         visual->accum_format = (options & BGL_ACCUM)
>>                 ? PIPE_FORMAT_R16G16B16A16_SNORM : PIPE_FORMAT_NONE;
>> --
>> 1.7.1
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Vjtt0vs_iqoI31UfJxBl7yv9I2FeiaeAYgMTLKRBc_I&m=GQBq-h7Jh1_X3J6zM9xSfKUdoAsXlJ2NgvVD7uWLVsw&s=eZUFlxs8isJiKIGlUt9cbL9-j9270B0EGoVpIKrjIDY&e= 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://urldefense.proofpoint.com/v2/url?u=http-3A__lists.freedesktop.org_mailman_listinfo_mesa-2Ddev&d=AwIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Vjtt0vs_iqoI31UfJxBl7yv9I2FeiaeAYgMTLKRBc_I&m=GQBq-h7Jh1_X3J6zM9xSfKUdoAsXlJ2NgvVD7uWLVsw&s=eZUFlxs8isJiKIGlUt9cbL9-j9270B0EGoVpIKrjIDY&e= 
> 



More information about the mesa-dev mailing list