[Mesa-dev] [PATCH 4/4] main: Use a derived value for the default sample count

[Ilia Mirkin]

On Thu, Feb 4, 2016 at 11:17 AM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Thu, Feb 4, 2016 at 11:12 AM, Neil Roberts <neil at linux.intel.com> wrote:
>> Previously the framebuffer default sample count was taken directly
>> from the value given by the application. On the i965 driver on HSW if
>> the value wasn't one that is supported by the hardware it would hit an
>> assert when it tried to program the state for it. This patch fixes it
>> by adding a derived sample count to the state for the default
>> framebuffer. The driver can then quantize this to one of the valid
>> values in its UpdateState handler when the _NEW_BUFFERS state changes.
>> _mesa_geometric_samples is changed to use the new derived value.
>>
>> Fixes the piglit test arb_framebuffer_no_attachments-query
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93957
>> Cc: Ilia Mirkin <imirkin at alum.mit.edu>
>> ---
>>  src/mesa/drivers/dri/i965/brw_context.c | 19 +++++++++++++++++++
>>  src/mesa/main/framebuffer.h             |  3 ++-
>>  src/mesa/main/mtypes.h                  |  4 ++++
>>  3 files changed, 25 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
>> index 1032e5a..44d2fe4 100644
>> --- a/src/mesa/drivers/dri/i965/brw_context.c
>> +++ b/src/mesa/drivers/dri/i965/brw_context.c
>> @@ -167,6 +167,19 @@ intel_viewport(struct gl_context *ctx)
>>  }
>>
>>  static void
>> +intel_update_framebuffer(struct gl_context *ctx,
>> +                         struct gl_framebuffer *fb)
>> +{
>> +   struct brw_context *brw = brw_context(ctx);
>> +
>> +   /* Quantize the derived default number of samples
>> +    */
>
> Not a lot of effort either way, but this only needs to be done if
> !fb->_HasAttachments right?

BTW, whether you choose to condition it on _HasAttachments or not, the
whole series is

Reviewed-by: Ilia Mirkin <imirkin at alum.mit.edu>

>
>> +   fb->DefaultGeometry._NumSamples =
>> +      intel_quantize_num_samples(brw->intelScreen,
>> +                                 fb->DefaultGeometry.NumSamples);
>> +}
>> +
>> +static void
>>  intel_update_state(struct gl_context * ctx, GLuint new_state)
>>  {
>>     struct brw_context *brw = brw_context(ctx);
>> @@ -245,6 +258,12 @@ intel_update_state(struct gl_context * ctx, GLuint new_state)
>>     }
>>
>>     _mesa_lock_context_textures(ctx);
>> +
>> +   if (new_state & _NEW_BUFFERS) {
>> +      intel_update_framebuffer(ctx, ctx->DrawBuffer);
>> +      if (ctx->DrawBuffer != ctx->ReadBuffer)
>> +         intel_update_framebuffer(ctx, ctx->ReadBuffer);
>> +   }
>>  }
>>
>>  #define flushFront(screen)      ((screen)->image.loader ? (screen)->image.loader->flushFrontBuffer : (screen)->dri2.loader->flushFrontBuffer)
>> diff --git a/src/mesa/main/framebuffer.h b/src/mesa/main/framebuffer.h
>> index ab077ed..fa434d4 100644
>> --- a/src/mesa/main/framebuffer.h
>> +++ b/src/mesa/main/framebuffer.h
>> @@ -97,7 +97,8 @@ static inline GLuint
>>  _mesa_geometric_samples(const struct gl_framebuffer *buffer)
>>  {
>>     return buffer->_HasAttachments ?
>> -      buffer->Visual.samples : buffer->DefaultGeometry.NumSamples;
>> +      buffer->Visual.samples :
>> +      buffer->DefaultGeometry._NumSamples;
>>  }
>>
>>  static inline GLuint
>> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
>> index 58064aa..745eeb8 100644
>> --- a/src/mesa/main/mtypes.h
>> +++ b/src/mesa/main/mtypes.h
>> @@ -3223,6 +3223,10 @@ struct gl_framebuffer
>>     struct {
>>       GLuint Width, Height, Layers, NumSamples;
>>       GLboolean FixedSampleLocations;
>> +     /* Derived from NumSamples by the driver so that it can choose a valid
>> +      * value for the hardware.
>> +      */
>> +     GLuint _NumSamples;
>>     } DefaultGeometry;
>>
>>     /** \name  Drawing bounds (Intersection of buffer size and scissor box)
>> --
>> 2.5.0
>>