From 2c5ba2ee6ea68aa3062156d1a4abfc3b2556775d Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Thu, 22 Aug 2019 11:29:23 -0700 Subject: panfrost: Implement gl_FragCoord correctly Rather than passing through the transformed gl_Position, we can use the hardware-level varying for this, which will correctly handle gl_FragCoord.w Signed-off-by: Alyssa Rosenzweig --- .../drivers/panfrost/ci/expected-failures.txt | 1 - src/gallium/drivers/panfrost/pan_assemble.c | 5 +++- src/gallium/drivers/panfrost/pan_context.h | 1 + src/gallium/drivers/panfrost/pan_varyings.c | 31 ++++++++++------------ src/panfrost/include/panfrost-job.h | 19 ++++++++----- 5 files changed, 32 insertions(+), 25 deletions(-) diff --git a/src/gallium/drivers/panfrost/ci/expected-failures.txt b/src/gallium/drivers/panfrost/ci/expected-failures.txt index a15c3f9c6705..b0fc872a3009 100644 --- a/src/gallium/drivers/panfrost/ci/expected-failures.txt +++ b/src/gallium/drivers/panfrost/ci/expected-failures.txt @@ -876,6 +876,5 @@ dEQP-GLES2.functional.polygon_offset.default_factor_1_slope Fail dEQP-GLES2.functional.polygon_offset.default_render_with_units Fail dEQP-GLES2.functional.polygon_offset.fixed16_factor_1_slope Fail dEQP-GLES2.functional.polygon_offset.fixed16_render_with_units Fail -dEQP-GLES2.functional.shaders.builtin_variable.fragcoord_w Fail dEQP-GLES2.functional.shaders.scoping.valid.local_variable_hides_function_parameter_fragment Fail dEQP-GLES2.functional.shaders.scoping.valid.local_variable_hides_function_parameter_vertex Fail diff --git a/src/gallium/drivers/panfrost/pan_assemble.c b/src/gallium/drivers/panfrost/pan_assemble.c index 47f6c1e5312d..b57cd5ef6ad2 100644 --- a/src/gallium/drivers/panfrost/pan_assemble.c +++ b/src/gallium/drivers/panfrost/pan_assemble.c @@ -145,7 +145,10 @@ panfrost_shader_compile( /* Check for special cases, otherwise assume general varying */ if (location == VARYING_SLOT_POS) { - v.format = MALI_VARYING_POS; + if (stage == MESA_SHADER_FRAGMENT) + state->reads_frag_coord = true; + else + v.format = MALI_VARYING_POS; } else if (location == VARYING_SLOT_PSIZ) { v.format = MALI_R16F; v.swizzle = default_vec1_swizzle; diff --git a/src/gallium/drivers/panfrost/pan_context.h b/src/gallium/drivers/panfrost/pan_context.h index 8efd97e779b6..4c1580b33931 100644 --- a/src/gallium/drivers/panfrost/pan_context.h +++ b/src/gallium/drivers/panfrost/pan_context.h @@ -231,6 +231,7 @@ struct panfrost_shader_state { bool writes_point_size; bool reads_point_coord; bool reads_face; + bool reads_frag_coord; struct mali_attr_meta varyings[PIPE_MAX_ATTRIBS]; gl_varying_slot varyings_loc[PIPE_MAX_ATTRIBS]; diff --git a/src/gallium/drivers/panfrost/pan_varyings.c b/src/gallium/drivers/panfrost/pan_varyings.c index 0ac3b9d88545..113043b699dc 100644 --- a/src/gallium/drivers/panfrost/pan_varyings.c +++ b/src/gallium/drivers/panfrost/pan_varyings.c @@ -73,19 +73,6 @@ panfrost_emit_streamout( slot->elements = addr; } -static void -panfrost_emit_point_coord(union mali_attr *slot) -{ - slot->elements = MALI_VARYING_POINT_COORD | MALI_ATTR_LINEAR; - slot->stride = slot->size = slot->shift = slot->extra_flags = 0; -} - -static void -panfrost_emit_front_face(union mali_attr *slot) -{ - slot->elements = MALI_VARYING_FRONT_FACING | MALI_ATTR_INTERNAL; -} - /* Given a shader and buffer indices, link varying metadata together */ static bool @@ -250,6 +237,7 @@ panfrost_emit_varying_descriptor( memcpy(trans.cpu + vs_size, fs->varyings, fs_size); union mali_attr varyings[PIPE_MAX_ATTRIBS]; + memset(varyings, 0, sizeof(varyings)); /* Figure out how many streamout buffers could be bound */ unsigned so_count = ctx->streamout.num_targets; @@ -269,6 +257,7 @@ panfrost_emit_varying_descriptor( signed gl_PointSize = vs->writes_point_size ? (idx++) : -1; signed gl_PointCoord = reads_point_coord ? (idx++) : -1; signed gl_FrontFacing = fs->reads_face ? (idx++) : -1; + signed gl_FragCoord = fs->reads_frag_coord ? (idx++) : -1; /* Emit the stream out buffers */ @@ -305,20 +294,25 @@ panfrost_emit_varying_descriptor( 2, vertex_count); if (reads_point_coord) - panfrost_emit_point_coord(&varyings[gl_PointCoord]); + varyings[gl_PointCoord].elements = MALI_VARYING_POINT_COORD; if (fs->reads_face) - panfrost_emit_front_face(&varyings[gl_FrontFacing]); + varyings[gl_FrontFacing].elements = MALI_VARYING_FRONT_FACING; + + if (fs->reads_frag_coord) + varyings[gl_FragCoord].elements = MALI_VARYING_FRAG_COORD; /* Let's go ahead and link varying meta to the buffer in question, now - * that that information is available */ + * that that information is available. VARYING_SLOT_POS is mapped to + * gl_FragCoord for fragment shaders but gl_Positionf or vertex shaders + * */ panfrost_emit_varying_meta(trans.cpu, vs, general, gl_Position, gl_PointSize, gl_PointCoord, gl_FrontFacing); panfrost_emit_varying_meta(trans.cpu + vs_size, fs, - general, gl_Position, gl_PointSize, + general, gl_FragCoord, gl_PointSize, gl_PointCoord, gl_FrontFacing); /* Replace streamout */ @@ -376,6 +370,9 @@ panfrost_emit_varying_descriptor( /* Fix up unaligned addresses */ for (unsigned i = 0; i < so_count; ++i) { + if (varyings[i].elements < MALI_VARYING_SPECIAL) + continue; + unsigned align = (varyings[i].elements & 63); /* While we're at it, the SO buffers are linear */ diff --git a/src/panfrost/include/panfrost-job.h b/src/panfrost/include/panfrost-job.h index b19bfacf1295..6f8a757e2c69 100644 --- a/src/panfrost/include/panfrost-job.h +++ b/src/panfrost/include/panfrost-job.h @@ -804,9 +804,9 @@ struct mali_payload_set_value { * let shift=extra_flags=0. Stride is set to the image format's bytes-per-pixel * (*NOT the row stride*). Size is set to the size of the image itself. * - * Special internal varyings (including gl_FrontFacing) are handled vai - * MALI_ATTR_INTERNAL, which has all fields set to zero and uses a special - * elements pseudo-pointer. + * Special internal varyings (including gl_FrontFacing) could be seen as + * IMAGE/INTERNAL as well as LINEAR, setting all fields set to zero and using a + * special elements pseudo-pointer. */ enum mali_attr_mode { @@ -819,16 +819,23 @@ enum mali_attr_mode { MALI_ATTR_INTERNAL = 6 }; -/* Pseudo-address for gl_FrontFacing */ +/* Pseudo-address for gl_FrontFacing, used with INTERNAL. Same addres is used + * for gl_FragCoord with IMAGE, needing a coordinate flip. Who knows. */ -#define MALI_VARYING_FRONT_FACING (0x20) +#define MALI_VARYING_FRAG_COORD (0x25) +#define MALI_VARYING_FRONT_FACING (0x26) /* This magic "pseudo-address" is used as `elements` to implement * gl_PointCoord. When read from a fragment shader, it generates a point * coordinate per the OpenGL ES 2.0 specification. Flipped coordinate spaces * require an affine transformation in the shader. */ -#define MALI_VARYING_POINT_COORD (0x60) +#define MALI_VARYING_POINT_COORD (0x61) + +/* Used for comparison to check if an address is special. Mostly a guess, but + * it doesn't really matter. */ + +#define MALI_VARYING_SPECIAL (0x100) union mali_attr { /* This is used for actual attributes. */ -- cgit v1.2.3