On Tue, 4 Apr 2017 17:54:20 +0100 Daniel Stone <[email protected]> wrote:
> Rather than duplicating knowledge of pixel formats across several > components, create a custom central repository. > > Signed-off-by: Daniel Stone <[email protected]> > --- > Makefile.am | 2 + > libweston/pixel-formats.c | 430 > ++++++++++++++++++++++++++++++++++++++++++++++ > libweston/pixel-formats.h | 194 +++++++++++++++++++++ > 3 files changed, 626 insertions(+) > create mode 100644 libweston/pixel-formats.c > create mode 100644 libweston/pixel-formats.h > > v10: Add more docs. Build without EGL. Fix format transposition errors. > Insert omitted [hv]sub. > > diff --git a/Makefile.am b/Makefile.am > index 519d9115..7b24d40c 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -105,6 +105,8 @@ libweston_@LIBWESTON_MAJOR@_la_SOURCES = > \ > libweston/timeline-object.h \ > libweston/linux-dmabuf.c \ > libweston/linux-dmabuf.h \ > + libweston/pixel-formats.c \ > + libweston/pixel-formats.h \ > shared/helpers.h \ > shared/matrix.c \ > shared/matrix.h \ Hi, all the actual content is good now, there are a couple of typoes in the comments. The remaining issue is the dependency on libdrm headers, more below. > diff --git a/libweston/pixel-formats.c b/libweston/pixel-formats.c > new file mode 100644 > index 00000000..2c58443c > --- /dev/null > +++ b/libweston/pixel-formats.c > @@ -0,0 +1,430 @@ > +/* > + * Copyright © 2016 Collabora, Ltd. > + * > + * Permission is hereby granted, free of charge, to any person obtaining a > + * copy of this software and associated documentation files (the "Software"), > + * to deal in the Software without restriction, including without limitation > + * the rights to use, copy, modify, merge, publish, distribute, sublicense, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the next > + * paragraph) shall be included in all copies or substantial portions of the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > + * DEALINGS IN THE SOFTWARE. > + * > + * Author: Daniel Stone <[email protected]> > + */ > + > +#include "config.h" > + > +#include <endian.h> > +#include <inttypes.h> > +#include <stdbool.h> > +#include <unistd.h> > +#include <libdrm/drm_fourcc.h> Shouldn't this be just <drm_fourcc.h>? We have pkg-config providing us with -I/usr/include/libdrm. The problem with this is that this makes libweston unconditionally depend on libdrm headers at build time (not the library, and nothing new at runtime). I think that is ok by now. Do people agree? An alternative would be to #ifdef the whole array as empty if libdrm headers are not available, or move this into drm-backend.so if possible. Anyway we need libdrm cflags for compiling this file when libdrm headers are needed. Btw. gl-renderer.c is similar, it needs drm_fourcc.h but not libdrm.so itself. I presume the plan is to start using these in gl-renderer and linux-dmabuf.c in the future, to reduce format definition duplication and add more sanity checks for dmabufs. Therefore I propose that we make libdrm a hard build dependency (not runtime dependency) for libweston.so, i.e. libweston core. I can write the patch to add that requirement, and then the changes needed to this pixel-format patch will be minor. How's that? > + > +#include "helpers.h" > +#include "wayland-util.h" > +#include "pixel-formats.h" > + > +#if ENABLE_EGL > +#include <EGL/egl.h> > +#include <EGL/eglext.h> > +#include <GLES2/gl2.h> > +#include <GLES2/gl2ext.h> > +#define GL_FORMAT(fmt) .gl_format = (fmt) > +#define GL_TYPE(type) .gl_type = (type) > +#define SAMPLER_TYPE(type) .sampler_type = (type) > +#else > +#define GL_FORMAT(fmt) .gl_format = 0 > +#define GL_TYPE(type) .gl_type = 0 > +#define SAMPLER_TYPE(type) .sampler_type = 0 > +#endif > + > +#include "weston-egl-ext.h" > + > + > +/** > + * Return the effective sampling height for a given plane > + * > + * When vertical subsampling is in effect, a sampler bound to a secondary > + * plane must bind the sampler with a smaller effective height. This function > + * returns the effective width to use for the sampler, i.e. dividing by vsub. s/width/height/ > + * > + * If vertical subsampling is not in effect, this will be equal to the > height. > + * > + * @param format Pixel format info structure > + * @param plane Zero-indexed plane number > + * @param width Height of the buffer s/width/height/ > + * @returns Effective width for sampling > + */ > +unsigned int > +pixel_format_height_for_plane(const struct pixel_format_info *format, > + unsigned int plane, > + unsigned int height); Thanks, pq
pgpbWaiL0D8BX.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
