[Mesa-dev] [PATCH] i915: Re-enable Z16/Z24 formats.

[Reimar Döffinger]

On 9 Aug 2012, at 00:16, Eric Anholt <eric at anholt.net> wrote:
> Reimar Döffinger <Reimar.Doeffinger at gmx.de> writes:
> 
>> This allows MPlayer to (mis-)use depth textures to quickly upload
>> and render 16-bit per component YUV video.
>> There is still the issue that the actual precision is only around
>> 10-bits, at least when using it in the way MPlayer does.
>> If someone knows an explanation for that I'd like to hear it, however
>> it is not related to this change as the precision is the same
>> with Z16 disable which ends up converting to X8Z24 first.
> 
> The floats in the fragment shader only have about 10 bits of precision.

Hm, aren't they normal 16-bit floats? Those have 11 bit.
Also they have 11 bit over the whole range, however what I see is that if you use only the lower 9 bits you will only get about 8 different shades.
I can imagine things that could cause this (like they skipped on unorm-> float conversion hardware and instead create denormal half-float from the highest bits of the input and "convert" by multiplying by a magic float constant), but it seems a bit crazy.

>> Signed-off-by: Reimar Döffinger <Reimar.Doeffinger at gmx.de>
>> ---
>> src/mesa/drivers/dri/i915/i915_context.c |    9 ---------
>> 1 file changed, 9 deletions(-)
>> 
>> diff --git a/src/mesa/drivers/dri/i915/i915_context.c b/src/mesa/drivers/dri/i915/i915_context.c
>> index dc32292..4de49f5 100644
>> --- a/src/mesa/drivers/dri/i915/i915_context.c
>> +++ b/src/mesa/drivers/dri/i915/i915_context.c
>> @@ -113,17 +113,8 @@ intel_init_texture_formats(struct gl_context *ctx)
>>    ctx->TextureFormatSupported[MESA_FORMAT_S8_Z24] = true;
>>    ctx->TextureFormatSupported[MESA_FORMAT_X8_Z24] = true;
>>    ctx->TextureFormatSupported[MESA_FORMAT_S8] = intel->has_separate_stencil;
>> -
>> -   /*
>> -    * This was disabled in initial FBO enabling to avoid combinations
>> -    * of depth+stencil that wouldn't work together.  We since decided
>> -    * that it was OK, since it's up to the app to come up with the
>> -    * combo that actually works, so this can probably be re-enabled.
>> -    */
>> -   /*
>>    ctx->TextureFormatSupported[MESA_FORMAT_Z16] = true;
>>    ctx->TextureFormatSupported[MESA_FORMAT_Z24] = true;
>> -   */
> 
> Before pushing a change like this, we'd need to add FBO validation code
> for the combinations that don't work (16 vs 32bpp between depth and
> color).
> 
> Also, there is no Z24 (that's 3 bytes per pixel) support in the
> hardware.  I would guess that a piglit run would have shown that.

Admittedly I was hoping I could just motivate someone to do the necessary work in the details, I doubt I'll have time to get into the details doing these things.
But if not, maybe removing the clearly wrong Z24 case and updating the comment to mention the additional tests needed before re-enabling would be a help to the next person looking at it?