[Mesa-dev] [PATCH 5/5] intel: Remove intel_renderbuffer::hiz_region

[Chad Versace]

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

On 10/18/2011 03:57 PM, Eric Anholt wrote:
> On Mon, 17 Oct 2011 07:40:28 -0700, Chad Versace <chad at chad-versace.us> wrote:
>> Replace it with intel_renderbuffer::region::hiz::region.
>>
>> Signed-off-by: Chad Versace <chad at chad-versace.us>
>> ---
>>  src/mesa/drivers/dri/i965/brw_misc_state.c     |   27 +++++++++--------
>>  src/mesa/drivers/dri/intel/intel_context.c     |   10 ++++--
>>  src/mesa/drivers/dri/intel/intel_fbo.c         |   36 +++++------------------
>>  src/mesa/drivers/dri/intel/intel_fbo.h         |    3 --
>>  src/mesa/drivers/dri/intel/intel_mipmap_tree.c |    1 -
>>  src/mesa/drivers/dri/intel/intel_mipmap_tree.h |   14 ---------
>>  6 files changed, 29 insertions(+), 62 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c b/src/mesa/drivers/dri/i965/brw_misc_state.c
>> index 2e6780b..6b521e0 100644
>> --- a/src/mesa/drivers/dri/i965/brw_misc_state.c
>> +++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
> 
>> @@ -583,21 +574,10 @@ intel_update_tex_wrapper_regions(struct intel_context *intel,
>>  
>>     /* Allocate the texture's hiz region if necessary. */
>>     if (intel->vtbl.is_hiz_depth_format(intel, rb->Format)
>> -       && !intel_image->mt->hiz_region) {
>> -      intel_image->mt->hiz_region =
>> -         intel_region_alloc(intel->intelScreen,
>> -                            I915_TILING_Y,
>> -                            _mesa_get_format_bytes(rb->Format),
>> -                            rb->Width,
>> -                            rb->Height,
>> -                            GL_TRUE);
>> -      if (!intel_image->mt->hiz_region)
>> -         return GL_FALSE;
>> -   }
>> -
>> -   /* Point the renderbuffer's hiz region to the texture's hiz region. */
>> -   if (irb->hiz_region != intel_image->mt->hiz_region) {
>> -      intel_region_reference(&irb->hiz_region, intel_image->mt->hiz_region);
>> +       && irb->region->hiz.region == NULL) {
>> +      bool ok = intel_renderbuffer_alloc_hiz(intel, irb);
>> +      if (!ok)
>> +	 return false;
>>     }
> 
> I think you're aware of this already, but just in case:
> 
> The hiz region allocated here using the new rb_alloc_hiz looks like it
> will be based on the size of the whole texture miptree (since it looks
> up the region's size, not rb w/h), while all that should be needed is
> the maximum size of any texture image.

I was aware of that, and chose to do that because it was compatible with my
vague plan of handling miptrees. But...

> 
> I think we could just use Width/Height, if we resolved at
> FinishRenderTexture time and freed the renderbuffer's hiz region (which
> I think would be reasonable for now, and should make mipmapped depth hiz
> pretty straightforward).

I like that approach. It's much simpler than my original plan, and it shouldn't cause
unnecessary resolves for apps that are written correctly. After rendering to a depth texture,
the app, if sensible, will eventually sample from the texture. The depth resolve must happen
eventually, so we might as well straightforwardly do it when detaching the texture.

At first, I though this plan would cause problems when multiple images/layers/levels of the same texture
object were attached to an FBO. But I no longer see a problem there. An FBO has only one
depth attachment point, so each image would be attached to a different FBO, which ensures
that FinishRenderTexture gets called each time we render to a new image/layer/level.

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

iQIcBAEBAgAGBQJOnxMaAAoJEAIvNt057x8iqMAQAMEYYxdHOVykZ5OuGw/aD3w8
EcwI7wc1KLIMT3aZv7vNZ1h0m2E9a024l2UyG52Oz1hB1tJXyDepEmHYUDWTj91n
dccV1c0D//pvir/oh6H48Ewh84/nBr4MGZs+mtWfAbw90FpDXX3i7IB37wnnwdSZ
kaXhA8zD4RhdL/LSqLWgazWQ+s4k75L/6x9dMRD8uZtj6bHKfJs6kyP9PnFhzgjh
wNF85BQ9xNUs6U2wCbF7Akja8dB6eIc5OQjC/HlyRmwH/pRJbSq2ip7SFAPdG33K
+dZ3uNG0grovRgjxsDuSqwhBPdXlI7rF0VVbRZDke7oAAtxsJdtjV1VizyiuUOCB
5ayI/50OkPpvbAKR+gJN89DP+0UYk9O/vq9t8L19YQ5PuZIpfy+Gh5PeA0wleGeJ
+3SCb7/QogDCM0i9pcS/NZaoIO/qe3Ond/HWo7x3DdeixUBMWqWETlUQ0R9B/ANT
LrHF+0IQ+3yYC6XK3AoJC63SJKLP1XQBz9lUFk4ns/BNIU/eP2dFbYlskxjpOYBf
vB40IWkZocDcKAOvEiXutU3Ori7MRDF6X4Jg+QvjM0zS1HrX50Vrql0KpJmzO27f
b6Y4BLT6rXtNI3pw6vFk1071SZbFXZza22tf1B5m6y9XVdiOogkSjB3Or+YBUU8l
T7N/ct8+to1QQA6r3VZa
=YWg+
-----END PGP SIGNATURE-----