[Mesa-dev] [PATCH 2/2] i965: don't clear resolve map when doing fast depth clears.

[Chad Versace]

On 08/22/2012 03:21 PM, Eric Anholt wrote:
> Paul Berry <stereotype441 at gmail.com> writes:
> 
>> On 22 August 2012 12:35, Eric Anholt <eric at anholt.net> wrote:
>>
>>> Paul Berry <stereotype441 at gmail.com> writes:
>>>
>>>> Previously, when performing a fast depth clear, we would also clear
>>>> the miptree's resolve map.  This destroyed important information,
>>>> since the resolve map contains information about needed resolves for
>>>> all levels and layers of the miptree, whereas a depth clear only
>>>> applies to a single level/layer combination at a time.  As a result,
>>>> resolves would sometimes fail to occur, leading to incorrect
>>>> rendering.
>>>>
>>>> Fixes rendering artifacts with shadow maps in Unigine Heaven and
>>>> Unigine Sanctuary.
>>>
>>> Nice catch!
>>>
>>> However, isn't the issue really that we're incorrectly setting
>>> needs_depth_resolve on layers/levels of the referenced mt that we're not
>>> actually changing hiz state for, since they didn't get cleared?  It
>>> seems like this fixes the current issue, but leaves that bug in place.
>>>
>>
>> No, those other layers of the miptree really did need a depth resolve,
>> because they were previously cleared and then rendered to.  The complete
>> sequence of operations performed by the Unigine engine in the bug looks
>> like this:
> 
> Yeah, I had misread the resolve code in place while insufficiently awake
> (I thought we were setting our new resolve flag on all layers, not just
> the one referenced by the rb).  Not clearing all resolve flags for other
> layers is obviously a requirement.
> 
> I'm ambivalent about losing the assertion, since it caught a bug for
> Chad apparently, but it looks like a good series in that it fixes the
> bug.
> 
> Reviewed-by: Eric Anholt <eric at anholt.net>

It would be nice to preserve the assertion, but this does fix the bug.

Reviewed-by: Chad Versace <chad.versace at linux.intel.com>