On 11/18/2011 05:03 PM, Eric Anholt wrote:
> On Fri, 18 Nov 2011 12:50:36 -0800, Chad Versace
> <chad.vers...@linux.intel.com> wrote:
>> Essentially, this patch just globally substitutes `irb->region` with
>> `irb->mt->region` and then does some minor cleanups to avoid segfaults
>> and other problems.
>>
>> This is in preparation for
>> 1. Fixing scatter/gather for mipmapped separate stencil textures.
>> 2. Supporting HiZ for mipmapped depth textures.
>>
>> As a nice benefit, this lays down some preliminary groundwork for easily
>> texturing from any renderbuffer, even those of the window system.
>>
>> A future commit will replace intel_mipmap_tree::hiz_region with a miptree.
>>
>> v2:
>> - Return early in intel_process_dri2_buffer_*() if region allocation
>> fails.
>> - Fix double semicolon.
>> - Fix miptree reference leaks in the following functions:
>> intel_process_dri2_buffer_with_separate_stencil()
>> intel_image_target_renderbuffer_storage()
>
>> @@ -702,20 +696,21 @@ intel_alloc_renderbuffer_storage(struct gl_context *
>> ctx, struct gl_renderbuffer
>> _mesa_reference_renderbuffer(&irb->wrapped_stencil, stencil_rb);
>>
>> } else {
>> - irb->region = intel_region_alloc(intel->intelScreen, tiling, cpp,
>> - width, height, true);
>> - if (!irb->region)
>> + irb->mt = intel_miptree_create_for_renderbuffer(intel, rb->Format,
>> + tiling, cpp,
>> + width, height);
>> + if (!irb->mt)
>> return false;
>>
>> if (intel->vtbl.is_hiz_depth_format(intel, rb->Format)) {
>> - irb->hiz_region = intel_region_alloc(intel->intelScreen,
>> - I915_TILING_Y,
>> - irb->region->cpp,
>> - irb->region->width,
>> - irb->region->height,
>> - true);
>> - if (!irb->hiz_region) {
>> - intel_region_release(&irb->region);
>> + irb->mt->hiz_region = intel_region_alloc(intel->intelScreen,
>> + I915_TILING_Y,
>> + cpp,
>> + rb->Width,
>> + rb->Height,
>> + true);
>> + if (!irb->mt) {
>> + intel_miptree_release(&irb->mt);
>> return false;
>
> I think this was meant to be if (!irb->mt->his_region).
>
> Other than that,
>
> Reviewed-by: Eric Anholt <e...@anholt.net>
I really meant to release irb->mt, not irb->mt->hiz_mt. Perhaps rephrasing
the code like this reveals the code's intention more explicity.
irb->mt = intel_miptree_create_for_renderbuffer(...)
if (!irb->mt)
return false;
if (intel->vtbl.is_hiz_depth_format(intel, rb->Format)) {
intel_miptree_alloc_hiz(intel, irb->mt);
if (!irb->mt->hiz_mt) {
intel_miptree_release(&irb->mt);
return false;
}
}
I chose to release the renderbuffer when the hiz miptree allocation fails
because on gen6, hiz and separate stencil must be enabled together. If,
when hiz miptree allocation failed, we allowed intel_alloc_rb_storage()
to continue and succeed, this problematic situation could occur:
1. User creates a GL_DEPTH_COMPONENT24 renderbuffer. HiZ miptree allocation
failed.
2. User attaches the depthbuffer to a fb and does some drawing. No problem.
3. User creates a GL_STENCIL_INDEX8 renderbuffer.
4. User attaches the stencilbuffer to the same fb. Here we must to detect
that the accompanying depthbuffer, without hiz, is incompatible, and
mark the fb as incomplete.
5. User says WTF.
I'm withholding your rb until you reply.
----
Chad Versace
chad.vers...@linux.intel.com
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
