[Mesa-dev] [PATCH 3/5] panfrost: Overhaul viewport code; fix scissor test

[Alyssa Rosenzweig]

The previous code for handling viewport changes and scissors was ad hoc
and subject to edge cases. This patch unifies and streamlines these
routines, improving code quality and partially fixing the scissor test.

Signed-off-by: Alyssa Rosenzweig <alyssa at rosenzweig.io>
---
 src/gallium/drivers/panfrost/pan_context.c | 94 +++++++++++++---------
 1 file changed, 55 insertions(+), 39 deletions(-)

diff --git a/src/gallium/drivers/panfrost/pan_context.c b/src/gallium/drivers/panfrost/pan_context.c
index bc92933ccda..31c0029ac1a 100644
--- a/src/gallium/drivers/panfrost/pan_context.c
+++ b/src/gallium/drivers/panfrost/pan_context.c
@@ -492,33 +492,42 @@ panfrost_attach_vt_framebuffer(struct panfrost_context *ctx)
         ctx->payload_tiler.postfix.framebuffer = framebuffer;
 }
 
+/* Helpers for filling out mali_viewport structure */
+
 static void
-panfrost_viewport(struct panfrost_context *ctx,
-                  float depth_range_n,
-                  float depth_range_f,
-                  int viewport_x0, int viewport_y0,
-                  int viewport_x1, int viewport_y1)
+panfrost_set_viewport_bounds(struct panfrost_context *ctx, bool scissored, 
+                unsigned minx, unsigned miny, unsigned maxx, unsigned maxy)
 {
-        /* Viewport encoding is asymmetric. Purpose of the floats is unknown? */
+        /* Set main viewport fields */
 
-        struct mali_viewport ret = {
-                .floats = {
-#if 0
-                        -inff, -inff,
-                        inff, inff,
-#endif
-                        0.0, 0.0,
-                        2048.0, 1600.0,
-                },
+        ctx->viewport->viewport0[0] = minx;
+        ctx->viewport->viewport0[1] = miny;
 
-                .depth_range_n = depth_range_n,
-                .depth_range_f = depth_range_f,
+        ctx->viewport->viewport1[0] = MALI_POSITIVE(maxx);
+        ctx->viewport->viewport1[1] = MALI_POSITIVE(maxy);
 
-                .viewport0 = { viewport_x0, viewport_y0 },
-                .viewport1 = { MALI_POSITIVE(viewport_x1), MALI_POSITIVE(viewport_y1) },
-        };
+        /* Set the floats if we're scissoring */
+        if (scissored) {
+                /* Clip to the scissor bounds */
+
+                ctx->viewport->floats[0] = minx;
+                ctx->viewport->floats[1] = miny;
+                ctx->viewport->floats[2] = maxx;
+                ctx->viewport->floats[3] = maxy;
+        } else {
+                /* Essentially, clip to (-inf, inf) */
+
+                ctx->viewport->floats[0] = ctx->viewport->floats[1] = -inff;
+                ctx->viewport->floats[2] = ctx->viewport->floats[3] = +inff;
+        }
+}
 
-        memcpy(ctx->viewport, &ret, sizeof(ret));
+static void
+panfrost_set_viewport_depth(struct panfrost_context *ctx,
+                float depth_near, float depth_far)
+{
+        ctx->viewport->depth_range_n = depth_near;
+        ctx->viewport->depth_range_f = depth_far;
 }
 
 /* Reset per-frame context, called on context initialisation as well as after
@@ -1698,16 +1707,25 @@ panfrost_set_scissor(struct panfrost_context *ctx)
 {
         const struct pipe_scissor_state *ss = &ctx->scissor;
 
-        if (ss && ctx->rasterizer && ctx->rasterizer->base.scissor && 0) {
-                ctx->viewport->viewport0[0] = ss->minx;
-                ctx->viewport->viewport0[1] = ss->miny;
-                ctx->viewport->viewport1[0] = MALI_POSITIVE(ss->maxx);
-                ctx->viewport->viewport1[1] = MALI_POSITIVE(ss->maxy);
+        if (ss && ctx->rasterizer && ctx->rasterizer->base.scissor) {
+                panfrost_set_viewport_bounds(ctx, true,
+                                ss->minx, ss->miny, ss->maxx, ss->maxy);
         } else {
-                ctx->viewport->viewport0[0] = 0;
-                ctx->viewport->viewport0[1] = 0;
-                ctx->viewport->viewport1[0] = MALI_POSITIVE(ctx->pipe_framebuffer.width);
-                ctx->viewport->viewport1[1] = MALI_POSITIVE(ctx->pipe_framebuffer.height);
+                /* Default to entire viewport */
+
+                float mx = abs(ctx->pipe_viewport.translate[0]);
+                float my = abs(ctx->pipe_viewport.translate[1]);
+                float w = abs(ctx->pipe_viewport.scale[0]);
+                float h = abs(ctx->pipe_viewport.scale[1]);
+
+                /* Compute the bounds based on the viewport floats */
+
+                float minx = mx - w, maxx = mx + w;
+                float miny = my - h, maxy = my + h;
+
+                /* Actualize the bounds */
+
+                panfrost_set_viewport_bounds(ctx, false, minx, miny, maxx, maxy);
         }
 }
 
@@ -2426,14 +2444,8 @@ panfrost_set_viewport_states(struct pipe_context *pipe,
 
         ctx->pipe_viewport = *viewports;
 
-#if 0
-        /* TODO: What if not centered? */
-        float w = abs(viewports->scale[0]) * 2.0;
-        float h = abs(viewports->scale[1]) * 2.0;
-
-        ctx->viewport.viewport1[0] = MALI_POSITIVE((int) w);
-        ctx->viewport.viewport1[1] = MALI_POSITIVE((int) h);
-#endif
+        /* Actualize update in the cmdstream */
+        panfrost_set_scissor(ctx);
 }
 
 static void
@@ -2682,7 +2694,11 @@ panfrost_create_context(struct pipe_screen *screen, void *priv, unsigned flags)
         panfrost_emit_vertex_payload(ctx);
         panfrost_emit_tiler_payload(ctx);
         panfrost_invalidate_frame(ctx);
-        panfrost_viewport(ctx, 0.0, 1.0, 0, 0, ctx->pipe_framebuffer.width, ctx->pipe_framebuffer.height);
+
+        /* Setup viewport implicitly via scissor */
+        panfrost_set_scissor(ctx);
+        panfrost_set_viewport_depth(ctx, 0.0, 1.0);
+
         panfrost_default_shader_backend(ctx);
         panfrost_generate_space_filler_indices();
 
-- 
2.20.1