[Mesa-dev] [PATCH 23/23] i965: Skip uploading border color when unnecessary.

[Chris Forbes]

Seems reasonable to me. Always nice to save a bit more batchbuffer space.

Reviewed-by: Chris Forbes <chrisf at ijw.co.nz>

On Wed, Oct 8, 2014 at 9:57 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> On Tuesday, July 29, 2014 04:29:28 PM Kenneth Graunke wrote:
>> The border color is only needed when using the
> GL_CLAMP_TO_BORDER or
>> (deprecated) GL_CLAMP wrap modes; all others ignore it, including
> the
>> common GL_CLAMP_TO_EDGE and GL_REPEAT wrap modes.
>>
>> In those cases, we can skip uploading it entirely, saving a bit of
> space
>> in the batchbuffer.  Instead, we just point it at the start of the
>> batch (offset 0); we have to program something, and that address is
> safe
>> to read.
>>
>> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
>> ---
>>  src/mesa/drivers/dri/i965/brw_sampler_state.c | 22
> ++++++++++++++++++++--
>>  1 file changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_sampler_state.c
>> b/src/mesa/drivers/dri/i965/brw_sampler_state.c index
> f48e3c9..ad9a527
>> 100644
>> --- a/src/mesa/drivers/dri/i965/brw_sampler_state.c
>> +++ b/src/mesa/drivers/dri/i965/brw_sampler_state.c
>> @@ -161,6 +161,16 @@ translate_wrap_mode(struct brw_context
> *brw, GLenum
>> wrap, bool using_nearest) }
>>
>>  /**
>> + * Return true if the given wrap mode requires the border color to
> exist.
>> + */
>> +static bool
>> +wrap_mode_needs_border_color(unsigned wrap_mode)
>> +{
>> +   return wrap_mode == BRW_TEXCOORDMODE_CLAMP_BORDER ||
>> +          wrap_mode == GEN8_TEXCOORDMODE_HALF_BORDER;
>> +}
>> +
>> +/**
>>   * Upload SAMPLER_BORDER_COLOR_STATE.
>>   */
>>  static void
>> @@ -410,8 +420,16 @@ brw_update_sampler_state(struct
> brw_context *brw,
>>                        lod_bits);
>>     base_level = U_FIXED(0, 1);
>>
>> -   uint32_t border_color_offset;
>> -   upload_default_color(brw, sampler, unit, &border_color_offset);
>> +   /* Upload the border color if necessary.  If not, just point it at
>> +    * offset 0 (the start of the batch) - the color should be ignored,
>> +    * but that address won't fault in case something reads it anyway.
>> +    */
>> +   uint32_t border_color_offset = 0;
>> +   if (wrap_mode_needs_border_color(wrap_s) ||
>> +       wrap_mode_needs_border_color(wrap_t) ||
>> +       wrap_mode_needs_border_color(wrap_r)) {
>> +      upload_default_color(brw, sampler, unit, &border_color_offset);
>> +   }
>>
>>     brw_emit_sampler_state(brw,
>>                            sampler_state,
>
> I don't think I ever received any review on this patch (23/23).  The rest
> got pushed a long time ago.
>
> Thoughts?
> --Ken
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>