Only allow one modifier per DMA-BUF, split attributes struct in render/

backend/drm/renderer.c

@@ -186,15 +186,15 @@ static struct wlr_texture *get_tex_for_bo(struct wlr_drm_renderer *renderer,
 		return NULL;
 	}
 
-	struct wlr_dmabuf_buffer_attribs attribs = {
+	struct wlr_dmabuf_attributes attribs = {
 		.n_planes = 1,
 		.width = gbm_bo_get_width(bo),
 		.height = gbm_bo_get_height(bo),
 		.format = gbm_bo_get_format(bo),
+		.modifier = DRM_FORMAT_MOD_LINEAR,
 	};
 	attribs.offset[0] = 0;
 	attribs.stride[0] = gbm_bo_get_stride_for_plane(bo, 0);
-	attribs.modifier[0] = DRM_FORMAT_MOD_LINEAR;
 	attribs.fd[0] = gbm_bo_get_fd(bo);
 
 	tex->tex = wlr_texture_from_dmabuf(renderer->wlr_rend, &attribs);

include/wlr/render/dmabuf.h

@@ -0,0 +1,29 @@
+#ifndef WLR_RENDER_DMABUF_H
+#define WLR_RENDER_DMABUF_H
+
+// So we don't have to pull in linux specific drm headers
+#ifndef DRM_FORMAT_MOD_INVALID
+#define DRM_FORMAT_MOD_INVALID ((1ULL<<56) - 1)
+#endif
+
+#define WLR_DMABUF_MAX_PLANES 4
+
+enum wlr_dmabuf_attributes_flags {
+	WLR_DMABUF_ATTRIBUTES_FLAGS_Y_INVERT = 1,
+	WLR_DMABUF_ATTRIBUTES_FLAGS_INTERLACED = 2,
+	WLR_DMABUF_ATTRIBUTES_FLAGS_BOTTOM_FIRST = 4,
+};
+
+struct wlr_dmabuf_attributes {
+	int32_t width, height;
+	uint32_t format;
+	uint32_t flags; // enum wlr_dmabuf_attributes_flags
+	uint64_t modifier;
+
+	int n_planes;
+	uint32_t offset[WLR_DMABUF_MAX_PLANES];
+	uint32_t stride[WLR_DMABUF_MAX_PLANES];
+	int fd[WLR_DMABUF_MAX_PLANES];
+};
+
+#endif

include/wlr/render/egl.h

@@ -6,7 +6,7 @@
 #include <pixman.h>
 #include <stdbool.h>
 #include <wayland-server.h>
-#include <wlr/types/wlr_linux_dmabuf.h>
+#include <wlr/render/dmabuf.h>
 
 struct wlr_egl {
 	EGLDisplay display;
@@ -65,14 +65,14 @@ EGLImageKHR wlr_egl_create_image_from_wl_drm(struct wlr_egl *egl,
  * of the dmabuf with wlr_egl_check_import_dmabuf once first.
  */
 EGLImageKHR wlr_egl_create_image_from_dmabuf(struct wlr_egl *egl,
-	struct wlr_dmabuf_buffer_attribs *attributes);
+	struct wlr_dmabuf_attributes *attributes);
 
 /**
  * Try to import the given dmabuf. On success return true false otherwise.
  * If this succeeds the dmabuf can be used for rendering on a texture
  */
 bool wlr_egl_check_import_dmabuf(struct wlr_egl *egl,
-	struct wlr_dmabuf_buffer *dmabuf);
+	struct wlr_dmabuf_attributes *attributes);
 
 /**
  * Get the available dmabuf formats

include/wlr/render/gles2.h

@@ -14,6 +14,6 @@ struct wlr_texture *wlr_gles2_texture_from_pixels(struct wlr_egl *egl,
 struct wlr_texture *wlr_gles2_texture_from_wl_drm(struct wlr_egl *egl,
 	struct wl_resource *data);
 struct wlr_texture *wlr_gles2_texture_from_dmabuf(struct wlr_egl *egl,
-	struct wlr_dmabuf_buffer_attribs *attribs);
+	struct wlr_dmabuf_attributes *attribs);
 
 #endif

include/wlr/render/interface.h

@@ -8,8 +8,8 @@
 #include <wlr/render/wlr_renderer.h>
 #include <wlr/render/wlr_texture.h>
 #include <wlr/types/wlr_box.h>
-#include <wlr/types/wlr_linux_dmabuf.h>
 #include <wlr/types/wlr_output.h>
+#include <wlr/render/dmabuf.h>
 
 struct wlr_renderer_impl {
 	void (*begin)(struct wlr_renderer *renderer, uint32_t width,
@@ -31,7 +31,7 @@ struct wlr_renderer_impl {
 	void (*wl_drm_buffer_get_size)(struct wlr_renderer *renderer,
 		struct wl_resource *buffer, int *width, int *height);
 	bool (*check_import_dmabuf)(struct wlr_renderer *renderer,
-		struct wlr_dmabuf_buffer *dmabuf);
+		struct wlr_dmabuf_attributes *attribs);
 	int (*get_dmabuf_formats)(struct wlr_renderer *renderer, int **formats);
 	int (*get_dmabuf_modifiers)(struct wlr_renderer *renderer, int format,
 		uint64_t **modifiers);
@@ -47,7 +47,7 @@ struct wlr_renderer_impl {
 	struct wlr_texture *(*texture_from_wl_drm)(struct wlr_renderer *renderer,
 		struct wl_resource *data);
 	struct wlr_texture *(*texture_from_dmabuf)(struct wlr_renderer *renderer,
-		struct wlr_dmabuf_buffer_attribs *attribs);
+		struct wlr_dmabuf_attributes *attribs);
 	void (*destroy)(struct wlr_renderer *renderer);
 	void (*init_wl_display)(struct wlr_renderer *renderer,
 		struct wl_display *wl_display);

include/wlr/render/wlr_renderer.h

@@ -89,7 +89,7 @@ int wlr_renderer_get_dmabuf_modifiers(struct wlr_renderer *renderer, int format,
  * If this succeeds the dmabuf can be used for rendering on a texture
  */
 bool wlr_renderer_check_import_dmabuf(struct wlr_renderer *renderer,
-	struct wlr_dmabuf_buffer *dmabuf);
+	struct wlr_dmabuf_attributes *attributes);
 /**
  * Reads out of pixels of the currently bound surface into data. `stride` is in
  * bytes.

include/wlr/render/wlr_texture.h

@@ -5,7 +5,7 @@
 #include <EGL/eglext.h>
 #include <stdint.h>
 #include <wayland-server-protocol.h>
-#include <wlr/types/wlr_linux_dmabuf.h>
+#include <wlr/render/dmabuf.h>
 
 struct wlr_renderer;
 struct wlr_texture_impl;
@@ -33,7 +33,7 @@ struct wlr_texture *wlr_texture_from_wl_drm(struct wlr_renderer *renderer,
  * Create a new texture from a DMA-BUF. The returned texture is immutable.
  */
 struct wlr_texture *wlr_texture_from_dmabuf(struct wlr_renderer *renderer,
-	struct wlr_dmabuf_buffer_attribs *attribs);
+	struct wlr_dmabuf_attributes *attribs);
 
 /**
  * Get the texture width and height.

include/wlr/types/wlr_linux_dmabuf.h

@@ -1,40 +1,16 @@
 #ifndef WLR_TYPES_WLR_LINUX_DMABUF_H
 #define WLR_TYPES_WLR_LINUX_DMABUF_H
 
-#define WLR_LINUX_DMABUF_MAX_PLANES 4
-
 #include <stdint.h>
 #include <wayland-server-protocol.h>
-
+#include <wlr/render/dmabuf.h>
-/* So we don't have to pull in linux specific drm headers */
-#ifndef DRM_FORMAT_MOD_INVALID
-#define DRM_FORMAT_MOD_INVALID ((1ULL<<56) - 1)
-#endif
-
-enum {
-	WLR_DMABUF_BUFFER_ATTRIBS_FLAGS_Y_INVERT = 1,
-	WLR_DMABUF_BUFFER_ATTRIBS_FLAGS_INTERLACED = 2,
-	WLR_DMABUF_BUFFER_ATTRIBS_FLAGS_BOTTOM_FIRST = 4,
-};
-
-struct wlr_dmabuf_buffer_attribs {
-	/* set via params_add */
-	int n_planes;
-	uint32_t offset[WLR_LINUX_DMABUF_MAX_PLANES];
-	uint32_t stride[WLR_LINUX_DMABUF_MAX_PLANES];
-	uint64_t modifier[WLR_LINUX_DMABUF_MAX_PLANES];
-	int fd[WLR_LINUX_DMABUF_MAX_PLANES];
-	/* set via params_create */
-	int32_t width, height;
-	uint32_t format;
-	uint32_t flags;
-};
 
 struct wlr_dmabuf_buffer {
 	struct wlr_renderer *renderer;
 	struct wl_resource *buffer_resource;
 	struct wl_resource *params_resource;
-	struct wlr_dmabuf_buffer_attribs attributes;
+	struct wlr_dmabuf_attributes attributes;
+	bool has_modifier;
 };
 
 /**

render/egl.c

@@ -340,9 +340,9 @@ EGLImageKHR wlr_egl_create_image_from_wl_drm(struct wlr_egl *egl,
 }
 
 EGLImageKHR wlr_egl_create_image_from_dmabuf(struct wlr_egl *egl,
-		struct wlr_dmabuf_buffer_attribs *attributes) {
+		struct wlr_dmabuf_attributes *attributes) {
 	bool has_modifier = false;
-	if (attributes->modifier[0] != DRM_FORMAT_MOD_INVALID) {
+	if (attributes->modifier != DRM_FORMAT_MOD_INVALID) {
 		if (!egl->egl_exts.dmabuf_import_modifiers) {
 			return NULL;
 		}
@@ -364,7 +364,7 @@ EGLImageKHR wlr_egl_create_image_from_dmabuf(struct wlr_egl *egl,
 		EGLint pitch;
 		EGLint mod_lo;
 		EGLint mod_hi;
-	} attr_names[WLR_LINUX_DMABUF_MAX_PLANES] = {
+	} attr_names[WLR_DMABUF_MAX_PLANES] = {
 		{
 			EGL_DMA_BUF_PLANE0_FD_EXT,
 			EGL_DMA_BUF_PLANE0_OFFSET_EXT,
@@ -401,9 +401,9 @@ EGLImageKHR wlr_egl_create_image_from_dmabuf(struct wlr_egl *egl,
 		attribs[atti++] = attributes->stride[i];
 		if (has_modifier) {
 			attribs[atti++] = attr_names[i].mod_lo;
-			attribs[atti++] = attributes->modifier[i] & 0xFFFFFFFF;
+			attribs[atti++] = attributes->modifier & 0xFFFFFFFF;
 			attribs[atti++] = attr_names[i].mod_hi;
-			attribs[atti++] = attributes->modifier[i] >> 32;
+			attribs[atti++] = attributes->modifier >> 32;
 		}
 	}
 	attribs[atti++] = EGL_NONE;
@@ -417,8 +417,8 @@ EGLImageKHR wlr_egl_create_image_from_dmabuf(struct wlr_egl *egl,
 #define DRM_FORMAT_BIG_ENDIAN 0x80000000
 #endif
 bool wlr_egl_check_import_dmabuf(struct wlr_egl *egl,
-		struct wlr_dmabuf_buffer *dmabuf) {
+		struct wlr_dmabuf_attributes *attribs) {
-	switch (dmabuf->attributes.format & ~DRM_FORMAT_BIG_ENDIAN) {
+	switch (attribs->format & ~DRM_FORMAT_BIG_ENDIAN) {
 		/* TODO: YUV based formats not yet supported, require multiple
 		 * wlr_create_image_from_dmabuf */
 	case WL_SHM_FORMAT_YUYV:
