[Mesa-dev] [PATCH 2/2] mesa: clamp viewport values only once when using glViewport()
Nicolai Hähnle
nhaehnle at gmail.com
Mon Jul 31 10:02:51 UTC 2017
On 28.07.2017 15:47, Samuel Pitoiset wrote:
> It's useless to clamp the same values for all viewports.
>
> +7% in the "viewport change" test (drawoverhead benchmark).
>
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
I think it would be cleaner to call clamp_viewport in all callers of
set_viewport_no_notify. Fewer boolean function args is better and
clearer in my book.
With that changed, the patch is
Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>
> ---
> src/mesa/main/viewport.c | 36 ++++++++++++++++++++++++------------
> 1 file changed, 24 insertions(+), 12 deletions(-)
>
> diff --git a/src/mesa/main/viewport.c b/src/mesa/main/viewport.c
> index e6004bad40..1cff4d846e 100644
> --- a/src/mesa/main/viewport.c
> +++ b/src/mesa/main/viewport.c
> @@ -36,13 +36,12 @@
> #include "viewport.h"
>
> static void
> -set_viewport_no_notify(struct gl_context *ctx, unsigned idx,
> - GLfloat x, GLfloat y,
> - GLfloat width, GLfloat height)
> +clamp_viewport(struct gl_context *ctx, GLfloat *x, GLfloat *y,
> + GLfloat *width, GLfloat *height)
> {
> /* clamp width and height to the implementation dependent range */
> - width = MIN2(width, (GLfloat) ctx->Const.MaxViewportWidth);
> - height = MIN2(height, (GLfloat) ctx->Const.MaxViewportHeight);
> + *width = MIN2(*width, (GLfloat) ctx->Const.MaxViewportWidth);
> + *height = MIN2(*height, (GLfloat) ctx->Const.MaxViewportHeight);
>
> /* The GL_ARB_viewport_array spec says:
> *
> @@ -55,11 +54,20 @@ set_viewport_no_notify(struct gl_context *ctx, unsigned idx,
> if (ctx->Extensions.ARB_viewport_array ||
> (ctx->Extensions.OES_viewport_array &&
> _mesa_is_gles31(ctx))) {
> - x = CLAMP(x,
> - ctx->Const.ViewportBounds.Min, ctx->Const.ViewportBounds.Max);
> - y = CLAMP(y,
> - ctx->Const.ViewportBounds.Min, ctx->Const.ViewportBounds.Max);
> + *x = CLAMP(*x,
> + ctx->Const.ViewportBounds.Min, ctx->Const.ViewportBounds.Max);
> + *y = CLAMP(*y,
> + ctx->Const.ViewportBounds.Min, ctx->Const.ViewportBounds.Max);
> }
> +}
> +
> +static void
> +set_viewport_no_notify(struct gl_context *ctx, unsigned idx,
> + GLfloat x, GLfloat y,
> + GLfloat width, GLfloat height, bool clamped)
> +{
> + if (!clamped)
> + clamp_viewport(ctx, &x, &y, &width, &height);
>
> if (ctx->ViewportArray[idx].X == x &&
> ctx->ViewportArray[idx].Width == width &&
> @@ -89,6 +97,10 @@ static void
> viewport(struct gl_context *ctx, GLint x, GLint y, GLsizei width,
> GLsizei height)
> {
> + /* Clamp the viewport to the implementation dependent values. */
> + clamp_viewport(ctx, (GLfloat *)&x, (GLfloat *)&y,
> + (GLfloat *)&width, (GLfloat *)&height);
> +
> /* The GL_ARB_viewport_array spec says:
> *
> * "Viewport sets the parameters for all viewports to the same values
> @@ -101,7 +113,7 @@ viewport(struct gl_context *ctx, GLint x, GLint y, GLsizei width,
> * signal the driver once at the end.
> */
> for (unsigned i = 0; i < ctx->Const.MaxViewports; i++)
> - set_viewport_no_notify(ctx, i, x, y, width, height);
> + set_viewport_no_notify(ctx, i, x, y, width, height, true);
>
> if (ctx->Driver.Viewport)
> ctx->Driver.Viewport(ctx);
> @@ -153,7 +165,7 @@ void
> _mesa_set_viewport(struct gl_context *ctx, unsigned idx, GLfloat x, GLfloat y,
> GLfloat width, GLfloat height)
> {
> - set_viewport_no_notify(ctx, idx, x, y, width, height);
> + set_viewport_no_notify(ctx, idx, x, y, width, height, false);
>
> if (ctx->Driver.Viewport)
> ctx->Driver.Viewport(ctx);
> @@ -165,7 +177,7 @@ viewport_array(struct gl_context *ctx, GLuint first, GLsizei count,
> {
> for (GLsizei i = 0; i < count; i++) {
> set_viewport_no_notify(ctx, i + first, inputs[i].X, inputs[i].Y,
> - inputs[i].Width, inputs[i].Height);
> + inputs[i].Width, inputs[i].Height, false);
> }
>
> if (ctx->Driver.Viewport)
>
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
More information about the mesa-dev
mailing list