On Monday, August 04, 2014 12:24:04 PM Ben Widawsky wrote: > On GEN8, a change in scissor state does not effect anything for the > clipper/sf hardware state. The hardware will always do the right thing > once the viewport extents are programmed. We can therefore remove the > unecessary state emission. > > Ken originally spotted this.
Scissoring state affects the value of ctx->DrawBuffer->_Xmin, _Xmax, _Ymin, and
_Ymax. So, _NEW_SCISSORS was actually necessary prior to your patch #2.
Perhaps reword the commit message to be something like this:
Now that we no longer use ctx->DrawBuffer->_Xmin and related fields to program
the screen-space viewport extents, we don't depend on any scissoring state. So
we can drop the _NEW_SCISSOR dependency.
> ---
> src/mesa/drivers/dri/i965/gen8_viewport_state.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/gen8_viewport_state.c
> b/src/mesa/drivers/dri/i965/gen8_viewport_state.c
> index 04a4530..d7e06c4 100644
> --- a/src/mesa/drivers/dri/i965/gen8_viewport_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_viewport_state.c
> @@ -100,13 +100,17 @@ 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_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;
> + if (viewport_Ymax < ctx->DrawBuffer->_Ymax ||
> + viewport_Xmax < ctx->DrawBuffer->_Xmax) {
> + perf_debug("Using viewport extents for savings\n");
> + }
I suspect you didn't mean to add this :) Please drop it, as using
ctx->DrawBuffer->_Xmax actually introduces a dependency on _NEW_SCISSOR again.
:)
With those fixed, this would get a:
Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
> if (render_to_fbo) {
> vp[12] = ctx->ViewportArray[i].X;
> vp[13] = viewport_Xmax - 1;
> @@ -130,7 +134,7 @@ gen8_upload_sf_clip_viewport(struct brw_context *brw)
>
> const struct brw_tracked_state gen8_sf_clip_viewport = {
> .dirty = {
> - .mesa = _NEW_BUFFERS | _NEW_SCISSOR | _NEW_VIEWPORT,
> + .mesa = _NEW_BUFFERS | _NEW_VIEWPORT,
> .brw = BRW_NEW_BATCH,
> .cache = 0,
> },
>
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
