[Piglit] [PATCH] arb_clear_texture: add a trivial test
Ilia Mirkin
imirkin at alum.mit.edu
Thu Mar 6 23:35:36 PST 2014
On Fri, Mar 7, 2014 at 2:26 AM, Fabian Bieler <fabianbieler at fastmail.fm> wrote:
> Sorry for the wrong In-reply-to-header. I can't use my email-client
> properly.
> On 2014-03-06 22:14, Ilia Mirkin wrote:
>>
>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>>
>> ---
>>
>> This is my first foray into piglit test-writing. Let me know if I missed
>> anything (except for better tests -- I know that this is very limited).
>>
>> This mechanism of transferring the texture to the screen is pretty poor
>> -- it
>> entirely defeats the point of ARB_clear_texture, which is to allow one to
>> clear even non-renderable-to textures. I guess I need to create a shader
>> that
>> uses the texture? Is there an example of a test that does this?
>>
>> For all its simplicity, this allowed me to debug my impl of this
>> extension on
>> nv50, which is now no longer completely broken.
>>
>> glapi/gl.spec | 41 +++++++++
>> tests/all.py | 4 +
>> tests/spec/CMakeLists.txt | 1 +
>> tests/spec/arb_clear_texture/CMakeLists.gl.txt | 13 +++
>> tests/spec/arb_clear_texture/CMakeLists.txt | 1 +
>> tests/spec/arb_clear_texture/clear-simple.c | 113
>> +++++++++++++++++++++++++
>> 6 files changed, 173 insertions(+)
>> create mode 100644 tests/spec/arb_clear_texture/CMakeLists.gl.txt
>> create mode 100644 tests/spec/arb_clear_texture/CMakeLists.txt
>> create mode 100644 tests/spec/arb_clear_texture/clear-simple.c
>>
>> diff --git a/glapi/gl.spec b/glapi/gl.spec
>> index a8fcc44..6a24f83 100644
>> --- a/glapi/gl.spec
>> +++ b/glapi/gl.spec
>> @@ -16410,6 +16410,47 @@ TextureStorage3DMultisampleEXT(texture, target,
>> samples, internalformat, width,
>> glxflags ignore
>> offset ?
>>
>>
>> +###############################################################################
>>
>> +#
>> +# Extension #ARB145
>> +# ARB_clear_texture commands
>> +#
>>
>> +###############################################################################
>>
>> +
>> +ClearTexImage(texture, level, format, type, data)
>> + return void
>> + param texture UInt32 in value
>> + param level Int32 in value
>> + param format GLenum in value
>> + param type GLenum in value
>> + param data Void in array [COMPSIZE(format/type)]
>> + category ARB_clear_texture
>> + version 4.4
>> + extension
>> + glxropcode ?
>> + glxflags ignore
>> + offset ?
>> +
>> +ClearTexSubImage(texture, level, xoffset, yoffset, zoffset, width,
>> height, depth, format, type, data)
>> + return void
>> + param texture UInt32 in value
>> + param level Int32 in value
>> + param xoffset Int32 in value
>> + param yoffset Int32 in value
>> + param zoffset Int32 in value
>> + param width Int32 in value
>> + param height Int32 in value
>> + param depth Int32 in value
>> + param format GLenum in value
>> + param type GLenum in value
>> + param data Void in array [COMPSIZE(format/type)]
>> + category ARB_clear_texture
>> + version 4.4
>> + extension
>> + glxropcode ?
>> + glxflags ignore
>> + offset ?
>> +
>>
>>
>>
>> ###############################################################################
>>
>>
>>
>> ###############################################################################
>>
>> diff --git a/tests/all.py b/tests/all.py
>> index d6daed2..8ed1d33 100644
>> --- a/tests/all.py
>> +++ b/tests/all.py
>> @@ -3075,6 +3075,10 @@ add_concurrent_test(arb_clear_buffer_object,
>> 'arb_clear_buffer_object-sub-overla
>> add_concurrent_test(arb_clear_buffer_object,
>> 'arb_clear_buffer_object-sub-simple')
>> add_concurrent_test(arb_clear_buffer_object,
>> 'arb_clear_buffer_object-zero-size')
>>
>> +arb_clear_texture = Group()
>> +spec['ARB_clear_texture'] = arb_clear_texture
>> +add_concurrent_test(arb_clear_texture, 'arb_clear_texture-clear-simple')
>> +
>> arb_copy_buffer = Group()
>> spec['ARB_copy_buffer'] = arb_copy_buffer
>> add_plain_test(arb_copy_buffer, 'copy_buffer_coherency')
>> diff --git a/tests/spec/CMakeLists.txt b/tests/spec/CMakeLists.txt
>> index 0a513c1..3ea6c1e 100644
>> --- a/tests/spec/CMakeLists.txt
>> +++ b/tests/spec/CMakeLists.txt
>> @@ -2,6 +2,7 @@ add_subdirectory (amd_performance_monitor)
>> add_subdirectory (arb_base_instance)
>> add_subdirectory (arb_buffer_storage)
>> add_subdirectory (arb_clear_buffer_object)
>> +add_subdirectory (arb_clear_texture)
>> add_subdirectory (arb_color_buffer_float)
>> add_subdirectory (arb_compute_shader)
>> add_subdirectory (arb_debug_output)
>> diff --git a/tests/spec/arb_clear_texture/CMakeLists.gl.txt
>> b/tests/spec/arb_clear_texture/CMakeLists.gl.txt
>> new file mode 100644
>> index 0000000..6a2a7d6
>> --- /dev/null
>> +++ b/tests/spec/arb_clear_texture/CMakeLists.gl.txt
>> @@ -0,0 +1,13 @@
>> +include_directories(
>> + ${GLEXT_INCLUDE_DIR}
>> + ${OPENGL_INCLUDE_PATH}
>> +)
>> +
>> +link_libraries (
>> + piglitutil_${piglit_target_api}
>> + ${OPENGL_gl_LIBRARY}
>> +)
>> +
>> +piglit_add_executable (arb_clear_texture-clear-simple clear-simple.c)
>
> I would've dropped the clear from the filename. But obviously this is just a
> nit-pick and either way is fine.
Good point. I was actually not particularly happy with the name
already, so I'll switch it to simple.c.
>
>> +
>> +# vim: ft=cmake:
>> diff --git a/tests/spec/arb_clear_texture/CMakeLists.txt
>> b/tests/spec/arb_clear_texture/CMakeLists.txt
>> new file mode 100644
>> index 0000000..144a306
>> --- /dev/null
>> +++ b/tests/spec/arb_clear_texture/CMakeLists.txt
>> @@ -0,0 +1 @@
>> +piglit_include_target_api()
>> diff --git a/tests/spec/arb_clear_texture/clear-simple.c
>> b/tests/spec/arb_clear_texture/clear-simple.c
>> new file mode 100644
>> index 0000000..520820a
>> --- /dev/null
>> +++ b/tests/spec/arb_clear_texture/clear-simple.c
>> @@ -0,0 +1,113 @@
>> +/*
>> + * Copyright © 2014 Ilia Mirkin
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining
>> a
>> + * copy of this software and associated documentation files (the
>> "Software"),
>> + * to deal in the Software without restriction, including without
>> limitation
>> + * the rights to use, copy, modify, merge, publish, distribute,
>> sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including the
>> next
>> + * paragraph) shall be included in all copies or substantial portions
>> of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
>> SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
>> OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
>> ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>> DEALINGS
>> + * IN THE SOFTWARE.
>> + */
>> +
>> +#include "piglit-util-gl-common.h"
>> +
>> +PIGLIT_GL_TEST_CONFIG_BEGIN
>> +
>> + config.supports_gl_compat_version = 13;
>> + config.supports_gl_core_version = 31;
>> +
>> + config.window_visual = PIGLIT_GL_VISUAL_RGB |
>> PIGLIT_GL_VISUAL_DOUBLE;
>> +
>> +PIGLIT_GL_TEST_CONFIG_END
>> +
>> +static GLuint texture;
>> +
>> +void
>> +piglit_init(int argc, char **argv)
>> +{
>> + int i;
>> + float *color = malloc(sizeof(float) * piglit_width * piglit_height
>> * 3);
>
> You could free this buffer after setting the texture. :)
Or I could let ths OS free it for me... :p I guess freeing it is the
classy thing to do.
>
>> + float colors[3] = {1.0, 0.0, 0.0};
>> +
>> + piglit_require_extension("GL_ARB_clear_texture");
>> + piglit_require_extension("GL_ARB_framebuffer_object");
>> +
>> + /* Create color data for texture */
>> + for(i = 0; i < piglit_width*piglit_height; i++) {
>> + color[i*3+0] = colors[0];
>> + color[i*3+1] = colors[1];
>> + color[i*3+2] = colors[2];
>> + }
>> +
>> + glGenTextures(1, &texture);
>> +
>> + /* Create texture */
>> + glBindTexture(GL_TEXTURE_2D, texture);
>> + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
>> + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
>> + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_REPEAT);
>> + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_REPEAT);
>> + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_R, GL_REPEAT);
>
> Just fyi, you're setting the mag filter and wrap modes to their default
> values. Also setting GL_TEXTURE_WRAP_R for a 2D texture seems a bit silly to
> me.
I haven't the slightest clue what these are or what they mean. I was
just copying from layered-rendering/clear-color.c. Will remove them.
>
>> + glTexImage2D(GL_TEXTURE_2D, 0, GL_RGB, piglit_width, piglit_height,
>> + 0, GL_RGB, GL_FLOAT, color);
>> +
>> + if(!piglit_check_gl_error(GL_NO_ERROR))
>> + piglit_report_result(PIGLIT_FAIL);
>> +}
>> +
>> +
>> +enum piglit_result
>> +piglit_display(void)
>> +{
>> + const float clearColor[3] = { 0.0, 1.0, 0.0 };
>> + GLuint fbo;
>> + GLenum status;
>> + bool pass = true;
>> +
>> + /* Clear texture with glClearTexImage() */
>> + glClearTexImage(texture, 0, GL_RGB, GL_FLOAT, clearColor);
>
> Maybe you could use piglit_probe_texel_rect_rgb here? If so, you could move
> the texture clear and probe to piglit_init and report the result from there.
Neat! Didn't know it existed -- that seems like a substantially better
option than attaching to an fbo and blitting, and satisfies the desire
not to require a fb-attachable format.
Thanks for your feedback,
-ilia
More information about the Piglit
mailing list