On 12.04.2016 07:45, Marek Olšák wrote:
On Tue, Apr 12, 2016 at 1:53 AM, Marek Olšák <[email protected]> wrote:
On Mon, Apr 11, 2016 at 7:37 PM, Nicolai Hähnle <[email protected]> wrote:
On 10.04.2016 17:34, Marek Olšák wrote:

From: Marek Olšák <[email protected]>

Guard band clipping speeds up rasterization for primitives that are
partially off-screen.  This change in particular results in small
framerate improvements in a wide range of games.

Started by Grigori Goronzy <[email protected]>.
---
   src/gallium/drivers/radeonsi/si_state.c | 73
+++++++++++++++++++++++++++++++--
   1 file changed, 69 insertions(+), 4 deletions(-)

diff --git a/src/gallium/drivers/radeonsi/si_state.c
b/src/gallium/drivers/radeonsi/si_state.c
index b5db456..bc04b46 100644
--- a/src/gallium/drivers/radeonsi/si_state.c
+++ b/src/gallium/drivers/radeonsi/si_state.c
@@ -887,6 +887,15 @@ static void si_clip_scissor(struct pipe_scissor_state
*out,
         out->maxy = MIN2(out->maxy, clip->maxy);
   }

+static void si_scissor_make_union(struct si_signed_scissor *out,
+                                 struct si_signed_scissor *in)
+{
+       out->minx = MIN2(out->minx, in->minx);
+       out->miny = MIN2(out->miny, in->miny);
+       out->maxx = MAX2(out->maxx, in->maxx);
+       out->maxy = MAX2(out->maxy, in->maxy);
+}
+
   static void si_emit_one_scissor(struct radeon_winsys_cs *cs,
                                 struct si_signed_scissor *vp_scissor,
                                 struct pipe_scissor_state *scissor)
@@ -908,12 +917,64 @@ static void si_emit_one_scissor(struct
radeon_winsys_cs *cs,
                         S_028254_BR_Y(final.maxy));
   }

+/* the range is [-MAX, MAX] */
+#define SI_MAX_VIEWPORT_RANGE 32768
+
+static void si_emit_guardband(struct si_context *sctx,
+                             struct si_signed_scissor *vp_as_scissor)
+{
+       struct radeon_winsys_cs *cs = sctx->b.gfx.cs;
+       struct pipe_viewport_state vp;
+       float left, top, right, bottom, max_range, guardband_x,
guardband_y;
+
+       /* Reconstruct the viewport transformation from the scissor. */
+       vp.translate[0] = (vp_as_scissor->minx + vp_as_scissor->maxx) /
2.0;
+       vp.translate[1] = (vp_as_scissor->miny + vp_as_scissor->maxy) /
2.0;
+       vp.scale[0] = vp_as_scissor->maxx - vp.translate[0];
+       vp.scale[1] = vp_as_scissor->maxy - vp.translate[1];
+
+       /* Treat a 0x0 viewport as 1x1 to prevent division by zero. */
+       if (vp_as_scissor->minx == vp_as_scissor->maxx)
+               vp.scale[0] = 0.5;
+       if (vp_as_scissor->miny == vp_as_scissor->maxy)
+               vp.scale[1] = 0.5;
+
+       /* Find the biggest guard band that is inside the supported
viewport
+        * range. The guard band is specified as a horizontal and vertical
+        * distance from (0,0) in clip space.
+        *
+        * This is done by applying the inverse viewport transformation
+        * on the viewport limits to get those limits in clip space.
+        *
+        * Use a limit one pixel smaller to allow for some precision
error.
+        */
+       max_range = SI_MAX_VIEWPORT_RANGE - 1;
+       left   = (-max_range - vp.translate[0]) / vp.scale[0];
+       right  = ( max_range - vp.translate[0]) / vp.scale[0];
+       top    = (-max_range - vp.translate[1]) / vp.scale[1];
+       bottom = ( max_range - vp.translate[1]) / vp.scale[1];
+
+       assert(left < 0 && top < 0 && right > 0 && bottom > 0);


The viewport transform is supplied directly by the user, isn't it? So if
they provide a crazy transform, this assert might trigger. On the other
hand, for reasonable viewport transforms, it should even be the case that
left <= -1.0, right >= 1.0, and so on, right? I guess what I'm saying is
that the spirit of the assert is good and could probably be even
strengthened, but in reality it probably needs to go away.

In a similar spirit, perhaps guardband_x/y should be bounded to be at least
1.0?

glViewport is clamped to ctx->Const.ViewportBounds.Min/Max. This
assures that all guard bands are >= 1. However, if we get invalid
coordinates from other sources, the guard band must prevent generating

That should be "if we get invalid viewport transform from other sources".

I think the assertion should stay and be strengthened to "x >= 1", because:
- we don't know how the discard guard band interacts with the clip guard band
- we should keep the assertion to prevent and detect possible
corruption caused by invalid viewport transformation

Does this sound good?

Yes, sounds good. With these changes, you can add my R-b to this patch as well.

Cheers,
Nicolai


Marek

_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to