On 3/2/19 7:21 am, Andres Gomez wrote:
On Sat, 2019-02-02 at 10:10 +1100, Timothy Arceri wrote:
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.
Mmmm ... maybe I should add the reference from the GLSL specs but the
patch doesn't solve exactly the overflow logic but just a specific
corner case, that's why I didn't include it in the first place.
FWIW, the spec in GLSL is not completely clear and you have to read it
twice and check the examples to understand that the limitation of an
"xfb_offset" overflowing a "xfb_stride" is not the value declared for
the "xfb_offset" qualifier but that value plus the size of the variable
to which the qualifier is applied.
It is clearer in the OpenGL spec, though:
From page 390 (page 412 of the PDF) of the OpenGL 4.6 (Core Profile) spec:
" If the set of output variables to record in transform feedback
mode is specified using layout qualifiers, a program will fail to
link if:"
...
" * any binding point has a stride declared using the xfb_stride
layout qualifier and the sum of the offset and size of any
variable associated with that binding point exceeds the value
of this stride;"
Anyway, let me know if the following chunk is enough and you want me to
add it into the commit log:
From page 76 (page 80 of the PDF) of the GLSL 4.60 v.5 spec:
" The xfb_stride qualifier specifies how many bytes are consumed by
each captured vertex. It applies to the transform feedback buffer
for that declaration, whether it is inherited or explicitly
declared. It can be applied to variables, blocks, block members, or
just the qualifier out. If the buffer is capturing any outputs with
double-precision components, the stride must be a multiple of 8,
otherwise it must be a multiple of 4, or a compile-time or
link-time error results. It is a compile-time or link-time error
to have any xfb_offset that overflows xfb_stride, whether stated
on declarations before or after the xfb_stride, or in different
compilation units. While xfb_stride can be declared multiple times
for the same buffer, it is a compile-time or link-time error to
have different values specified for the stride for the same buffer.
For example:"
...
" layout(xfb_buffer = 2, xfb_stride = 32) out block3 {
layout(xfb_offset = 12) vec3 c;
layout(xfb_offset = 24) vec3 d; // ERROR, requires stride of 36
layout(xfb_offset = 0) vec3 g; // okay, increasing order not required
};"
};
...
"
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) {
After reading the spec I think you are right.
Reviewed-by: Timothy Arceri <tarc...@itsqueeze.com>
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
