[weston PATCH] Allow simple-dmabuf-drm to pass y_inverted flag

[Pekka Paalanen]

On Thu, 8 Mar 2018 18:06:14 +0100
Guido Günther <agx at sigxcpu.org> wrote:

> This allows to check if ZWP_LINUX_BUFFER_PARAMS_V1_FLAGS_Y_INVERT is
> interpreted correctly by the compositor.
> ---
>  clients/simple-dmabuf-drm.c | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c
> index 14d716de..1073930f 100644
> --- a/clients/simple-dmabuf-drm.c
> +++ b/clients/simple-dmabuf-drm.c
> @@ -379,11 +379,11 @@ static const struct zwp_linux_buffer_params_v1_listener params_listener = {
>  
>  static int
>  create_dmabuf_buffer(struct display *display, struct buffer *buffer,
> -		     int width, int height, int format)
> +		     int width, int height, int format, int is_y_inverted)

Hi,

rather than adding boolean arguments (pretending to be int) for each
flag one might want to control, I would prefer just uint32_t flags with
a doc about the possible values.

>  {
>  	struct zwp_linux_buffer_params_v1 *params;
>  	uint64_t modifier = 0;
> -	uint32_t flags;
> +	uint32_t flags = 0;
>  	struct drm_device *drm_dev;
>  
>  	if (!drm_connect(buffer)) {
> @@ -434,7 +434,8 @@ create_dmabuf_buffer(struct display *display, struct buffer *buffer,
>  	 * correct height to the compositor.
>  	 */
>  	buffer->height = height;
> -	flags = 0;
> +	if (is_y_inverted)
> +		flags |= ZWP_LINUX_BUFFER_PARAMS_V1_FLAGS_Y_INVERT;
>  
>  	params = zwp_linux_dmabuf_v1_create_params(display->dmabuf);
>  	zwp_linux_buffer_params_v1_add(params,
> @@ -517,7 +518,8 @@ static const struct zxdg_toplevel_v6_listener xdg_toplevel_listener = {
>  };
>  
>  static struct window *
> -create_window(struct display *display, int width, int height, int format)
> +create_window(struct display *display, int width, int height, int format,
> +	      int is_y_inverted)

Here as well.

>  {
>  	struct window *window;
>  	int i;
> @@ -566,7 +568,7 @@ create_window(struct display *display, int width, int height, int format)
>  
>  	for (i = 0; i < NUM_BUFFERS; ++i) {
>  		ret = create_dmabuf_buffer(display, &window->buffers[i],
> -		                               width, height, format);
> +		                               width, height, format, is_y_inverted);
>  
>  		if (ret < 0)
>  			return NULL;
> @@ -814,13 +816,15 @@ print_usage_and_exit(void)
>  	printf("usage flags:\n"
>  		"\t'--import-immediate=<>'\n\t\t0 to import dmabuf via roundtrip,"
>  		"\n\t\t1 to enable import without roundtrip\n"
> +		"\t'--y-inverted=<>'\n\t\t0 to not pass Y_INVERTED flag,"
> +		"\n\t\t1 to pass Y_INVERTED flag\n"
>  		"\t'--import-format=<>'\n\t\tXRGB to import dmabuf as XRGB8888,"
>  		"\n\t\tNV12 to import as multi plane NV12 with tiling modifier\n");
>  	exit(0);
>  }
>  
>  static int
> -is_import_mode_immediate(const char* c)
> +is_true(const char* c)
>  {
>  	if (!strcmp(c, "1"))
>  		return 1;
> @@ -852,22 +856,29 @@ main(int argc, char **argv)
>  	struct display *display;
>  	struct window *window;
>  	int is_immediate = 0;
> +	int is_y_inverted= 0;
>  	int import_format = DRM_FORMAT_XRGB8888;
>  	int ret = 0, i = 0;
>  
>  	if (argc > 1) {
>  		static const char import_mode[] = "--import-immediate=";
>  		static const char format[] = "--import-format=";
> +		static const char y_inverted[] = "--y-inverted=";
>  		for (i = 1; i < argc; i++) {
>  			if (!strncmp(argv[i], import_mode,
>  				     sizeof(import_mode) - 1)) {
> -				is_immediate = is_import_mode_immediate(argv[i]
> +				is_immediate = is_true(argv[i]
>  							+ sizeof(import_mode) - 1);
>  			}
>  			else if (!strncmp(argv[i], format, sizeof(format) - 1)) {
>  				import_format = parse_import_format(argv[i]
>  							+ sizeof(format) - 1);
>  			}
> +			else if (!strncmp(argv[i], y_inverted,
> +				     sizeof(y_inverted) - 1)) {
> +				is_y_inverted = is_true(argv[i]
> +							+ sizeof(y_inverted) - 1);
> +			}
>  			else {
>  				print_usage_and_exit();
>  			}

The command line parsing bit is probably ok, but if we are going to add
more options, I'd really prefer it migrating to getopt_long().

> @@ -875,7 +886,7 @@ main(int argc, char **argv)
>  	}
>  
>  	display = create_display(is_immediate, import_format);
> -	window = create_window(display, 256, 256, import_format);
> +	window = create_window(display, 256, 256, import_format, is_y_inverted);
>  	if (!window)
>  		return 1;
>  

The idea is good.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180315/2f5c4d14/attachment.sig>