On 10/10/2012 12:00 PM, Brian Paul wrote:
> On 10/10/2012 12:27 PM, Chad Versace wrote:
>> On 10/08/2012 06:23 PM, Brian Paul wrote:
>>> On Mon, Oct 8, 2012 at 6:17 PM, Jordan Justen<[email protected]>
>>> wrote:
>>>> Signed-off-by: Jordan Justen<[email protected]>
>>>> Cc: Brian Paul<[email protected]>
>>>> Cc: Chad Versace<[email protected]>
>>>> ---
>>>> tests/util/piglit-util-gl-common.c | 7 +++++++
>>>> 1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/tests/util/piglit-util-gl-common.c
>>>> b/tests/util/piglit-util-gl-common.c
>>>> index 62b5312..4f9fe5f 100644
>>>> --- a/tests/util/piglit-util-gl-common.c
>>>> +++ b/tests/util/piglit-util-gl-common.c
>>>> @@ -447,8 +447,14 @@ piglit_get_compressed_block_size(GLenum format,
>>>> unsigned *bw, unsigned *bh, unsigned
>>>> *bytes)
>>>> {
>>>> switch (format) {
>>>> +#if defined(USE_OPENGL) || defined(USE_OPENGL_ES2)
>>>> case GL_COMPRESSED_RGB_S3TC_DXT1_EXT:
>>>> case GL_COMPRESSED_RGBA_S3TC_DXT1_EXT:
>>>> + *bw = *bh = 4;
>>>> + *bytes = 8;
>>>> + return true;
>>>> +#endif
>>>> +#if defined(USE_OPENGL)
>>>> case GL_COMPRESSED_SRGB_S3TC_DXT1_EXT:
>>>> case GL_COMPRESSED_SRGB_ALPHA_S3TC_DXT1_EXT:
>>>> case GL_COMPRESSED_RED_RGTC1:
>>>> @@ -475,6 +481,7 @@ piglit_get_compressed_block_size(GLenum format,
>>>> *bh = 4;
>>>> *bytes = 16;
>>>> return true;
>>>> +#endif
>>>> default:
>>>> /* return something rather than uninitialized values */
>>>> *bw = *bh = *bytes = 1;
>>>
>>> Actually, it would probably be better to test the extension #define,
>>> such as GL_EXT_texture_compression_s3tc because off-hand I don't know
>>> which compressed formats are supported by which APIs.
>>
>>
>> Brian,
>>
>> The GL headers on my system don't define the GL_EXT_texture_compression_s3tc
>> feature macro.
>>
>> Since the Piglit build is currently broken, this patch fixes it, and the
>> suggested alternative doesn't work on my system, I've gone ahead and
>> committed
>> the patch. If we later find a better method to filter the enums in this
>> switch,
>> we can apply the better method then.
>
> That's fine but I think I'm missing something.
>
> Aren't all the GL_(s3tc)_EXT tokens coming from the generated_dispatch.h
> file?
> It looks like that header also has a whole mess of "#define GL_EXT_foo_bar"
> lines. Shouldn't there be one for GL_EXT_texture_compression_s3tc also?
>
> Wouldn't everyone have the s3tc-related tokens defined in generated_dispatch.h
> like I do?
>
> AFAICT, generated_dispatch.[ch] are generated from the glapi.json file. And
> glapi.json is generated from enumext.spec and that file defines the extension
> tokens above. So I don't see where the variation in #defined tokens is coming
> from.
I might know where the confusion is coming from.
Support for ES1 and ES2 hasn't been added to piglit-dispatch yet. So,
generated_dispatch.h is only included when building for OpenGL. When building
for ES2, <GLES2/gl2.h> is directly included. Likewise for ES1. So, when building
the ES tests, many of these s3tc tokens are undefined.
Regarding the lack of the `#define GL_EXT_texture_compression_s3tc`...
I've checked both /usr/include/glext.h and generated_dispatch.h, and grep is
unable to find `#define GL_EXT_texture_compression_s3tc` in either. The only
s3tc tokens in those headers are for the texture formats.
I don't understand why generated_dispatch.h contains a whole mess of `#define
$EXT_FEATURE_MACRO` but lacks one for s3tc. Perhaps it special-cased this macro
in order to follow the precedent of glext.h?
Paul, ^^^, is there an explanation for this?
Given that there really is no feature macro for s3tc, then maybe the switch
statement should look something like this:
#ifdef GL_COMPRESSED_SRGB_S3TC_DXT1_EXT
case GL_COMPRESSED_SRGB_S3TC_DXT1_EXT:
#endif
#ifdef GL_COMPRESSED_RGBA_S3TC_DXT1_EXT:
case GL_COMPRESSED_SRGBA_S3TC_DXT1_EXT:
#endif
It's ugly, but it works around the problem of needing to know which formats are
supported by which API's. What do you think?
_______________________________________________
Piglit mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/piglit
