[Mesa-dev] [PATCH] i965: Add driconf option clamp_max_samples

[Chad Versace]

On 11/02/2013 12:21 PM, Kenneth Graunke wrote:
> On 11/02/2013 08:41 AM, Chad Versace wrote:
>> This clamps GL_MAX_SAMPLES to the given value. If negative, then no
>> clamping occurs.
>>
>> CC: Paul Berry <stereotype441 at gmail.com>
>> CC: Eric Anholt <eric at anholt.net>
>> Signed-off-by: Chad Versace <chad.versace at linux.intel.com>
>> ---
>>
>>
>> This patch lives on my branch driconf-clamp-max_samples.
>>

> I also think "supported_msaa_modes" might be nicer than "legal_max_samples."
> Again, up to you though.

Done. I renamed it to supported_msaa_modes.

> Would you mind adding this:
>
> if (brw->gen >= 8) {
>     legal_max_samples = (int[]){16, 8, 4, 2, 0};
> } else if (brw->gen >= 7) {

I don't want to add that. The patch would do more than its commit message claims.
After all, adding the MSAA modes for gen8 is orthogonal to adding a new driconf
option.


>> +   /* Set max_samples = min(max(legal_max_samples), clamp_max_samples). */
>
> It doesn't actually - if I specify clamp_max_samples=3, I won't get max_samples == 3 :)
>
>> +   max_samples = 0;
>> +   for (int i = 0; legal_max_samples[i] != 0; ++i) {
>> +      if (legal_max_samples[i] <= clamp_max_samples) {
>> +         max_samples = legal_max_samples[i];
>> +         break;
>> +      }
>>      }
>
> This might be clearer as:
>
>     int max_samples = legal_max_samples[0];
>     int clamp_max_samples = driQueryOptioni(&brw->optionCache, "clamp_max_samples");
>     if (clamp_max_samples > 0) {
>        /* Select the largest supported MSAA mode at or below clamp_max_samples. */
>        for (int i = 0; legal_max_samples[i] != 0; ++i) {
>           if (legal_max_samples[i] <= clamp_max_samples) {
>              max_samples = legal_max_samples[i];
>              break;
>           }
>        }
>     }
>
> Since the true maximum is always the first element, you can just set it to that,
> and override it to a smaller value when clamped.

Ok. I reworked the loop to look more like yours.


> That said, there's another larger problem with this patch:
>
> $ clamp_max_samples=2 glxinfo |& grep 'OpenGL version'
> OpenGL version string: 2.1 Mesa 10.0.0-devel (git-1733459)
>
> I would prefer to see it not affect the version advertised.  This probably means
> overriding these values later.

Oops! I didn't intend to change the context version.

To fix that, I moved the override below _mesa_compute_version().

Patch v3 is sent.