| Summary: | [Bisected] dEQP-GLES3: uniform linking logic in the presence of structs | ||
|---|---|---|---|
| Product: | Mesa | Reporter: | Samuel Iglesias Gonsálvez <siglesias> |
| Component: | Mesa core | Assignee: | mesa-dev |
| Status: | RESOLVED FIXED | QA Contact: | mesa-dev |
| Severity: | normal | ||
| Priority: | medium | CC: | imirkin, siglesias |
| Version: | git | ||
| Hardware: | Other | ||
| OS: | All | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: | patch to fix the issue | ||
Where might one find a copy of this test to see what it's doing? Nevermind, reproduced, will send a piglit test shortly. (And perhaps a fix, too...) Created attachment 114547 [details] [review] patch to fix the issue I took a look earlier, this might be the fix. Yeah.... IMO that's the wrong fix though. I think my earlier code was just bonkers (which I knew at the time, but was hoping I could get away with). This is what I'm testing right now:
diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp
index 38b37a6..be87b0a 100644
--- a/src/glsl/glsl_types.cpp
+++ b/src/glsl/glsl_types.cpp
@@ -1077,15 +1077,6 @@ glsl_type::std140_base_alignment(bool row_major) const
return base_alignment;
}
- /* A sampler may never occur in a UBO (without bindless of some sort),
- * however it is convenient to use this alignment function even with
- * regular uniforms. This allows use of this function on uniform structs
- * that contain samplers.
- */
- if (this->is_sampler()) {
- return 0;
- }
-
assert(!"not reached");
return -1;
}
diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
index 799c74b..59adc29 100644
--- a/src/glsl/link_uniforms.cpp
+++ b/src/glsl/link_uniforms.cpp
@@ -547,6 +547,8 @@ private:
virtual void enter_record(const glsl_type *type, const char *name,
bool row_major) {
assert(type->is_record());
+ if (this->ubo_block_index == -1)
+ return;
this->ubo_byte_offset = glsl_align(
this->ubo_byte_offset, type->std140_base_alignment(row_major));
}
@@ -554,6 +556,8 @@ private:
virtual void leave_record(const glsl_type *type, const char *name,
bool row_major) {
assert(type->is_record());
+ if (this->ubo_block_index == -1)
+ return;
this->ubo_byte_offset = glsl_align(
this->ubo_byte_offset, type->std140_base_alignment(row_major));
}
(In reply to Ilia Mirkin from comment #4) > Yeah.... IMO that's the wrong fix though. I think my earlier code was just > bonkers (which I knew at the time, but was hoping I could get away with). I think 1ec715ce was fine as then you don't need to make special cases here and there in the already quite complicated code. > This is what I'm testing right now: > > diff --git a/src/glsl/glsl_types.cpp b/src/glsl/glsl_types.cpp > index 38b37a6..be87b0a 100644 > --- a/src/glsl/glsl_types.cpp > +++ b/src/glsl/glsl_types.cpp > @@ -1077,15 +1077,6 @@ glsl_type::std140_base_alignment(bool row_major) const > return base_alignment; > } > > - /* A sampler may never occur in a UBO (without bindless of some sort), > - * however it is convenient to use this alignment function even with > - * regular uniforms. This allows use of this function on uniform structs > - * that contain samplers. > - */ > - if (this->is_sampler()) { > - return 0; > - } > - > assert(!"not reached"); > return -1; > } > diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp > index 799c74b..59adc29 100644 > --- a/src/glsl/link_uniforms.cpp > +++ b/src/glsl/link_uniforms.cpp > @@ -547,6 +547,8 @@ private: > virtual void enter_record(const glsl_type *type, const char *name, > bool row_major) { > assert(type->is_record()); > + if (this->ubo_block_index == -1) > + return; > this->ubo_byte_offset = glsl_align( > this->ubo_byte_offset, type->std140_base_alignment(row_major)); > } > @@ -554,6 +556,8 @@ private: > virtual void leave_record(const glsl_type *type, const char *name, > bool row_major) { > assert(type->is_record()); > + if (this->ubo_block_index == -1) > + return; > this->ubo_byte_offset = glsl_align( > this->ubo_byte_offset, type->std140_base_alignment(row_major)); > } Yep, that should do it (if nothing else depends on 1ec715ce). Pushed as baa22c8825133a69fd0657f09d2a027236233eb1 upstream. Sorry for the breakage! |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.
Failed tests: * dEQP-GLES3.functional.uniform_api.info_query.active_uniform.array_in_struct.sampler2D_samplerCube_vertex * dEQP-GLES3.functional.uniform_api.info_query.active_uniform.array_in_struct.sampler2D_samplerCube_fragment * dEQP-GLES3.functional.uniform_api.info_query.active_uniform.array_in_struct.sampler2D_samplerCube_both * dEQP-GLES3.functional.uniform_api.info_query.active_uniform.nested_structs_arrays.sampler2D_samplerCube_vertex * dEQP-GLES3.functional.uniform_api.info_query.active_uniform.nested_structs_arrays.sampler2D_samplerCube_fragment * dEQP-GLES3.functional.uniform_api.info_query.active_uniform.nested_structs_arrays.sampler2D_samplerCube_both * dEQP-GLES3.functional.uniform_api.info_query.active_uniform.unused_uniforms.sampler2D_samplerCube_vertex * dEQP-GLES3.functional.uniform_api.info_query.active_uniform.unused_uniforms.sampler2D_samplerCube_fragment * dEQP-GLES3.functional.uniform_api.info_query.active_uniform.unused_uniforms.sampler2D_samplerCube_both * dEQP-GLES3.functional.uniform_api.info_query.indices_active_uniformsiv.array_in_struct.sampler2D_samplerCube_vertex * dEQP-GLES3.functional.uniform_api.info_query.indices_active_uniformsiv.array_in_struct.sampler2D_samplerCube_fragment * dEQP-GLES3.functional.uniform_api.info_query.indices_active_uniformsiv.array_in_struct.sampler2D_samplerCube_both * dEQP-GLES3.functional.uniform_api.info_query.indices_active_uniformsiv.nested_structs_arrays.sampler2D_samplerCube_vertex * dEQP-GLES3.functional.uniform_api.info_query.indices_active_uniformsiv.nested_structs_arrays.sampler2D_samplerCube_fragment * dEQP-GLES3.functional.uniform_api.info_query.indices_active_uniformsiv.nested_structs_arrays.sampler2D_samplerCube_both * dEQP-GLES3.functional.uniform_api.info_query.indices_active_uniformsiv.unused_uniforms.sampler2D_samplerCube_vertex * dEQP-GLES3.functional.uniform_api.info_query.indices_active_uniformsiv.unused_uniforms.sampler2D_samplerCube_fragment * dEQP-GLES3.functional.uniform_api.info_query.indices_active_uniformsiv.unused_uniforms.sampler2D_samplerCube_both * dEQP-GLES3.functional.uniform_api.info_query.consistency.array_in_struct.sampler2D_samplerCube_vertex * dEQP-GLES3.functional.uniform_api.info_query.consistency.array_in_struct.sampler2D_samplerCube_fragment * dEQP-GLES3.functional.uniform_api.info_query.consistency.array_in_struct.sampler2D_samplerCube_both * dEQP-GLES3.functional.uniform_api.info_query.consistency.nested_structs_arrays.sampler2D_samplerCube_vertex * dEQP-GLES3.functional.uniform_api.info_query.consistency.nested_structs_arrays.sampler2D_samplerCube_fragment * dEQP-GLES3.functional.uniform_api.info_query.consistency.nested_structs_arrays.sampler2D_samplerCube_both * dEQP-GLES3.functional.uniform_api.info_query.consistency.unused_uniforms.sampler2D_samplerCube_vertex * dEQP-GLES3.functional.uniform_api.info_query.consistency.unused_uniforms.sampler2D_samplerCube_fragment * dEQP-GLES3.functional.uniform_api.info_query.consistency.unused_uniforms.sampler2D_samplerCube_both * dEQP-GLES3.functional.uniform_api.value.initial.get_uniform.array_in_struct.sampler2D_samplerCube_vertex * dEQP-GLES3.functional.uniform_api.value.initial.get_uniform.array_in_struct.sampler2D_samplerCube_fragment * dEQP-GLES3.functional.uniform_api.value.initial.get_uniform.array_in_struct.sampler2D_samplerCube_both * dEQP-GLES3.functional.uniform_api.value.initial.get_uniform.nested_structs_arrays.sampler2D_samplerCube_vertex * dEQP-GLES3.functional.uniform_api.value.initial.get_uniform.nested_structs_arrays.sampler2D_samplerCube_fragment * dEQP-GLES3.functional.uniform_api.value.initial.get_uniform.nested_structs_arrays.sampler2D_samplerCube_both * dEQP-GLES3.functional.uniform_api.value.assigned.by_pointer.get_uniform.array_in_struct.sampler2D_samplerCube_vertex * dEQP-GLES3.functional.uniform_api.value.assigned.by_pointer.get_uniform.array_in_struct.sampler2D_samplerCube_fragment * dEQP-GLES3.functional.uniform_api.value.assigned.by_pointer.get_uniform.array_in_struct.sampler2D_samplerCube_both * dEQP-GLES3.functional.uniform_api.value.assigned.by_pointer.get_uniform.nested_structs_arrays.sampler2D_samplerCube_vertex * dEQP-GLES3.functional.uniform_api.value.assigned.by_pointer.get_uniform.nested_structs_arrays.sampler2D_samplerCube_fragment * dEQP-GLES3.functional.uniform_api.value.assigned.by_pointer.get_uniform.nested_structs_arrays.sampler2D_samplerCube_both * dEQP-GLES3.functional.uniform_api.value.assigned.by_pointer.render.array_in_struct.sampler2D_samplerCube_vertex * dEQP-GLES3.functional.uniform_api.value.assigned.by_pointer.render.array_in_struct.sampler2D_samplerCube_fragment * dEQP-GLES3.functional.uniform_api.value.assigned.by_pointer.render.array_in_struct.sampler2D_samplerCube_both * dEQP-GLES3.functional.uniform_api.value.assigned.by_pointer.render.nested_structs_arrays.sampler2D_samplerCube_vertex * dEQP-GLES3.functional.uniform_api.value.assigned.by_pointer.render.nested_structs_arrays.sampler2D_samplerCube_fragment * dEQP-GLES3.functional.uniform_api.value.assigned.by_pointer.render.nested_structs_arrays.sampler2D_samplerCube_both * dEQP-GLES3.functional.uniform_api.value.assigned.by_value.get_uniform.array_in_struct.sampler2D_samplerCube_vertex * dEQP-GLES3.functional.uniform_api.value.assigned.by_value.get_uniform.array_in_struct.sampler2D_samplerCube_fragment * dEQP-GLES3.functional.uniform_api.value.assigned.by_value.get_uniform.array_in_struct.sampler2D_samplerCube_both * dEQP-GLES3.functional.uniform_api.value.assigned.by_value.get_uniform.nested_structs_arrays.sampler2D_samplerCube_vertex * dEQP-GLES3.functional.uniform_api.value.assigned.by_value.get_uniform.nested_structs_arrays.sampler2D_samplerCube_fragment * dEQP-GLES3.functional.uniform_api.value.assigned.by_value.get_uniform.nested_structs_arrays.sampler2D_samplerCube_both * dEQP-GLES3.functional.uniform_api.value.assigned.by_value.render.array_in_struct.sampler2D_samplerCube_vertex * dEQP-GLES3.functional.uniform_api.value.assigned.by_value.render.array_in_struct.sampler2D_samplerCube_fragment * dEQP-GLES3.functional.uniform_api.value.assigned.by_value.render.array_in_struct.sampler2D_samplerCube_both * dEQP-GLES3.functional.uniform_api.value.assigned.by_value.render.nested_structs_arrays.sampler2D_samplerCube_vertex * dEQP-GLES3.functional.uniform_api.value.assigned.by_value.render.nested_structs_arrays.sampler2D_samplerCube_fragment * dEQP-GLES3.functional.uniform_api.value.assigned.by_value.render.nested_structs_arrays.sampler2D_samplerCube_both * dEQP-GLES3.functional.uniform_api.value.assigned.basic_array_assign_full.array_in_struct.sampler2D_samplerCube_vertex * dEQP-GLES3.functional.uniform_api.value.assigned.basic_array_assign_full.array_in_struct.sampler2D_samplerCube_fragment * dEQP-GLES3.functional.uniform_api.value.assigned.basic_array_assign_full.array_in_struct.sampler2D_samplerCube_both * dEQP-GLES3.functional.uniform_api.value.assigned.basic_array_assign_partial.array_in_struct.sampler2D_samplerCube_vertex * dEQP-GLES3.functional.uniform_api.value.assigned.basic_array_assign_partial.array_in_struct.sampler2D_samplerCube_fragment * dEQP-GLES3.functional.uniform_api.value.assigned.basic_array_assign_partial.array_in_struct.sampler2D_samplerCube_both * dEQP-GLES3.functional.uniform_api.value.assigned.unused_uniforms.sampler2D_samplerCube_vertex * dEQP-GLES3.functional.uniform_api.value.assigned.unused_uniforms.sampler2D_samplerCube_fragment * dEQP-GLES3.functional.uniform_api.value.assigned.unused_uniforms.sampler2D_samplerCube_both Bisected: commit 53bf7c8fd2e11a6c64d6ff3a98b56d64d2242505 Author: Ilia Mirkin <imirkin@alum.mit.edu> Date: Tue Feb 10 03:02:09 2015 -0500 glsl: fix uniform linking logic in the presence of structs Add a enter/leave record callback so that the offset may be aligned to the proper value. Otherwise only leaf fields are called, and the first field needs to be aligned to the outer struct's base alignment while the last field needs to be aligned to the inner struct's base alignment. This removes most usage of the last field/record type values passed into visit_field. Signed-off-by: Ilia Mirkin <imirkin@alum.mit.edu> Reviewed-by: Dave Airlie <airlied@redhat.com> How to reproduce it: $ cd modules/gles3/ $ ./deqp-gles3 -n dEQP-GLES3.functional.uniform_api.info_query.active_uniform.array_in_struct.sampler2D_samplerCube_vertex Error message: $ ./deqp-gles3 -n dEQP-GLES3.functional.uniform_api.info_query.active_uniform.array_in_struct.sampler2D_samplerCube_vertex dEQP Core 2014.x (0xcafebabe) starting.. target implementation = 'Default' Test case 'dEQP-GLES3.functional.uniform_api.info_query.active_uniform.array_in_struct.sampler2D_samplerCube_vertex'.. deqp-gles3: glsl_types.cpp:1019: unsigned int glsl_type::std140_base_alignment(bool) const: Assertion `this->fields.array->is_record()' failed. Aborted (core dumped) Tested on HSW with Mesa master HEAD: 2fd21d8a84bd28461c964e2ab350389a55b7f001