[Piglit] [PATCH 10/10] MSAA/formats: fix testing A, L, LA, and I integer formats
Brian Paul
brianp at vmware.com
Mon Mar 17 08:06:23 PDT 2014
On 03/17/2014 09:00 AM, Marek Olšák wrote:
> On Mon, Mar 17, 2014 at 3:41 PM, Brian Paul <brianp at vmware.com> wrote:
>> On 03/15/2014 10:53 AM, Marek Olšák wrote:
>>>
>>> From: Marek Olšák <marek.olsak at amd.com>
>>>
>>> ---
>>> tests/spec/ext_framebuffer_multisample/formats.cpp | 14 +++-
>>> tests/util/piglit-util-gl-common.h | 1 +
>>> tests/util/piglit-util-gl.c | 87
>>> ++++++++++++++++++++++
>>> 3 files changed, 100 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tests/spec/ext_framebuffer_multisample/formats.cpp
>>> b/tests/spec/ext_framebuffer_multisample/formats.cpp
>>> index 3694302..7a54851 100644
>>> --- a/tests/spec/ext_framebuffer_multisample/formats.cpp
>>> +++ b/tests/spec/ext_framebuffer_multisample/formats.cpp
>>> @@ -168,18 +168,28 @@ PatternRenderer::try_setup(GLenum internalformat)
>>> GL_FRAMEBUFFER_ATTACHMENT_COMPONENT_TYPE,
>>> (GLint *) &component_type);
>>>
>>> + piglit_get_luminance_intensity_bits(internalformat, color_bits);
>>
>>
>> Do you want to check the return value here to make sure that internalformat
>> is handled?
>
> No. The function only handles formats not supported by the framebuffer
> queries. Using the framebuffer queries and then calling the function
> without checking the return value should give you reasonable values
> for any format.
OK, I didn't look at the existing code so I didn't see the preceding
glGetFramebufferAttachmentParameteriv() calls. A comment about this
might be helpful though.
>>> +
>>> + int num_bits = 0;
>>> + for (int i = 0; i < 4 && !num_bits; i++)
>>> + num_bits = color_bits[i];
>>> + if (!num_bits) {
>>> + printf("Red, green, blue, and alpha sizes are all
>>> zero.\n");
>>> + return false;
>>> + }
>>> +
>>> color_clamping_mode = GL_FIXED_ONLY;
>>> switch (component_type) {
>>> case GL_INT:
>>> assert(test_pattern_ivec4);
>>> test_pattern = test_pattern_ivec4;
>>> - color_offset = 1.0 - pow(2.0, color_bits[0] - 1);
>>> + color_offset = 1.0 - pow(2.0, num_bits - 1);
>>> color_scale = -2.0 * color_offset;
>>> break;
>>> case GL_UNSIGNED_INT:
>>> assert(test_pattern_uvec4);
>>> test_pattern = test_pattern_uvec4;
>>> - color_scale = pow(2.0, color_bits[0]) - 1.0;
>>> + color_scale = pow(2.0, num_bits) - 1.0;
>>> color_offset = 0.0;
>>> break;
>>> case GL_UNSIGNED_NORMALIZED:
>>> diff --git a/tests/util/piglit-util-gl-common.h
>>> b/tests/util/piglit-util-gl-common.h
>>> index 73f2f83..ffa3781 100644
>>> --- a/tests/util/piglit-util-gl-common.h
>>> +++ b/tests/util/piglit-util-gl-common.h
>>> @@ -121,6 +121,7 @@ void piglit_require_gl_version(int
>>> required_version_times_10);
>>> void piglit_require_extension(const char *name);
>>> void piglit_require_not_extension(const char *name);
>>> unsigned piglit_num_components(GLenum base_format);
>>> +bool piglit_get_luminance_intensity_bits(GLenum internalformat, int
>>> *bits);
>>> int piglit_probe_pixel_rgb_silent(int x, int y, const float* expected,
>>> float *out_probe);
>>> int piglit_probe_pixel_rgba_silent(int x, int y, const float* expected,
>>> float *out_probe);
>>> int piglit_probe_pixel_rgb(int x, int y, const float* expected);
>>> diff --git a/tests/util/piglit-util-gl.c b/tests/util/piglit-util-gl.c
>>> index 3bb4adf..4f38336 100644
>>> --- a/tests/util/piglit-util-gl.c
>>> +++ b/tests/util/piglit-util-gl.c
>>> @@ -64,6 +64,93 @@ piglit_num_components(GLenum base_format)
>>> }
>>> }
>>>
>>> +/* OpenGL lacks luminace and intensity bits queries for framebuffers
>>> + * and renderbuffers. */
>>
>>
>> You might beef up the comment to say something like the returned number of
>> bits is an approximation but should be no less than the actual number of
>> bits for the format chosen by OpenGL.
>>
>>
>>
>>> +bool
>>> +piglit_get_luminance_intensity_bits(GLenum internalformat, int *bits)
>>> +{
>>> + switch (internalformat) {
>>> + case GL_LUMINANCE4:
>>> + bits[0] = 4; bits[1] = 4; bits[2] = 4; bits[3] = 0;
>>> + return true;
>>> +
>>> + case GL_LUMINANCE:
>>> + case GL_LUMINANCE_SNORM:
>>> + case GL_LUMINANCE8:
>>> + case GL_LUMINANCE8_SNORM:
>>> + case GL_LUMINANCE8I_EXT:
>>> + case GL_LUMINANCE8UI_EXT:
>>> + bits[0] = 8; bits[1] = 8; bits[2] = 8; bits[3] = 0;
>>> + return true;
>>> +
>>> + case GL_LUMINANCE12:
>>> + bits[0] = 12; bits[1] = 12; bits[2] = 12; bits[3] = 0;
>>> + return true;
>>> +
>>> + case GL_LUMINANCE16:
>>> + case GL_LUMINANCE16_SNORM:
>>> + case GL_LUMINANCE16I_EXT:
>>> + case GL_LUMINANCE16UI_EXT:
>>> + case GL_LUMINANCE16F_ARB:
>>> + bits[0] = 16; bits[1] = 16; bits[2] = 16; bits[3] = 0;
>>> + return true;
>>> +
>>> + case GL_LUMINANCE32I_EXT:
>>> + case GL_LUMINANCE32UI_EXT:
>>> + case GL_LUMINANCE32F_ARB:
>>> + bits[0] = 32; bits[1] = 32; bits[2] = 32; bits[3] = 0;
>>> + return true;
>>> +
>>> + case GL_LUMINANCE4_ALPHA4:
>>> + case GL_INTENSITY4:
>>> + bits[0] = 4; bits[1] = 4; bits[2] = 4; bits[3] = 4;
>>
>>
>> Minor nit, I'd write lines like that as:
>
> As what?
bits[0] = bits[1] = bits[2] = bits[3] = 4;
>
>>
>>
>>> + return true;
>>> +
>>> + case GL_LUMINANCE_ALPHA:
>>> + case GL_LUMINANCE_ALPHA_SNORM:
>>> + case GL_LUMINANCE8_ALPHA8:
>>> + case GL_LUMINANCE8_ALPHA8_SNORM:
>>> + case GL_LUMINANCE_ALPHA8I_EXT:
>>> + case GL_LUMINANCE_ALPHA8UI_EXT:
>>> + case GL_INTENSITY:
>>> + case GL_INTENSITY_SNORM:
>>> + case GL_INTENSITY8:
>>> + case GL_INTENSITY8_SNORM:
>>> + case GL_INTENSITY8I_EXT:
>>> + case GL_INTENSITY8UI_EXT:
>>> + bits[0] = 8; bits[1] = 8; bits[2] = 8; bits[3] = 8;
>>> + return true;
>>> +
>>> + case GL_LUMINANCE12_ALPHA12:
>>> + case GL_INTENSITY12:
>>> + bits[0] = 12; bits[1] = 12; bits[2] = 12; bits[3] = 12;
>>> + return true;
>>> +
>>> + case GL_LUMINANCE16_ALPHA16:
>>> + case GL_LUMINANCE16_ALPHA16_SNORM:
>>> + case GL_LUMINANCE_ALPHA16I_EXT:
>>> + case GL_LUMINANCE_ALPHA16UI_EXT:
>>> + case GL_LUMINANCE_ALPHA16F_ARB:
>>> + case GL_INTENSITY16:
>>> + case GL_INTENSITY16_SNORM:
>>> + case GL_INTENSITY16I_EXT:
>>> + case GL_INTENSITY16UI_EXT:
>>> + case GL_INTENSITY16F_ARB:
>>> + bits[0] = 16; bits[1] = 16; bits[2] = 16; bits[3] = 16;
>>> + return true;
>>> +
>>> + case GL_LUMINANCE_ALPHA32I_EXT:
>>> + case GL_LUMINANCE_ALPHA32UI_EXT:
>>> + case GL_LUMINANCE_ALPHA32F_ARB:
>>> + case GL_INTENSITY32I_EXT:
>>> + case GL_INTENSITY32UI_EXT:
>>> + case GL_INTENSITY32F_ARB:
>>> + bits[0] = 32; bits[1] = 32; bits[2] = 32; bits[3] = 32;
>>> + return true;
>>> + }
>>
>>
>> Do you want to set bits[0..3] = 0 here just to be safe?
>
> See the reason I said above. This should be used in combination with
> the framebuffer queries, therefore the function shouldn't change the
> bits if it gets a format it doesn't handle.
OK, it might be good to have a comment on the function explaining this
for the next person who might want to use it.
-Brian
More information about the Piglit
mailing list