[Mesa-dev] [PATCH] mesa: add missing RGB9_E5 format in _mesa_base_fbo_format

[Roland Scheidegger]

Am 25.01.2018 um 16:30 schrieb Michel Dänzer:
> On 2018-01-24 05:38 PM, Juan A. Suarez Romero wrote:
>> This fixes KHR-GL45.internalformat.renderbuffer.rgb9_e5.
>> ---
>>  src/mesa/main/fbobject.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
>> index d23916d1ad7..c72204e11a0 100644
>> --- a/src/mesa/main/fbobject.c
>> +++ b/src/mesa/main/fbobject.c
>> @@ -1976,6 +1976,9 @@ _mesa_base_fbo_format(const struct gl_context *ctx, GLenum internalFormat)
>>                 ctx->Extensions.ARB_texture_float) ||
>>                _mesa_is_gles3(ctx) /* EXT_color_buffer_float */ )
>>           ? GL_RGBA : 0;
>> +   case GL_RGB9_E5:
>> +      return (_mesa_is_desktop_gl(ctx) && ctx->Extensions.EXT_texture_shared_exponent)
>> +         ? GL_RGB: 0;
>>     case GL_ALPHA16F_ARB:
>>     case GL_ALPHA32F_ARB:
>>        return ctx->API == API_OPENGL_COMPAT &&
>>
> 
> Unfortunately, this broke the "spec at arb_internalformat_query2@samples
> and num_sample_counts pname checks" piglit tests with radeonsi and
> llvmpipe, see below.
> 
> Any idea what might need to be done in Gallium to fix this?
> 
> 
>     32 bit failing case: pname = GL_NUM_SAMPLE_COUNTS, target = GL_RENDERBUFFER, internalformat = GL_RGB9_E5, params[0] = (1,GL_TRUE), supported=1
>     32 bit failing case: pname = GL_NUM_SAMPLE_COUNTS, target = GL_TEXTURE_2D_MULTISAMPLE, internalformat = GL_RGB9_E5, params[0] = (1,GL_TRUE), supported=1
>     32 bit failing case: pname = GL_NUM_SAMPLE_COUNTS, target = GL_TEXTURE_2D_MULTISAMPLE_ARRAY, internalformat = GL_RGB9_E5, params[0] = (1,GL_TRUE), supported=1
>     64 bit failing case: pname = GL_NUM_SAMPLE_COUNTS, target = GL_RENDERBUFFER, internalformat = GL_RGB9_E5, params[0] = (1,GL_TRUE), supported=1
>     64 bit failing case: pname = GL_NUM_SAMPLE_COUNTS, target = GL_TEXTURE_2D_MULTISAMPLE, internalformat = GL_RGB9_E5, params[0] = (1,GL_TRUE), supported=1
>     64 bit failing case: pname = GL_NUM_SAMPLE_COUNTS, target = GL_TEXTURE_2D_MULTISAMPLE_ARRAY, internalformat = GL_RGB9_E5, params[0] = (1,GL_TRUE), supported=1
> PIGLIT: {"subtest": {"GL_NUM_SAMPLE_COUNTS" : "fail"}}
>     32 bit failing case: pname = GL_SAMPLES, target = GL_RENDERBUFFER, internalformat = GL_RGB9_E5, params[0] = (1,GL_TRUE), supported=1
>     32 bit failing case: pname = GL_SAMPLES, target = GL_TEXTURE_2D_MULTISAMPLE, internalformat = GL_RGB9_E5, params[0] = (1,GL_TRUE), supported=1
>     32 bit failing case: pname = GL_SAMPLES, target = GL_TEXTURE_2D_MULTISAMPLE_ARRAY, internalformat = GL_RGB9_E5, params[0] = (1,GL_TRUE), supported=1
>     64 bit failing case: pname = GL_SAMPLES, target = GL_RENDERBUFFER, internalformat = GL_RGB9_E5, params[0] = (1,GL_TRUE), supported=1
>     64 bit failing case: pname = GL_SAMPLES, target = GL_TEXTURE_2D_MULTISAMPLE, internalformat = GL_RGB9_E5, params[0] = (1,GL_TRUE), supported=1
>     64 bit failing case: pname = GL_SAMPLES, target = GL_TEXTURE_2D_MULTISAMPLE_ARRAY, internalformat = GL_RGB9_E5, params[0] = (1,GL_TRUE), supported=1
> PIGLIT: {"subtest": {"GL_SAMPLES" : "fail"}}
> PIGLIT: {"result": "fail" }
> 
> 

Purely coincidentally, I was trying to clean up the formatquery code
recently (should help some failures with r600 too), and I think these
cleanups would fix it.
Basically outright say "no" to target/pname combinations which don't
make sense rather than trying to find a format suitable for another
target and then asking the driver for the nonsense combination, plus
some other small bits like not validating things again (sometimes, a
third time...).
Albeit it will cause some breakage with the piglit test, which I believe
is a test error, but that might be open for debate...
(For TEXTURE_BUFFER and the internalformat size/type queries, do you
return valid values or unsupported? The problem here is ARB_tbo says you
can't get these values via the equivalent GetTexLevelParameter queries,
whereas with GL 3.1 you can. And internalformat_query2 says it returns
"the same information" as GetTexLevelParameter, albeit it's not entirely
true in any case since the equivalent of the internalformat stencil type
doesn't even exist. My stance would be that valid values should be
reported even without GL 3.1, but the piglit test thinks differently.)

Roland
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mesa_formatquery.diff
Type: text/x-patch
Size: 3868 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180125/8928c70e/attachment-0001.bin>