[Mesa-dev] [PATCH 1/3] i965: Don't double the depth width for separate-stencil-only rendering.

[Chad Versace]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

The code that this patch fixes is indeed correct, but not for the reason you believe.

On 11/28/2011 10:48 AM, Eric Anholt wrote:
> This looks to have been confused with pitch setup, which does have to
> be doubled.

It was intentional.

> The width is inherited by separate stencil, and it should
> naturally look like the setup in the block below involving a valid
> depthbuffer.

I disagree. The setup in the block below is different because
depth_irb->region->height is the depth buffer's height, but
stencil_irb->region->height is not the stencil buffer's height.
That's because of the hack we resort to when allocating stencil
buffers.

Many months ago, the hack looked like this:
        irb->region = intel_region_alloc(...,
		  rb->Format,
		  I915_TILING_Y
		  cpp * 2,
		  width,
		  (height + 1) / 2);

So, in the line ``((2 * region->height - 1) << 19)``, I was attempting
to adjust for the divide-by-2. (Yes, there is a potential off-by-one
error here, but that's moot now).

Currently, the hack looks like this, both throughout Mesa and in
the DDX:
        irb->mt = intel_miptree_create_for_rendebuffer(
		  intel,
		  rb->Format,
		  I915_TILING_NONE,
		  cpp * 2,
		  ALIGN(width, 64),
		  ALIGN((height + 1) / 2, 64));

So, it is now impossible to determine the renderbuffer's height from
stencil_irb->region->height. Instead, we should directly appeal to
stencil_irb->Base.Height.


> ---
>  src/mesa/drivers/dri/i965/brw_misc_state.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c b/src/mesa/drivers/dri/i965/brw_misc_state.c
> index cb1405c..f6a5ad6 100644
> --- a/src/mesa/drivers/dri/i965/brw_misc_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
> @@ -298,7 +298,7 @@ static void emit_depthbuffer(struct brw_context *brw)
>  	        (BRW_SURFACE_2D << 29));
>        OUT_BATCH(0);
>        OUT_BATCH(((region->width - 1) << 6) |
> -	         (2 * region->height - 1) << 19);
> +	         (region->height - 1) << 19);
>        OUT_BATCH(0);
>        OUT_BATCH(0);

I think this is the diff we want:

> -      OUT_BATCH(((region->width - 1) << 6) |
> -	         (2 * region->height - 1) << 19);
> +      OUT_BATCH(((stencil_irb->Base.Width - 1) << 6) |
> +	         (stencil_irb->Base.Height - 1) << 19);

- ----
Chad Versace
chad.versace at linux.intel.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJO1B1dAAoJEAIvNt057x8iCucP/iltzSWigV6G7rO4Ngg55j+8
ZEWEG5uLAv77ULqGdScoDupX3SSARvDU+dsKGbg1jL8vBg3lxZcHJji8Jb46/5pe
YnbKg/BSA4OQgt/GNBtYmjF+nFAipOqWxiQdaN8ytAGhraykOgJMTVx06Pw6+xZU
V/PRlRbLef7ECZT9FvI+XThiVJnyPq+OGQhpO6rYxXzUlRnutsLH0c1dohvoqHFh
qD6577KK0hHjVFK+duXg6Y1N5QPWlFi5PfqUbf8UcPBYv/tAHtYyfCghaDuqtN1w
uNTdv0w/2dQkgWVpBrRtZQIwGDRxoH4YN7aQi1gW4plWI6FED1XHmP5DllLH/4lI
W7foFGZb3raztYDjotTV/BstIcIXm1B0ASLv8ljjOjA17t0uZarZQKapLTNYMwd8
KUCtp2Sgr7EC7Teuck3tsaQLZxJinpkIWhqotajSNjWtdww5HG7Kdkc/zQDJORF8
Ri04nTZ94RMHOfGq3/+/9J/S5zFlCMbcDrFd9c7vCJ6bGrThHEEwe18HTXYJ/fOK
KlgrgTgEcmPKN32ExSkbZQm++XDenQEtQTo4Z9Nj8lQC51vWbcsPphXq3BWW8orc
ju/iTFmSRNXWDoL0W7PlL7pmfX2TuLTjZV0V4Ei/BUSQXt5oRx2n7VCExXQZvdOC
H9sZeKNNiQxm9VvlZxPT
=31Dx
-----END PGP SIGNATURE-----