[Mesa-dev] [PATCH 1/3] mesa: only emit _NEW_MULTISAMPLE when coverage parameters change

[Samuel Pitoiset]

On 06/05/2017 06:00 PM, Brian Paul wrote:
> On 06/05/2017 09:26 AM, Samuel Pitoiset wrote:
>>
>>
>> On 06/02/2017 09:11 PM, Brian Paul wrote:
>>> On 06/02/2017 09:52 AM, Samuel Pitoiset wrote:
>>>> We usually check that given parameters are different before
>>>> updating the state.
>>>>
>>>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>>>> ---
>>>>   src/mesa/main/multisample.c | 10 +++++++---
>>>>   1 file changed, 7 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/src/mesa/main/multisample.c b/src/mesa/main/multisample.c
>>>> index 5453e38632..f0e7a61180 100644
>>>> --- a/src/mesa/main/multisample.c
>>>> +++ b/src/mesa/main/multisample.c
>>>> @@ -41,11 +41,15 @@ _mesa_SampleCoverage(GLclampf value, GLboolean
>>>> invert)
>>>>   {
>>>>      GET_CURRENT_CONTEXT(ctx);
>>>>
>>>> -   FLUSH_VERTICES(ctx, 0);
>>>> +   value = CLAMP(value, 0.0f, 1.0f);
>>>> +
>>>> +   if (ctx->Multisample.SampleCoverageInvert == invert &&
>>>> +       ctx->Multisample.SampleCoverageValue == value)
>>>> +      return;
>>>>
>>>> -   ctx->Multisample.SampleCoverageValue = CLAMP(value, 0.0f, 1.0f);
>>>> +   FLUSH_VERTICES(ctx, _NEW_MULTISAMPLE);
>>>> +   ctx->Multisample.SampleCoverageValue = value;
>>>>      ctx->Multisample.SampleCoverageInvert = invert;
>>>> -   ctx->NewState |= _NEW_MULTISAMPLE;
>>>>   }
>>>>
>>>>
>>>>
>>>
>>> In this patch and 3, you could leave the CLAMP() where it was.
>>
>> Yeah, but it looks like we usually do the CLAMP() before checking the
>> values.
>>
>> Except this, any other comments?
> 
> Looks fine otherwise.

Is your previous comment just a suggestion? Or do you really want this 
CLAMP change? Your call, I can send a v2 if you want.

> 
> -Brian
> 
>