Fwd: [PATCH weston 01/68] libweston: Add pixel-format helpers

[Armin Krezović]

On 09.12.2016 22:30, Daniel Stone wrote:
> Hi,
> 

Hi,

> On 9 December 2016 at 20:37, Armin Krezović <krezovic.armin at gmail.com> wrote:
>> On 09.12.2016 20:57, Daniel Stone wrote:
>>>  libweston/pixel-formats.c | 398 ++++++++++++++++++++++++++++++++++++++++++++++
>>>  libweston/pixel-formats.h | 112 +++++++++++++
>>
>> Where are corresponding build system modifications?
> 
> Missing, apparently ... :\
> 
>>> +#include <EGL/egl.h>
>>> +#include <EGL/eglext.h>
>>> +#include <GLES2/gl2.h>
>>> +#include <GLES2/gl2ext.h>
>>
>> Is it supposed to work without EGL/GLESv2 enabled?
> 
> Not really, no; note that the DRM backend has a hard dependency on GBM
> and the Wayland backend has a hard dependency on libwayland-egl, so
> this only really affects people building the X11/headless backends
> only with no EGL. I suppose we could do a giant #ifdef tree, or just
> pull all the tokens we use into weston-egl-ext.h and use that. Do you
> have any preferences?
> 

Well, if my guess is correct, this is supposed to be part of libweston, no?

I don't think some people would like to have hard EGL dep on libweston itself,
and conditionally building this into the library might produce incompatible
lib with and without --disable-egl using the same source.

That said, I'm all for importing the needed bits into weston-egl-ext.h (if
you prefer to #ifdef a lot, go ahead).

>>> +/**
>>> + * Table of DRM formats supported by Weston; RGB, ARGB and YUV formats are
>>> + * supported. Indexed/greyscale formats, and formats not containing complete
>>> + * colour channels, are not supported.
>>> + */
>>
>> I expected something using this immediately. I suggest you squash it with something
>> else that uses this.
> 
> It could be squashed with 'Store format in drm_fb', but as the
> DRM-specific parts of the series were getting very little review, and
> it can also be useful for gl-renderer's SHM uploads in particular, I
> didn't want to muddle it in with the rest of the series. Depending on
> how that goes (whether I get to porting gl-renderer, if earlier parts
> of the series get reviewed so we can merge this, etc), it can be
> squashed into its first user.
> 

Well, you could've squashed it into that patch and sent it (from what I've
seen it doesn't involve any new code and doesn't depend on anything else),
and I can help you with review and gl-renderer porting, as this can be
landed independently from rest of the series.

Even if they don't fit together, at least make the patch that uses this a
next one, so someone can take a look at example usage without having to
dig through whole series to find out what is using this.

> Cheers,
> Daniel
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 870 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20161209/11736abb/attachment.sig>