[Piglit] [PATCH 10/10] MSAA/formats: fix testing A, L, LA, and I integer formats
Marek Olšák
maraeo at gmail.com
Mon Mar 17 08:00:24 PDT 2014
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.
>
>
>
>> +
>> + 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?
>
>
>> + 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.
Marek
More information about the Piglit
mailing list