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@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
diff --git a/src/mesa/main/formatquery.c b/src/mesa/main/formatquery.c
index 61f798c88f..3f5da272c3 100644
--- a/src/mesa/main/formatquery.c
+++ b/src/mesa/main/formatquery.c
@@ -398,8 +398,6 @@ _is_target_supported(struct gl_context *ctx, GLenum target)
case GL_TEXTURE_1D:
case GL_TEXTURE_2D:
case GL_TEXTURE_3D:
- if (!_mesa_is_desktop_gl(ctx))
- return false;
break;
case GL_TEXTURE_1D_ARRAY:
@@ -560,15 +558,29 @@ _is_internalformat_supported(struct gl_context *ctx, GLenum target,
* implementation accepts it for any texture specification commands, and
* - unsized or base internal format, if the implementation accepts
* it for texture or image specification.
+ *
+ * But also:
+ * "If the particualar <target> and <internalformat> combination do not make
+ * sense, or if a particular type of <target> is not supported by the
+ * implementation the "unsupported" answer should be given. This is not an
+ * error.
*/
GLint buffer[1];
- /* At this point an internalformat is valid if it is valid as a texture or
- * as a renderbuffer format. The checks are different because those methods
- * return different values when passing non supported internalformats */
- if (_mesa_base_tex_format(ctx, internalformat) < 0 &&
- _mesa_base_fbo_format(ctx, internalformat) == 0)
- return false;
+ if (target == GL_RENDERBUFFER) {
+ if (_mesa_base_fbo_format(ctx, internalformat) == 0) {
+ return false;
+ }
+ } else if (target == GL_TEXTURE_BUFFER) {
+ if (_mesa_validate_texbuffer_format(ctx, internalformat) ==
+ MESA_FORMAT_NONE) {
+ return false;
+ }
+ } else {
+ if (_mesa_base_tex_format(ctx, internalformat) < 0) {
+ return false;
+ }
+ }
/* Let the driver have the final word */
ctx->Driver.QueryInternalFormat(ctx, target, internalformat,
@@ -702,6 +714,12 @@ _mesa_query_internal_format_default(struct gl_context *ctx, GLenum target,
case GL_FRAMEBUFFER_RENDERABLE_LAYERED:
case GL_FRAMEBUFFER_BLEND:
case GL_FILTER:
+ /*
+ * XXX seems a tad optimistic just saying yes to everything here.
+ * Even for combinations which make no sense...
+ * And things like TESS_CONTROL_TEXTURE should definitely default to
+ * NONE if the driver doesn't even support tessellation...
+ */
params[0] = GL_FULL_SUPPORT;
break;
case GL_NUM_TILING_TYPES_EXT:
@@ -942,9 +960,12 @@ _mesa_GetInternalformativ(GLenum target, GLenum internalformat, GLenum pname,
mesa_format texformat;
if (target != GL_RENDERBUFFER) {
- if (!_mesa_legal_get_tex_level_parameter_target(ctx, target, true))
- goto end;
-
+ /* XXX there was a _mesa_legal_get_tex_level_parameter_target call here.
+ * This makes a difference for TEXTURE_BUFFER with just ARB_tbo but
+ * not GL 3.1 and will break the piglit test in this case. But my
+ * reading of the spec would be that we should not refuse this here,
+ * despite GetTexLevelParameter refusing it.
+ */
baseformat = _mesa_base_tex_format(ctx, internalformat);
} else {
baseformat = _mesa_base_fbo_format(ctx, internalformat);
@@ -965,10 +986,7 @@ _mesa_GetInternalformativ(GLenum target, GLenum internalformat, GLenum pname,
* and glGetRenderbufferParameteriv functions.
*/
if (pname == GL_INTERNALFORMAT_SHARED_SIZE) {
- if (_mesa_has_EXT_texture_shared_exponent(ctx) &&
- target != GL_TEXTURE_BUFFER &&
- target != GL_RENDERBUFFER &&
- texformat == MESA_FORMAT_R9G9B9E5_FLOAT) {
+ if (texformat == MESA_FORMAT_R9G9B9E5_FLOAT) {
buffer[0] = 5;
}
goto end;
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
