[Mesa-dev] [PATCH 2/9] radv: simplify if statement

[Thomas H.P. Andersen]

On Wed, Apr 12, 2017 at 12:50 AM, Bas Nieuwenhuizen
<bas at basnieuwenhuizen.nl> wrote:
> On Wed, Apr 12, 2017 at 12:48 AM, Dave Airlie <airlied at gmail.com> wrote:
>> On 12 April 2017 at 08:19, Bas Nieuwenhuizen <bas at basnieuwenhuizen.nl> wrote:
>>> On Wed, Apr 12, 2017 at 12:04 AM, Thomas Hindoe Paaboel Andersen
>>> <phomes at gmail.com> wrote:
>>>> ---
>>>>  src/amd/vulkan/radv_wsi.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/amd/vulkan/radv_wsi.c b/src/amd/vulkan/radv_wsi.c
>>>> index b8999f4..37cb322 100644
>>>> --- a/src/amd/vulkan/radv_wsi.c
>>>> +++ b/src/amd/vulkan/radv_wsi.c
>>>> @@ -210,7 +210,7 @@ radv_wsi_image_create(VkDevice device_h,
>>>>          * return the fd for the image in the no copy mode,
>>>>          * or the fd for the linear image if a copy is required.
>>>>          */
>>>> -       if (!needs_linear_copy || (needs_linear_copy && linear)) {
>>>> +       if (!needs_linear_copy || linear) {
>>>
>>> Dave, shouldn't this be
>>>
>>> (!needs_linear_copy && !linear) || (needs_linear_copy && linear)
>>
>> The only valid options on the API are
>>
>> needs_linear_copy = FALSE, linear = FALSE
>> needs_linear_copy = TRUE, linear = FALSE
>> needs_linear_copy = TRUE. linear = TRUE.
>>
>> So I suspect the patch is right.
>
> Yeah, I think so too, but I'm wondering if this just reduces clarity.

The intention was to make the code easier to read. If you consider the
code less clear after the change then let's drop this patch.

> Either way no strong opinions on this.
>
>>
>> Dave.