[Mesa-dev] [PATCH 1/7] i965/fs_surface_builder: Explicitly handle FORMAT_NONE in num_image_coordinates

[Jason Ekstrand]

On Fri, Nov 20, 2015 at 5:49 AM, Francisco Jerez <currojerez at riseup.net> wrote:
> Chad Versace <chad.versace at intel.com> writes:
>
>> On Wed 04 Nov 2015, Jason Ekstrand wrote:
>>> Previously, we were relying on has_matching_typed_format returning true for
>>> MESA_FORMAT_NONE which, in turn, relied on _mesa_get_format_bytes returning
>>> 1 for MESA_FORMAT_NONE.  All of this is extremely non-obvious.  Instead,
>>> this commit makes us handle it explicitly.
>>> ---
>>>  src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp b/src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp
>>> index 534d849..31ecb5b 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp
>>> +++ b/src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp
>>> @@ -409,6 +409,7 @@ namespace {
>>>            * reads want the array index to be at the Z component.
>>>            */
>>>           const bool array_index_at_z =
>>> +            format != MESA_FORMAT_NONE &&
>>>              !image_format_info::has_matching_typed_format(
>>>                 bld.shader->devinfo, format);
>>>           const unsigned zero_dims =
>>
>>
>> Knowing nothing about the implicit assumptions you discovered that
>> relied on _mesa_get_format_bytes(MESA_FORMAT_NONE) => 1, the patch is
>> still looks like an improvement to me.
>>
> It didn't.  It relied on _mesa_get_format_bytes(MESA_FORMAT_NONE) not
> being greater than 4, which seems sensible anyway.

I can change the commit message to say

Previously, we were relying on has_matching_typed_format returning true for
MESA_FORMAT_NONE which, in turn, relied on _mesa_get_format_bytes returning
a value <= 4 for MESA_FORMAT_NONE.  While reliable, this is extremely
non-obvious.  Instead,
this commit makes us handle it explicitly.

>> Acked-by: Chad Versace <chad.versace at intel.com>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev