On Mon, Apr 11, 2016 at 7:37 PM, Nicolai Hähnle <nhaeh...@gmail.com> wrote:
> On 10.04.2016 17:34, Marek Olšák wrote:
>>
>> From: Marek Olšák <marek.ol...@amd.com>
>>
>> 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 <g...@chown.ath.cx>.
>> ---
>> 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
coordinates outside of the viewport bounds. That means we must allow
guard bands in 0 < x < 1. Sadly, I don't know how the discard guard
band affects all this - it may trivially accept all primitives at x <
1.
Removing the assertion is probably not a good idea. I think it's
better to fail and terminate the program than get undebuggable
corruption.
Marek
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
