On 01/08/2016 05:12 PM, srol...@vmware.com wrote:
> From: Roland Scheidegger <srol...@vmware.com>
>
> The rect halves comparison is in fact not valid in general. The reason is that
> the same geometry does not have the same precision even if it it just shifted
> by some fixed amount in one direction.
> As an example, some calculated x value near 7 will be near 263 if drawn with
> a viewport x value of 256. The value near 7 has 5 bits more precision -
> when calculating the fixed-point values for rasterization (with subpixel
> accuracy), the lower value thus may be rounded in a different direction
> than the higher one, even with correct rounding (not even entirely sure what
> "correct" rounding here means, nearest-floor or nearest-even or whatever, the
> problem is independent from that). This can then cause different pixels to be
> covered by the primitive.
> This causes a failure with this test in llvmpipe when the rounding mode is
> changed slightly (from "mostly" nearest-away-from-zero to nearest-even). I was
> not able to reproduce this with this test on nvidia or amd hardware, but they
> are definitely affected in theory by the same issue (proven by some quick and
> dirty test).
> So, simply add then subtract some fixed amount to the position values, which
> ensures there should be no differences in the position later even after
> viewport translate (we use window size + 1 here, so the values in range
> -1.0 - +1.0 will all have the same exponent (just above the window size)).
> (We must do that with uniforms otherwise mesa will optimize the add/sub away -
> this is definitely unsafe math optimization but may well be allowed.)
>
> This may be a problem with more tests, albeit most tend to use vertex data
> which is already sufficiently aligned (e.g. drawing simple aligned rects).
> ---
> tests/general/quad-invariance.c | 55
> +++++++++-------------
> .../glsl-1.50/execution/geometry/end-primitive.c | 15 +++++-
> 2 files changed, 36 insertions(+), 34 deletions(-)
>
> diff --git a/tests/general/quad-invariance.c b/tests/general/quad-invariance.c
> index b5741d0..fd51dec 100644
> --- a/tests/general/quad-invariance.c
> +++ b/tests/general/quad-invariance.c
> @@ -38,9 +38,13 @@
>
> #include "piglit-util-gl.h"
>
> +#define PATTERN_SIZE 256
> +
> PIGLIT_GL_TEST_CONFIG_BEGIN
>
> config.supports_gl_compat_version = 10;
> + config.window_width = 2*PATTERN_SIZE;
> + config.window_height = PATTERN_SIZE;
>
> config.window_visual = PIGLIT_GL_VISUAL_RGB | PIGLIT_GL_VISUAL_DOUBLE;
>
> @@ -49,41 +53,24 @@ PIGLIT_GL_TEST_CONFIG_END
> enum piglit_result
> piglit_display(void)
> {
> + int i;
> GLboolean pass = GL_TRUE;
> - float verts[12][2] = {
> + float verts[3][2] = {
> /* prim 1: left half of screen. */
> - {-1.0, -1.0},
> - { 0.0, -1.0},
> - { 0.0, 1.0},
> - {-1.0, 1.0},
> - /* prim 2: right half of screen. */
> - { 0.0, -1.0},
> - { 1.0, -1.0},
> - { 1.0, 1.0},
> + {-1.0 + 2.0f / PATTERN_SIZE, -1.0},
> + {-1.0 + 2.0f / PATTERN_SIZE, 1.0},
> { 0.0, 1.0},
> - /* prim 3: somewhere off the screen. */
> - { 2.0, -1.0},
> - { 3.0, -1.0},
> - { 3.0, 1.0},
> - { 2.0, 1.0},
> };
> - float colors[12][4] = {
> - {1.0, 0.0, 0.0, 0.0},
> - {0.0, 1.0, 0.0, 0.0},
> - {0.0, 0.0, 1.0, 0.0},
> - {1.0, 1.0, 1.0, 0.0},
> -
> - {1.0, 0.0, 0.0, 0.0},
> - {0.0, 1.0, 0.0, 0.0},
> - {0.0, 0.0, 1.0, 0.0},
> - {1.0, 1.0, 1.0, 0.0},
> -
> - {1.0, 0.0, 0.0, 0.0},
> - {0.0, 1.0, 0.0, 0.0},
> - {0.0, 0.0, 1.0, 0.0},
> - {1.0, 1.0, 1.0, 0.0},
> + float colors[3][4] = {
> + {1.0, 0.0, 0.0, 1.0},
> + {1.0, 0.0, 0.0, 1.0},
> + {1.0, 0.0, 0.0, 1.0},
> +
> };
> +
> static GLboolean once = GL_TRUE;
> + for (i = 0; i < (1 << 24) && pass; i++) {
> + verts[1][0] += 5.96046447754E-08f * 2.0f;
>
> glClearColor(0.0, 0.0, 0.0, 0.0);
> glClear(GL_COLOR_BUFFER_BIT);
> @@ -94,19 +81,21 @@ piglit_display(void)
> glEnableClientState(GL_VERTEX_ARRAY);
>
> /* left: 1 prim */
> - glDrawArrays(GL_QUADS, 0, 4);
> + glViewport(0, 0, piglit_width/2, piglit_height);
> + glDrawArrays(GL_TRIANGLES, 0, 3);
>
> + glViewport(piglit_width/2, 0, piglit_width/2, piglit_height);
> /* right: 1 prim */
> - glDrawArrays(GL_QUADS, 4, 8);
> + glDrawArrays(GL_TRIANGLES, 0, 3);
>
> if (once) {
> printf("Left and right half should match.\n");
> once = GL_FALSE;
> }
>
> - pass = piglit_probe_rect_halves_equal_rgba(0, 0, piglit_width,
> + pass &= piglit_probe_rect_halves_equal_rgba(0, 0, piglit_width,
> piglit_height);
> -
> + }
> piglit_present_results();
>
> return pass ? PIGLIT_PASS : PIGLIT_WARN;
> diff --git a/tests/spec/glsl-1.50/execution/geometry/end-primitive.c
> b/tests/spec/glsl-1.50/execution/geometry/end-primitive.c
> index 6df3a89..e251ad0 100644
> --- a/tests/spec/glsl-1.50/execution/geometry/end-primitive.c
> +++ b/tests/spec/glsl-1.50/execution/geometry/end-primitive.c
> @@ -92,6 +92,8 @@ static const char *spiral_text =
> "#version 150\n"
> "\n"
> "uniform int num_vertices;\n"
> + "uniform float hack1;\n"
> + "uniform float hack2;\n"
> "\n"
> "vec2 spiral(int vertex_id)\n"
> "{\n"
> @@ -105,7 +107,10 @@ static const char *spiral_text =
> " if (vertex_id % 2 == 1) r += 1.0;\n"
> " float max_r = b*sqrt(a*float(num_vertices)) + 1.0;\n"
> " r /= max_r;\n"
> - " return r*vec2(cos(theta), sin(theta));\n"
> + " vec2 tmp = r*vec2(cos(theta), sin(theta));\n"
> + " // ensure reasonably aligned vertices \n"
> + " // note cannot use literals as they'd get optimized away... \n"
> + " return tmp + hack1 - hack2;\n"
This works only by luck, and it's probably the only use-case I've seen
for the precise keyword. :) If the compiler stack decided to rearrange
this expression as tmp + (hack1 - hack2), you'd be right back where you
started. Without the precise keyword, the GLSL compiler is allowed to
do that.
I haven't looked at the rest of the shader, so I don't have an alternate
suggestion. Maybe do something like floor(tmp * large_constant) /
large_constant?
Also, piglit uses hard tabs for indentation.
> "}\n";
>
> /**
> @@ -303,6 +308,10 @@ draw_ref_pattern()
> glUseProgram(prog_ref);
> glUniform1i(glGetUniformLocation(prog_ref, "num_vertices"),
> num_vertices);
> + glUniform1f(glGetUniformLocation(prog_ref, "hack1"),
> + (float)PATTERN_SIZE * 2.0f + 1.0f);
> + glUniform1f(glGetUniformLocation(prog_ref, "hack2"),
> + (float)PATTERN_SIZE * 2.0f + 1.0f);
> glEnable(GL_PRIMITIVE_RESTART);
> glPrimitiveRestartIndex(0xffffffff);
>
> @@ -342,6 +351,10 @@ draw_test_pattern()
> glUseProgram(prog_test);
> glUniform1i(glGetUniformLocation(prog_test, "num_vertices"),
> num_vertices);
> + glUniform1f(glGetUniformLocation(prog_ref, "hack1"),
> + (float)PATTERN_SIZE * 2.0f + 1.0f);
> + glUniform1f(glGetUniformLocation(prog_ref, "hack2"),
> + (float)PATTERN_SIZE * 2.0f + 1.0f);
> glDrawArrays(GL_POINTS, 0, 3);
> }
>
>
_______________________________________________
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
