On 21/10/17 03:00, Ilia Mirkin wrote:
On Fri, Oct 20, 2017 at 11:55 AM, Timothy Arceri <tarc...@itsqueeze.com> wrote:
On 21/10/17 00:35, Ilia Mirkin wrote:
On Fri, Oct 20, 2017 at 6:46 AM, Iago Toral Quiroga <ito...@igalia.com>
wrote:
---
src/compiler/glsl/link_varyings.cpp | 53
+++++++++++++++++++++++++++++++++++++
src/compiler/glsl/link_varyings.h | 4 +++
src/compiler/glsl/linker.cpp | 6 +++++
3 files changed, 63 insertions(+)
diff --git a/src/compiler/glsl/link_varyings.cpp
b/src/compiler/glsl/link_varyings.cpp
index dffb4f98df..d7e407184d 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -599,6 +599,59 @@ validate_explicit_variable_location(struct
gl_context *ctx,
}
/**
+ * Validate explicit locations for SSO programs. For non-SSO programs
this
+ * is alreadty done in cross_validate_outputs_to_inputs().
+ */
+void
+validate_sso_explicit_locations(struct gl_context *ctx,
+ struct gl_shader_program *prog)
+{
+ assert(prog->SeparateShader);
+ struct explicit_location_info
explicit_locations_in[MAX_VARYINGS_INCL_PATCH][4];
+ struct explicit_location_info
explicit_locations_out[MAX_VARYINGS_INCL_PATCH][4];
These comments may point to issues in earlier patches / overall logic,
but I'll still make them here. I don't have time to do a full review
of all the patches, unfortunately. Thanks for addressing my concerns
with your earlier patchset.
This should be MAX_VARYINGS. The patch varyings are meant to be
aliased against the non-patch varyings, and their indices must be
assigned as such, otherwise you won't get the overlaps you're supposed
to.
Are you sure this is really required?
https://github.com/KhronosGroup/OpenGL-API/issues/13
Furthermore, I haven't checked how your code works, but I found it was
easier to not have the [4]. None of the checks need to be
per-component (and in fact, you're supposed to raise an error when
components disagree on type-related things, except for VS inputs which
you skip here anyways). Just store the first value in the info, and
then if anything comes in counter to that, return an error. You can
see what I did in https://patchwork.freedesktop.org/patch/175959/. I
spent quite some time to make sure that the patch was correct, so if
you see any functional differences to what you've done, I'd recommend
considering why there are differences -- there shouldn't be any.
As I keep pointing out your patch doesn't handle component aliasing.
Dropping the [4] would mean we not longer check for this.
I'm 50% sure that something else checks that already.
This is where it's done. Without it you won't catch:
> layout (location=0, component=0) int a;
> layout (location=0, component=0) int b;
I'm talking about
layout (location=0, component=0) int a;
layout (location=0, component=1) float b;
being disallowed due to int/float mixing (location aliasing, I think
it's called, but it's been a month or two since I've read the spec).
-ilia
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
