[Mesa-dev] [PATCH] EGL/android: Enhance pbuffer implementation

[Tomasz Figa]

On Mon, Dec 5, 2016 at 10:20 PM, Liu, Zhiquan <zhiquan.liu at intel.com> wrote:
> Hi Tomasz,
>
> Thanks for the review.  I will change these logs and code styles errors in v2 version.
>
>> > +   } else if (dri2_surf->base.Type == EGL_PBUFFER_BIT) {
>>
>> We won't be called with anything else than window or pbuffer bit here,
>> because we don't advertise pixmap support and createPixmapSurface is
>> stubbed out. For better coding style (less indentation) it's enough to
>> just return 0 in the if above and then move the code below out of the conditional block completely.
> The original code (if...else if...)is very clearly to describe what will happen for each type(window, pbuffer, pixmap).
> It's good for readability.
>
>> > +         return -1;
>> > +      }
>> > +   } else {
>> > +      _eglLog(_EGL_WARNING, "pixmap is not supported now !");
>> >     }
>>
>> This else block is not needed, as I explained above.
> I suggest to remove this pixmap log, and add follow description in else.
> /*pixmap is not supported now, add the implementation of pixmap here in the future.*/

This is just speculating about future. Odds are that pixmap support
won't show up in this backend at all, so for now it would be just
adding unnecessary lines of code. Let's stick to the "keep it simple
s." principle.

Best regards,
Tomasz