On 10/09/2015 10:56 AM, Tapani Pälli wrote:


On 10/09/2015 10:26 AM, Iago Toral wrote:
On Thu, 2015-10-08 at 15:13 +0300, Tapani Pälli wrote:
Signed-off-by: Tapani Pälli <[email protected]>
---
  tests/all.py                             |   5 +
  tests/spec/gles-3.0/CMakeLists.gles3.txt |   1 +
  tests/spec/gles-3.0/read-depth.c         | 176
+++++++++++++++++++++++++++++++
  3 files changed, 182 insertions(+)
  create mode 100644 tests/spec/gles-3.0/read-depth.c

diff --git a/tests/all.py b/tests/all.py
index 5bc36d0..79d6314 100644
--- a/tests/all.py
+++ b/tests/all.py
@@ -4179,6 +4179,11 @@ with profile.group_manager(
      g(['khr_compressed_astc-miptree_gles2'], 'miptree-gles')

  with profile.group_manager(
+         PiglitGLTest,
+         grouptools.join('spec', 'nv_read_depth')) as g:
+    g(['read_depth_gles3'])
+
+with profile.group_manager(
          PiglitGLTest,
          grouptools.join('spec', 'oes_compressed_paletted_texture'))
as g:
      g(['oes_compressed_paletted_texture-api'], 'basic API')
diff --git a/tests/spec/gles-3.0/CMakeLists.gles3.txt
b/tests/spec/gles-3.0/CMakeLists.gles3.txt
index d56a43e..864c862 100644
--- a/tests/spec/gles-3.0/CMakeLists.gles3.txt
+++ b/tests/spec/gles-3.0/CMakeLists.gles3.txt
@@ -6,5 +6,6 @@ piglit_add_executable (gles-3.0-drawarrays-vertexid
drawarrays-vertexid.c)
  piglit_add_executable(minmax_${piglit_target_api} minmax.c)
  piglit_add_executable(oes_compressed_etc2_texture-miptree_gles3
oes_compressed_etc2_texture-miptree.c)
  piglit_add_executable(texture-immutable-levels_gles3
texture-immutable-levels.c)
+piglit_add_executable(read_depth_gles3 read-depth.c)

  # vim: ft=cmake:
diff --git a/tests/spec/gles-3.0/read-depth.c
b/tests/spec/gles-3.0/read-depth.c
new file mode 100644
index 0000000..456c7e0
--- /dev/null
+++ b/tests/spec/gles-3.0/read-depth.c
@@ -0,0 +1,176 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * 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.
+ */
+
+/** @file read-depth.c
+ *
+ * Tests NV_read_depth implementation
+ *
+ * Test iterates over table of depth buffer formats and expected
types to
+ * read values back from each format. For each format it renders a
rectangle at
+ * different depth levels, reads back a pixel and verifies expected
depth value.
+ */
+
+#include "piglit-util-gl.h"
+
+PIGLIT_GL_TEST_CONFIG_BEGIN
+
+    config.supports_gl_es_version = 30;
+    config.window_visual = PIGLIT_GL_VISUAL_DEPTH;
+
+PIGLIT_GL_TEST_CONFIG_END
+
+static GLint prog;
+
+const char *vs_source =
+        "attribute vec4 vertex;\n"
+    "uniform float depth;\n"

Indentation

will fix


+        "void main()\n"
+        "{\n"
+        "       gl_Position = vec4(vertex.xy, depth, 1.0);\n"
+        "}\n";
+
+const char *fs_source =
+        "void main()\n"
+        "{\n"
+        "       gl_FragColor = vec4(1.0, 1.0, 1.0, 1.0);\n"
+        "}\n";
+
+const GLenum tests[] = {
+    GL_DEPTH_COMPONENT16, GL_UNSIGNED_INT_24_8_OES,
+    GL_DEPTH_COMPONENT24, GL_UNSIGNED_INT_24_8_OES,
+    GL_DEPTH_COMPONENT32F, GL_FLOAT,
+};
+#define TESTS_SIZE (sizeof(tests)/sizeof(tests[0]))
+
+
+static bool
+equals(float a, float b)
+{
+   if (fabs(a - b) < 0.00001)
+      return true;

Indentation (piglit is 8 spaces like you do in other places)

will fix

+   return false;
+}
+
+static GLuint
+create_depth_fbo(GLenum depth_type)
+{
+    GLuint fbo, buffer;
+    GLenum status;
+
+    glGenRenderbuffers(1, &buffer);
+    glBindRenderbuffer(GL_RENDERBUFFER, buffer);
+    glRenderbufferStorage(GL_RENDERBUFFER,
+        depth_type, piglit_width, piglit_height);

The secons line should be aligned with GL_RENDERBUFFER, you have this
same issue in every other place where you break a function call in two
lines.

Hmm, I did not know this style is used in Piglit. I'll go through some
files and fix if this is used by others too.

+    glGenFramebuffers(1, &fbo);
+    glBindFramebuffer(GL_FRAMEBUFFER, fbo);
+
+    glFramebufferRenderbuffer(GL_FRAMEBUFFER,
+        GL_DEPTH_ATTACHMENT, GL_RENDERBUFFER, buffer);
+
+    status = glCheckFramebufferStatus(GL_FRAMEBUFFER);
+    if (status != GL_FRAMEBUFFER_COMPLETE) {
+        fprintf(stderr, "error creating framebuffer, status 0x%x\n",
+            status);
+        return 0;
+    }
+    return fbo;
+}
+
+static bool
+read_depth(GLenum type, float expect)
+{
+    GLfloat data;
+    GLuint uint_pixel;
+    if (type == GL_FLOAT) {
+        glReadPixels(0, 0, 1, 1, GL_DEPTH_COMPONENT, type,
+            (void *) &data);
+    } else {
+        glReadPixels(0, 0, 1, 1, GL_DEPTH_COMPONENT, type,
+            (void *) &uint_pixel);
+        data = (1.0 * ((float) uint_pixel)) / 4294967295.0;

Is this correct? For GL_UNSIGNED_INT_24_8_OES the maximum value we
should see in uint_pixel is that in which the 24 most significant bits
are 1 and the 8 stencil bits are undefined (actually they should be zero
according to your patch in Mesa). The number you divide by, however, is
2^32-1...., so the result is always going to be a stretch lower than
1.0. Shouldn't we do this instead?:

uint_pixel = uint_pixel >> 8;
data = (1.0 * ((float) uint_pixel)) / 16777215.0;

Right, you are correct. Looks like this it won't really make a
difference with this test though, the values will match.

+    }
+    piglit_check_gl_error(GL_NO_ERROR);
+
+    if (!equals(data, expect)) {
+        fprintf(stderr, "expected %f, got %f\n", expect, data);
+        return false;
+    }
+    return true;
+}
+
+enum piglit_result
+piglit_display(void)
+{
+    GLboolean pass = true;
+    const GLfloat rect[] = {
+        -1.0f,  1.0f,
+        1.0f,  1.0f,
+        -1.0f, -1.0f,
+        1.0f, -1.0f,
+    };
+
+    glEnable(GL_DEPTH_TEST);
+    glVertexAttribPointer(PIGLIT_ATTRIB_POS, 2, GL_FLOAT, GL_FALSE, 0,
+            rect);
+    glEnableVertexAttribArray(PIGLIT_ATTRIB_POS);
+
+    /* Loop through formats listed in 'tests'. */
+    for (unsigned j = 0; j < TESTS_SIZE; j+=2) {
+
+        GLuint fbo = create_depth_fbo(tests[j]);
+        if (!fbo)
+            return PIGLIT_FAIL;
+
+        float i = -1.0;
+        float step = 0.1;
+        float expect = 0.0;
+
+        /* Step from -1.0 to 1.0, linear depth. Render a
+                 * rectangle at depth i, read pixel and verify
+                 * expected depth value.
+                 */
+        for (i = -1.0; !equals(i, 1.0 + step); i += step) {

Since this is the only place where you call equals I think it is just
easier to do:

for (i = -1.0; i < 1.00001 + step; i += step) {

yep agreed

well actually not, I also call equals in the read_depth function for verifying the read back result!


+            glClear(GL_DEPTH_BUFFER_BIT);
+            glUniform1f(glGetUniformLocation(prog, "depth"), i);
+
+            glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);
+
+            if (!(read_depth(tests[j + 1], expect)))
+                return PIGLIT_FAIL;
+
+            expect += step / 2.0;
+        }
+        glDeleteFramebuffers(1, &fbo);
+    }
+    return pass ? PIGLIT_PASS : PIGLIT_FAIL;
+}
+
+void
+piglit_init(int argc, char **argv)
+{
+    piglit_require_extension("GL_NV_read_depth");
+    prog = piglit_build_simple_program(vs_source, fs_source);
+    glUseProgram(prog);
+}


_______________________________________________
Piglit mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/piglit
_______________________________________________
Piglit mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/piglit

Reply via email to