[Mesa-dev] [PATCH 4/4] i965: Change assertion condition from implicit to explicit

[Chad Versace]

On 04/12/2011 03:52 PM, Ian Romanick wrote:
> On 04/12/2011 03:33 PM, chad at chad-versace.us wrote:
>> From: Chad Versace <chad at chad-versace.us>
> 
>> ... because grokking explicit assertions requires fewer neurons.
> 
>> In brw_misc_state.c:emit_depthbuffer, change
>>     assert(tiling != I915_TILING_X && tiling != I915_TILING_NONE)
>> to
>>     assert(tiling == I915_TILING_Y)
> 
>> Signed-off-by: Chad Versace <chad at chad-versace.us>
>> ---
>>  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 74e911b..617712e 100644
>> --- a/src/mesa/drivers/dri/i965/brw_misc_state.c
>> +++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
>> @@ -249,7 +249,7 @@ static void emit_depthbuffer(struct brw_context *brw)
> 
>>        assert(region->tiling != I915_TILING_X);
>>        if (intel->gen >= 6)
>> -	 assert(region->tiling != I915_TILING_NONE);
>> +	 assert(region->tiling != I915_TILING_Y);
>                                ^^
> Based on the commit message, I think you meant == here.

Yes.

> Also, I hate code like this.  I've never been a fan of empty
> if-statements like this.  Unless someone else objects, I'd prefer:
> 
> 	/* X-tiling is never allowed for the depth buffer.  GEN6 and
> 	 * later require y-tiling for the depth buffer.
> 	 */
> 	assert(region->tiling != I915_TILING_X);
> 	assert(intel->gen < 6 || region->tiling == I915_TILING_Y);
> 
> 
>>        BEGIN_BATCH(len);
>>        OUT_BATCH(_3DSTATE_DEPTH_BUFFER << 16 | (len - 2));

I prefer ``!x || y`` to ``if (x) y`` too, but didn't think others would.
I'll change it accordingly.

-- 
Chad Versace
chad at chad-versace.us