@@ -431,8 +431,7 @@ bool wlr_egl_check_import_dmabuf(struct wlr_egl *egl,
 		break;
 	}
 
-	EGLImage egl_image = wlr_egl_create_image_from_dmabuf(egl,
+	EGLImage egl_image = wlr_egl_create_image_from_dmabuf(egl, attribs);
-		&dmabuf->attributes);
 	if (egl_image) {
 		/* We can import the image, good. No need to keep it
 		   since wlr_texture_upload_dmabuf will import it again */

render/gles2/renderer.c

@@ -243,9 +243,9 @@ static int gles2_get_dmabuf_modifiers(struct wlr_renderer *wlr_renderer,
 }
 
 static bool gles2_check_import_dmabuf(struct wlr_renderer *wlr_renderer,
-		struct wlr_dmabuf_buffer *dmabuf) {
+		struct wlr_dmabuf_attributes *attribs) {
 	struct wlr_gles2_renderer *renderer = gles2_get_renderer(wlr_renderer);
-	return wlr_egl_check_import_dmabuf(renderer->egl, dmabuf);
+	return wlr_egl_check_import_dmabuf(renderer->egl, attribs);
 }
 
 static bool gles2_read_pixels(struct wlr_renderer *wlr_renderer,
@@ -299,7 +299,7 @@ static struct wlr_texture *gles2_texture_from_wl_drm(
 
 static struct wlr_texture *gles2_texture_from_dmabuf(
 		struct wlr_renderer *wlr_renderer,
-		struct wlr_dmabuf_buffer_attribs *attribs) {
+		struct wlr_dmabuf_attributes *attribs) {
 	struct wlr_gles2_renderer *renderer = gles2_get_renderer(wlr_renderer);
 	return wlr_gles2_texture_from_dmabuf(renderer->egl, attribs);
 }

render/gles2/texture.c

@@ -199,7 +199,7 @@ struct wlr_texture *wlr_gles2_texture_from_wl_drm(struct wlr_egl *egl,
 }
 
 struct wlr_texture *wlr_gles2_texture_from_dmabuf(struct wlr_egl *egl,
-		struct wlr_dmabuf_buffer_attribs *attribs) {
+		struct wlr_dmabuf_attributes *attribs) {
 	assert(wlr_egl_is_current(egl));
 
 	if (!glEGLImageTargetTexture2DOES) {
@@ -225,7 +225,7 @@ struct wlr_texture *wlr_gles2_texture_from_dmabuf(struct wlr_egl *egl,
 	texture->type = WLR_GLES2_TEXTURE_DMABUF;
 	texture->has_alpha = true;
 	texture->inverted_y =
-		(attribs->flags & WLR_DMABUF_BUFFER_ATTRIBS_FLAGS_Y_INVERT) != 0;
+		(attribs->flags & WLR_DMABUF_ATTRIBUTES_FLAGS_Y_INVERT) != 0;
 
 	texture->image = wlr_egl_create_image_from_dmabuf(egl, attribs);
 	if (texture->image == NULL) {

render/wlr_renderer.c

@@ -136,11 +136,11 @@ int wlr_renderer_get_dmabuf_modifiers(struct wlr_renderer *r, int format,
 }
 
 bool wlr_renderer_check_import_dmabuf(struct wlr_renderer *r,
-		struct wlr_dmabuf_buffer *dmabuf) {
+		struct wlr_dmabuf_attributes *attribs) {
 	if (!r->impl->check_import_dmabuf) {
 		return false;
 	}
-	return r->impl->check_import_dmabuf(r, dmabuf);
+	return r->impl->check_import_dmabuf(r, attribs);
 }
 
 bool wlr_renderer_read_pixels(struct wlr_renderer *r, enum wl_shm_format fmt,

render/wlr_texture.c

@@ -35,7 +35,7 @@ struct wlr_texture *wlr_texture_from_wl_drm(struct wlr_renderer *renderer,
 }
 
 struct wlr_texture *wlr_texture_from_dmabuf(struct wlr_renderer *renderer,
-		struct wlr_dmabuf_buffer_attribs *attribs) {
+		struct wlr_dmabuf_attributes *attribs) {
 	if (!renderer->impl->texture_from_dmabuf) {
 		return NULL;
 	}

types/wlr_linux_dmabuf.c

@@ -66,7 +66,7 @@ static void params_destroy(struct wl_client *client,
 }
 
 static void params_add(struct wl_client *client,
-		struct wl_resource *params_resource, int32_t name_fd,
+		struct wl_resource *params_resource, int32_t fd,
 		uint32_t plane_idx, uint32_t offset, uint32_t stride,
 		uint32_t modifier_hi, uint32_t modifier_lo) {
 	struct wlr_dmabuf_buffer *buffer =
@@ -76,33 +76,42 @@ static void params_add(struct wl_client *client,
 		wl_resource_post_error(params_resource,
 			ZWP_LINUX_BUFFER_PARAMS_V1_ERROR_ALREADY_USED,
 			"params was already used to create a wl_buffer");
-		close(name_fd);
+		close(fd);
 		return;
 	}
 
-	if (plane_idx >= WLR_LINUX_DMABUF_MAX_PLANES) {
+	if (plane_idx >= WLR_DMABUF_MAX_PLANES) {
 		wl_resource_post_error(params_resource,
 			ZWP_LINUX_BUFFER_PARAMS_V1_ERROR_PLANE_IDX,
-			"plane index %u > %u", plane_idx, WLR_LINUX_DMABUF_MAX_PLANES);
+			"plane index %u > %u", plane_idx, WLR_DMABUF_MAX_PLANES);
-		close(name_fd);
+		close(fd);
 		return;
 	}
 
 	if (buffer->attributes.fd[plane_idx] != -1) {
 		wl_resource_post_error(params_resource,
 			ZWP_LINUX_BUFFER_PARAMS_V1_ERROR_PLANE_SET,
-			"a dmabuf with id %d has already been added for plane %u",
+			"a dmabuf with FD %d has already been added for plane %u",
-			buffer->attributes.fd[plane_idx],
+			buffer->attributes.fd[plane_idx], plane_idx);
-			plane_idx);
+		close(fd);
-		close(name_fd);
 		return;
 	}
 
-	buffer->attributes.fd[plane_idx] = name_fd;
+	uint64_t modifier = ((uint64_t)modifier_hi << 32) | modifier_lo;
+	if (buffer->has_modifier && modifier != buffer->attributes.modifier) {
+		wl_resource_post_error(params_resource,
+			ZWP_LINUX_BUFFER_PARAMS_V1_ERROR_INVALID_FORMAT,
+			"sent modifier %lu for plane %u, expected modifier %lu like other planes",
+			modifier, plane_idx, buffer->attributes.modifier);
+		close(fd);
+		return;
+	}
+	buffer->attributes.modifier = modifier;
+	buffer->has_modifier = true;
+
+	buffer->attributes.fd[plane_idx] = fd;
 	buffer->attributes.offset[plane_idx] = offset;
 	buffer->attributes.stride[plane_idx] = stride;
-	buffer->attributes.modifier[plane_idx] =
-		((uint64_t)modifier_hi << 32) | modifier_lo;
 	buffer->attributes.n_planes++;
 }
 
@@ -181,7 +190,8 @@ static void params_create_common(struct wl_client *client,
 		}
 
 		off_t size = lseek(buffer->attributes.fd[i], 0, SEEK_END);
-		if (size == -1) { /* Skip checks if kernel does no support seek on buffer */
+		if (size == -1) {
+			// Skip checks if kernel does no support seek on buffer
 			continue;
 		}
 		if (buffer->attributes.offset[i] >= size) {
@@ -200,8 +210,9 @@ static void params_create_common(struct wl_client *client,
 			goto err_out;
 		}
 
-		if (i == 0 && /* planes > 0 might be subsampled according to fourcc format */
+		// planes > 0 might be subsampled according to fourcc format
-			buffer->attributes.offset[i] + buffer->attributes.stride[i] * height > size) {
+		if (i == 0 && buffer->attributes.offset[i] +
+				buffer->attributes.stride[i] * height >= size) {
 			wl_resource_post_error(params_resource,
 				ZWP_LINUX_BUFFER_PARAMS_V1_ERROR_OUT_OF_BOUNDS,
 				"invalid buffer stride or height for plane %d", i);
@@ -218,7 +229,8 @@ static void params_create_common(struct wl_client *client,
 	}
 
 	/* Check if dmabuf is usable */
-	if (!wlr_renderer_check_import_dmabuf(buffer->renderer, buffer)) {
+	if (!wlr_renderer_check_import_dmabuf(buffer->renderer,
+			&buffer->attributes)) {
 		goto err_failed;
 	}
 
@@ -319,7 +331,7 @@ static void linux_dmabuf_create_params(struct wl_client *client,
 		goto err;
 	}
 
-	for (int i = 0; i < WLR_LINUX_DMABUF_MAX_PLANES; i++) {
+	for (int i = 0; i < WLR_DMABUF_MAX_PLANES; i++) {
 		buffer->attributes.fd[i] = -1;
 	}
 

types/wlr_surface.c

@@ -4,6 +4,7 @@
 #include <wlr/render/egl.h>
 #include <wlr/render/interface.h>
 #include <wlr/types/wlr_compositor.h>
+#include <wlr/types/wlr_linux_dmabuf.h>
 #include <wlr/types/wlr_matrix.h>
 #include <wlr/types/wlr_region.h>
 #include <wlr/types/wlr_surface.h>