This hardly qualifies as a full review, but I have a few comments.
On Mon, Nov 02, 2015 at 07:10:51PM -0600, Ryan Houdek wrote:
> This has been tested on the Nvidia proprietary driver and llvmpipe.
> This piglit requires GLES 3.1 support due to glMemoryBarrier.
> To test Mesa I temporarily moved glMemoryBarrier as a ES 3.0 function.
> ---
> tests/all.py | 12 +
> tests/spec/CMakeLists.txt | 1 +
> tests/spec/ext_buffer_storage/CMakeLists.gles2.txt | 11 +
> tests/spec/ext_buffer_storage/CMakeLists.txt | 1 +
> tests/spec/ext_buffer_storage/bufferstorage.c | 256
> +++++++++++++++++++++
> 5 files changed, 281 insertions(+)
> create mode 100644 tests/spec/ext_buffer_storage/CMakeLists.gles2.txt
> create mode 100644 tests/spec/ext_buffer_storage/CMakeLists.txt
> create mode 100644 tests/spec/ext_buffer_storage/bufferstorage.c
>
> diff --git a/tests/all.py b/tests/all.py
> index acfc586..f42581a 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -4489,5 +4489,17 @@ with profile.group_manager(
> g(['oes_draw_elements_base_vertex-multidrawelements'],
> run_concurrent=False)
>
> +with profile.group_manager(
> + PiglitGLTest,
> + grouptools.join('spec', 'ext_buffer_storage')) as g:
> + g(['ext-bufferstorage-persistent', 'draw'])
> + g(['ext-bufferstorage-persistent', 'draw', 'coherent'])
> + g(['ext-bufferstorage-persistent', 'draw', 'client-storage'])
> + g(['ext-bufferstorage-persistent', 'draw', 'coherent', 'client-storage'])
> + g(['ext-bufferstorage-persistent', 'read'])
> + g(['ext-bufferstorage-persistent', 'read', 'coherent'])
> + g(['ext-bufferstorage-persistent', 'read', 'client-storage'])
> + g(['ext-bufferstorage-persistent', 'read', 'coherent', 'client-storage'])
> +
You could refactor this into a loop:
with profile.group_manager(
PiglitGLTest,
grouptools.join('spec', 'ext_buffer_storage')) as g:
for mode in ['read', 'draw']:
g(['ext-bufferstorage-persistent', mode])
g(['ext-bufferstorage-persistent', mode, 'coherent'])
g(['ext-bufferstorage-persistent', mode, 'client-storage'])
g(['ext-bufferstorage-persistent', mode, 'coherent', 'client-storage'])
> if platform.system() is 'Windows':
> profile.filter_tests(lambda p, _: not p.startswith('glx'))
> diff --git a/tests/spec/CMakeLists.txt b/tests/spec/CMakeLists.txt
> index eeedc4c..582804e 100644
> --- a/tests/spec/CMakeLists.txt
> +++ b/tests/spec/CMakeLists.txt
> @@ -136,3 +136,4 @@ add_subdirectory (ext_framebuffer_blit)
> add_subdirectory (mesa_pack_invert)
> add_subdirectory (ext_texture_format_bgra8888)
> add_subdirectory (oes_draw_elements_base_vertex)
> +add_subdirectory (ext_buffer_storage)
> diff --git a/tests/spec/ext_buffer_storage/CMakeLists.gles2.txt
> b/tests/spec/ext_buffer_storage/CMakeLists.gles2.txt
> new file mode 100644
> index 0000000..a39aac6
> --- /dev/null
> +++ b/tests/spec/ext_buffer_storage/CMakeLists.gles2.txt
> @@ -0,0 +1,11 @@
> +include_directories(
> + ${GLEXT_INCLUDE_DIR}
> + ${OPENGL_INCLUDE_PATH}
> +)
> +
> +link_libraries (
> + piglitutil_${piglit_target_api}
> + ${OPENGL_gl_LIBRARY}
> +)
> +
> +piglit_add_executable (ext-bufferstorage-persistent bufferstorage.c)
> diff --git a/tests/spec/ext_buffer_storage/CMakeLists.txt
> b/tests/spec/ext_buffer_storage/CMakeLists.txt
> new file mode 100644
> index 0000000..144a306
> --- /dev/null
> +++ b/tests/spec/ext_buffer_storage/CMakeLists.txt
> @@ -0,0 +1 @@
> +piglit_include_target_api()
> diff --git a/tests/spec/ext_buffer_storage/bufferstorage.c
> b/tests/spec/ext_buffer_storage/bufferstorage.c
> new file mode 100644
> index 0000000..a5acb7b
> --- /dev/null
> +++ b/tests/spec/ext_buffer_storage/bufferstorage.c
> @@ -0,0 +1,256 @@
> +/*
> + * Copyright © 2014 Advanced Micro Devices, Inc.
> + * All rights reserved.
> + *
> + * 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.
> + *
> + * Authors:
> + * Marek Olšák <[email protected]>
> + */
> +
> +/**
> + * This tests GL_MAP_PERSISTENT_BIT and glBufferStorageEXT
> + * from EXT_buffer_storage.
> + */
> +
> +#include "piglit-util-gl.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> + config.supports_gl_es_version = 31;
> + config.window_width = 100;
> + config.window_height = 100;
> +
> + config.window_visual = PIGLIT_GL_VISUAL_RGB | PIGLIT_GL_VISUAL_DOUBLE;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +enum test_flag {
> + NONE,
> + READ,
> + DRAW
> +};
> +
> +const char *vs_source = {
> + "#version 300 es\n"
> + "in vec2 vertex;\n"
> + "void main() {\n"
> + " gl_Position = vec4(vertex.xy, 0, 1);\n"
> + "}\n"
> +};
> +
> +const char *fs_source = {
> + "#version 300 es\n"
> + "out highp vec4 ocol;\n"
> + "void main() {\n"
> + " ocol = vec4(1, 1, 1, 1);\n"
> + "}\n"
> +};
> +
> +static GLuint vao;
> +static GLuint buffer;
> +static GLfloat *map;
> +static GLboolean coherent, client_storage;
> +static enum test_flag test = NONE;
> +
> +#define BUF_SIZE (12 * 4 * sizeof(float))
> +
> +void
> +piglit_init(int argc, char **argv)
> +{
> + int i;
> +
> + for (i = 1; i < argc; i++) {
> + if (!strcmp(argv[i], "coherent")) {
> + coherent = GL_TRUE;
> + continue;
> + }
> + if (!strcmp(argv[i], "read")) {
> + test = READ;
> + continue;
> + }
> + if (!strcmp(argv[i], "draw")) {
> + test = DRAW;
> + continue;
> + }
> + if (!strcmp(argv[i], "client-storage")) {
> + client_storage = GL_TRUE;
> + continue;
> + }
> +
> + printf("Unknown param: %s\n", argv[i]);
> + piglit_report_result(PIGLIT_FAIL);
> + }
> +
> + if (test == NONE) {
> + puts("Wrong parameters.");
> + piglit_report_result(PIGLIT_FAIL);
> + }
> +
> + GLuint program;
> + GLuint vertex_index;
> +
> + piglit_require_extension("GL_EXT_buffer_storage");
> +
> + /* Create program */
> + program = piglit_build_simple_program(vs_source, fs_source);
> + glUseProgram(program);
> +
> + glGenBuffers(1, &buffer);
> + glBindBuffer(GL_ARRAY_BUFFER, buffer);
> + glBufferStorageEXT(GL_ARRAY_BUFFER, BUF_SIZE, NULL,
> + GL_MAP_WRITE_BIT |
> + GL_MAP_PERSISTENT_BIT |
> + (coherent ? GL_MAP_COHERENT_BIT : 0) |
> + GL_DYNAMIC_STORAGE_BIT |
> + (client_storage ? GL_CLIENT_STORAGE_BIT : 0));
> +
> + piglit_check_gl_error(GL_NO_ERROR);
> +
> + map = glMapBufferRange(GL_ARRAY_BUFFER, 0, BUF_SIZE,
> + GL_MAP_WRITE_BIT |
> + GL_MAP_PERSISTENT_BIT |
> + (coherent ? GL_MAP_COHERENT_BIT : 0));
> +
> + piglit_check_gl_error(GL_NO_ERROR);
> +
> + if (!map)
> + piglit_report_result(PIGLIT_FAIL);
> +
> + /* Gen VAO */
> + glGenVertexArrays(1, &vao);
> + glBindVertexArray(vao);
> +
> + /* Retrieve indices from vs */
> + vertex_index = glGetAttribLocation(program, "vertex");
> +
> + /* Enable vertex attrib array */
> + glEnableVertexAttribArray(vertex_index);
> + glVertexAttribPointer(vertex_index, 3, GL_FLOAT, GL_FALSE, 0, 0);
> +
> + piglit_check_gl_error(GL_NO_ERROR);
> +
> + glBindBuffer(GL_ARRAY_BUFFER, 0);
> +}
> +
> +enum piglit_result
> +piglit_display(void)
> +{
> + float white[4] = {1.0, 1.0, 1.0, 1.0};
> + GLboolean pass = GL_TRUE;
This should just be a bool obth her and elsewhere. The rule of thumb is
use generic C types when not interfacing with OpenGL.
> + int i;
> +
> + float array[] = {
> + 1.00, 0.75, 0.0,
> + 1.00, 1.00, 0.0,
> + 0.75, 0.75, 0.0,
> + 0.75, 1.00, 0.0,
> +
> + 0.50, 0.75, 0.0,
> + 0.50, 1.00, 0.0,
> + 0.25, 0.75, 0.0,
> + 0.25, 1.00, 0.0,
> +
> + 0.00, 0.75, 0.0,
> + 0.00, 1.00, 0.0,
> + -0.25, 0.75, 0.0,
> + -0.25, 1.00, 0.0,
> +
> + -0.50, 0.75, 0.0,
> + -0.50, 1.00, 0.0,
> + -0.75, 0.75, 0.0,
> + -0.75, 1.00, 0.0,
> + };
> +
> + glClear(GL_COLOR_BUFFER_BIT);
> +
> + if (test == DRAW) {
> + glBindVertexArray(vao);
> +
> + memcpy(map, array, 12 * sizeof(float));
> + if (!coherent)
> + glMemoryBarrier(GL_CLIENT_MAPPED_BUFFER_BARRIER_BIT);
> +
> + glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
> +
> + memcpy(map+12, array+12, 12 * sizeof(float));
> + if (!coherent)
> + glMemoryBarrier(GL_CLIENT_MAPPED_BUFFER_BARRIER_BIT);
> +
> + glDrawArrays(GL_TRIANGLE_STRIP, 4, 4);
> +
> + memcpy(map+12*2, array+12*2, 12 * sizeof(float));
> + if (!coherent)
> + glMemoryBarrier(GL_CLIENT_MAPPED_BUFFER_BARRIER_BIT);
> +
> + glDrawArrays(GL_TRIANGLE_STRIP, 8, 4);
> +
> + memcpy(map+12*3, array+12*3, 12 * sizeof(float));
> + if (!coherent)
> + glMemoryBarrier(GL_CLIENT_MAPPED_BUFFER_BARRIER_BIT);
> +
> + glDrawArrays(GL_TRIANGLE_STRIP, 12, 4);
> +
> + piglit_check_gl_error(0);
> +
> + pass = pass && piglit_probe_pixel_rgba(13, 87, white);
> + pass = pass && piglit_probe_pixel_rgba(39, 87, white);
> + pass = pass && piglit_probe_pixel_rgba(65, 87, white);
> + pass = pass && piglit_probe_pixel_rgba(91, 87, white);
> + }
> + else if (test == READ) {
> + GLuint srcbuf;
> + GLsync fence;
> +
> + glGenBuffers(1, &srcbuf);
> + glBindBuffer(GL_COPY_READ_BUFFER, srcbuf);
> + glBufferData(GL_COPY_READ_BUFFER, BUF_SIZE, array,
> GL_STATIC_DRAW);
> +
> + /* Copy some data to the mapped buffer and check if the CPU can
> see it. */
> + glBindBuffer(GL_COPY_WRITE_BUFFER, buffer);
> + glCopyBufferSubData(GL_COPY_READ_BUFFER, GL_COPY_WRITE_BUFFER,
> + 0, 0, BUF_SIZE);
> +
> + glBindBuffer(GL_COPY_READ_BUFFER, 0);
> + glBindBuffer(GL_COPY_WRITE_BUFFER, 0);
> + glDeleteBuffers(1, &srcbuf);
> +
> + if (!coherent)
> + glMemoryBarrier(GL_CLIENT_MAPPED_BUFFER_BARRIER_BIT);
> +
> + fence = glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0);
> + glClientWaitSync(fence, GL_SYNC_FLUSH_COMMANDS_BIT,
> GL_TIMEOUT_IGNORED);
> +
> + for (i = 0; i < ARRAY_SIZE(array); i++) {
> + if (map[i] != array[i]) {
> + printf("Probe [%i] failed. Expected: %f
> Observed: %f\n",
> + i, array[i], map[i]);
> + pass = GL_FALSE;
> + }
> + }
> + }
> + else {
> + assert(0);
> + }
> +
> + piglit_present_results();
> +
> + return pass ? PIGLIT_PASS : PIGLIT_FAIL;
> +}
> --
> 2.5.0
>
> _______________________________________________
> Piglit mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/piglit
signature.asc
Description: PGP signature
_______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
