On Thu, Jul 03, 2014 at 11:23:13AM -0700, Ben Widawsky wrote:
> Viewport extents are a 3rd rectangle that defines which pixels get
> discarded as part of the rasterization process. This can potentially
> improve performance by reducing cache usage, and freeing up PS cycles.
> This will get hit if one's viewport is smaller than the full
> renderbuffer, and scissoring is not used.
>
> The functionality itself very much resembles scissoring.
> ---
> src/mesa/drivers/dri/i965/gen8_viewport_state.c | 24 +++++++++++++++---------
> 1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/gen8_viewport_state.c
> b/src/mesa/drivers/dri/i965/gen8_viewport_state.c
> index b366246..2bf5fbb 100644
> --- a/src/mesa/drivers/dri/i965/gen8_viewport_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_viewport_state.c
> @@ -86,17 +86,23 @@ gen8_upload_sf_clip_viewport(struct brw_context *brw)
> vp[10] = -gby; /* y-min */
> vp[11] = gby; /* y-max */
>
> - /* _NEW_SCISSOR | _NEW_VIEWPORT | _NEW_BUFFERS: Screen Space Viewport
> */
> + /* _NEW_SCISSOR | _NEW_VIEWPORT | _NEW_BUFFERS: Screen Space Viewport
> + * The hardware will take the intersection of the drawing rectangle,
> + * scissor rectangle, and the viewport extents. We don't need to be
> + * smart, and can therefore just program the viewport extents.
> + */
> + float viewport_Xmax = ctx->ViewportArray[i].X +
> ctx->ViewportArray[i].Width;
> + float viewport_Ymax = ctx->ViewportArray[i].Y +
> ctx->ViewportArray[i].Height;
These could be both declared as constants and the right hand sides should go
to their own lines as they are now overflowing the 80-char limit.
Otherwise this looks to me as the right thing to do, and not only from
performance point of view. I'm thinking a case where the limits for the
drawbuffer are not set but where the viewport limits are - with the current
logic we wouldn't apply the limits, right? I guess we don't have any such
piglit test case.
But then I also realized that if we applied this, then gen8 wouldn't take the
drawbuffer limits into account anywhere else. So this should break some piglit
tests?
I tried to look at the earlier generations from six onwards and actually
couldn't find any state atom using the drawbuffer limits. (The only reference
is SF-scissor setting in brw_sf_state.c:upload_sf_vp() which is for gen < 6).
I guess I can say I'm confused.
> if (render_to_fbo) {
> - vp[12] = ctx->DrawBuffer->_Xmin;
> - vp[13] = ctx->DrawBuffer->_Xmax - 1;
> - vp[14] = ctx->DrawBuffer->_Ymin;
> - vp[15] = ctx->DrawBuffer->_Ymax - 1;
> + vp[12] = ctx->ViewportArray[i].X;
> + vp[13] = viewport_Xmax - 1;
> + vp[14] = ctx->ViewportArray[i].Y;
> + vp[15] = viewport_Ymax - 1;
> } else {
> - vp[12] = ctx->DrawBuffer->_Xmin;
> - vp[13] = ctx->DrawBuffer->_Xmax - 1;
> - vp[14] = ctx->DrawBuffer->Height - ctx->DrawBuffer->_Ymax;
> - vp[15] = ctx->DrawBuffer->Height - ctx->DrawBuffer->_Ymin - 1;
> + vp[12] = ctx->ViewportArray[i].X;
> + vp[13] = viewport_Xmax - 1;
> + vp[14] = ctx->DrawBuffer->Height - viewport_Ymax;
> + vp[15] = ctx->DrawBuffer->Height - ctx->ViewportArray[i].Y - 1;
> }
>
> vp += 16;
> --
> 2.0.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
