[Mesa-dev] [PATCH] mesa/teximage: Fix ASTC-caused S3TC regression

[Roland Scheidegger]

Am 28.10.2015 um 20:48 schrieb Ian Romanick:
> On 10/28/2015 09:52 AM, Nanley Chery wrote:
>> From: Nanley Chery <nanley.g.chery at intel.com>
>>
>> The ASTC spec forbids other compressed formats from being used against
>> the targets: TEXTURE_CUBE_MAP_ARRAY and TEXTURE_3D. Because other
> 
> I apologize for not paying very much attention to this previously.
> 
> I think we're taking too literal of a reading of the spec.  The ASTC
> extension extends the core OpenGL ES and desktop OpenGL specifications.
>  It does not interact with other extensions that are not part of the
> core.  I see no interactions in the ASTC specification with any S3TC
> spec, GL_3DFX_texture_compression_FXT1, GL_EXT_texture_compression_latc,
> or GL_ATI_texture_compression_3dc.  As a result, the existence of ASTC
> should have *no* affect on what is allowed with those compression formats.
> 
> Judging from table 8.17 in the OpenGL ES 3.2 spec, I think the only
> formats that should ever be disallowed for cubemap arrays is the ETC
> formats.  From reading the errors in section 8.5.3 (Texture Image
> Structure) of the OpenGL 4.5 spec, even that limitation should only
> apply to OpenGL ES.
Well, that's not _quite_ true. There's this bit from the 8.5.3 section
for instance (there's more in the compressed section): "An
INVALID_OPERATION error is generated by TexImage3D if internal-
format is one of the EAC, ETC2, or RGTC compressed formats and either
border is non-zero, or target is not TEXTURE_2D_ARRAY." Which makes it
impossible to use cube map array targets.
I filed bugs to fix this in the spec (my stance is clear: everything
saying something which results in specific compressed formats not being
allowed for cube map array targets is a spec bug, both in GL and GLES -
all these formats are allowed for both 2d array and cube map targets,
therefore they must be allowed in cube map array targets, the specific
compression specification not saying that is a result of these specs
being developed without cube map arrays being available, not excluding
them on purpose). Albeit the situation isn't the same for 3D targets,
because they clearly were made not available for 3d targets on purpose
(which btw is unfortunate as some other apis allow this, but probably
some hw really couldn't do it at that time...).
And at least for GLES, everybody seems to agree (for GL, noone seems to
care much what the spec says, there was no feedback, all the blobs
ignore this bit certainly already).
These are the relevant bugs:
https://cvs.khronos.org/bugzilla/show_bug.cgi?id=14713
https://cvs.khronos.org/bugzilla/show_bug.cgi?id=14366

Roland


> 
>> compressed formats were previously supported for these targets in
>> desktop GL, it's considered to be a spec bug and Mesa will diverge from
>> this behavior for desktop GL.
>>
>> Fixes the following Piglit tests on Gen9:
>> piglit.spec.arb_direct_state_access.getcompressedtextureimage
>> piglit.spec.arb_get_texture_sub_image.arb_get_texture_sub_image-getcompressed
>> piglit.spec.arb_texture_cube_map_array.fbo-generatemipmap-cubemap array s3tc_dxt1
>> piglit.spec.ext_texture_compression_s3tc.getteximage-targets cube_array s3tc
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91927
>> Suggested-by: Neil Roberts <neil at linux.intel.com>
>> Signed-off-by: Nanley Chery <nanley.g.chery at intel.com>
>> ---
>>  src/mesa/main/teximage.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
>> index d9453e3..eccf009 100644
>> --- a/src/mesa/main/teximage.c
>> +++ b/src/mesa/main/teximage.c
>> @@ -1373,7 +1373,8 @@ _mesa_target_can_be_compressed(const struct gl_context *ctx, GLenum target,
>>        /* Throw an INVALID_OPERATION error if the target is
>>         * TEXTURE_CUBE_MAP_ARRAY and the format is not ASTC.
>>         */
>> -      if (target_can_be_compresed &&
>> +      /* FIXME: Correct the spec to restrict this behavior to GLES as well. */
>> +      if (target_can_be_compresed && _mesa_is_gles(ctx) &&
>>            ctx->Extensions.KHR_texture_compression_astc_ldr &&
>>            layout != MESA_FORMAT_LAYOUT_ASTC)
>>           return write_error(error, GL_INVALID_OPERATION);
>> @@ -1405,7 +1406,9 @@ _mesa_target_can_be_compressed(const struct gl_context *ctx, GLenum target,
>>            * the format is not ASTC.
>>            * See comment in switch case GL_TEXTURE_CUBE_MAP_ARRAY for more info.
> 
> I'm a bit confused about the comment above and the strangeness with the
> error codes.  I'll dig a bit into the specs.
> 
>>            */
>> -         if (ctx->Extensions.KHR_texture_compression_astc_ldr)
>> +         /* FIXME: Correct the spec to restrict this behavior to GLES as well. */
>> +         if (_mesa_is_gles(ctx) &&
>> +             ctx->Extensions.KHR_texture_compression_astc_ldr)
>>              return write_error(error, GL_INVALID_OPERATION);
>>           break;
>>        }
>>
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>