On 2/2/19 5:05 am, Andres Gomez wrote:
Current implementation uses a complicated calculation which relies in
an implicit conversion to check the integral part of 2 division
results.

However, the calculation actually checks that the xfb_offset is
smaller or a multiplier of the xfb_stride. For example, while this is
expected to fail, it actually succeeds:

   "

     ...

     layout(xfb_buffer = 2, xfb_stride = 12) out block3 {
       layout(xfb_offset = 0) vec3 c;
       layout(xfb_offset = 12) vec3 d; // ERROR, requires stride of 24

Why does this require a stride of 24?

vec3 c uses bytes 0-11. So there is no issue with vec3 d having an offset of 12. Its been a long time since I worked on this but I think this change is wrong. I see no reason this should fail compilation.


     };

     ...

   "

Fixes: 2fab85aaea5 ("glsl: add xfb_stride link time validation")
Cc: Timothy Arceri <tarc...@itsqueeze.com>
Signed-off-by: Andres Gomez <ago...@igalia.com>
---
  src/compiler/glsl/link_varyings.cpp | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/compiler/glsl/link_varyings.cpp 
b/src/compiler/glsl/link_varyings.cpp
index 6cebc5b3c5a..ab66ceb0d00 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -1213,8 +1213,7 @@ tfeedback_decl::store(struct gl_context *ctx, struct 
gl_shader_program *prog,
           return false;
        }
- if ((this->offset / 4) / info->Buffers[buffer].Stride !=
-          (xfb_offset - 1) / info->Buffers[buffer].Stride) {
+      if (xfb_offset > info->Buffers[buffer].Stride) {
           linker_error(prog, "xfb_offset (%d) overflows xfb_stride (%d) for "
                        "buffer (%d)", xfb_offset * 4,
                        info->Buffers[buffer].Stride * 4, buffer);

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to