Require INVALID for implicit format modifiers

backend/drm/drm.c

@@ -118,9 +118,14 @@ static bool add_plane(struct wlr_drm_backend *drm,
 	p->id = drm_plane->plane_id;
 	p->props = *props;
 
-	for (size_t j = 0; j < drm_plane->count_formats; ++j) {
-		wlr_drm_format_set_add(&p->formats, drm_plane->formats[j],
-			DRM_FORMAT_MOD_INVALID);
+	for (size_t i = 0; i < drm_plane->count_formats; ++i) {
+		// Force a LINEAR layout for the cursor if the driver doesn't support
+		// modifiers
+		uint64_t mod = DRM_FORMAT_MOD_INVALID;
+		if (type == DRM_PLANE_TYPE_CURSOR) {
+			mod = DRM_FORMAT_MOD_LINEAR;
+		}
+		wlr_drm_format_set_add(&p->formats, drm_plane->formats[i], mod);
 	}
 
 	if (p->props.in_formats && drm->addfb2_modifiers) {
@@ -150,13 +155,6 @@ static bool add_plane(struct wlr_drm_backend *drm,
 		}
 
 		drmModeFreePropertyBlob(blob);
-	} else if (type == DRM_PLANE_TYPE_CURSOR) {
-		// Force a LINEAR layout for the cursor if the driver doesn't support
-		// modifiers
-		for (size_t i = 0; i < p->formats.len; ++i) {
-			wlr_drm_format_set_add(&p->formats, p->formats.formats[i]->format,
-				DRM_FORMAT_MOD_LINEAR);
-		}
 	}
 
 	switch (type) {

backend/drm/renderer.c

@@ -256,6 +256,13 @@ static uint32_t get_fb_for_bo(struct wlr_drm_backend *drm,
 			wlr_log_errno(WLR_DEBUG, "drmModeAddFB2WithModifiers failed");
 		}
 	} else {
+		if (dmabuf->modifier != DRM_FORMAT_MOD_INVALID &&
+				dmabuf->modifier != DRM_FORMAT_MOD_LINEAR) {
+			wlr_log(WLR_ERROR, "Cannot import DRM framebuffer with explicit "
+				"modifier 0x%"PRIX64, dmabuf->modifier);
+			return 0;
+		}
+
 		int ret = drmModeAddFB2(drm->fd, dmabuf->width, dmabuf->height,
 			dmabuf->format, handles, dmabuf->stride, dmabuf->offset, &id, 0);
 		if (ret != 0 && dmabuf->format == DRM_FORMAT_ARGB8888 &&

backend/x11/backend.c

@@ -354,6 +354,7 @@ static bool query_formats(struct wlr_x11_backend *x11) {
 			}
 
 			if (x11->have_dri3) {
+				// X11 always supports implicit modifiers
 				wlr_drm_format_set_add(&x11->dri3_formats, format->drm,
 					DRM_FORMAT_MOD_INVALID);
 				if (!query_dri3_modifiers(x11, format)) {

include/render/drm_format_set.h

@@ -4,6 +4,7 @@
 #include <wlr/render/drm_format_set.h>
 
 struct wlr_drm_format *wlr_drm_format_create(uint32_t format);
+bool wlr_drm_format_has(const struct wlr_drm_format *fmt, uint64_t modifier);
 bool wlr_drm_format_add(struct wlr_drm_format **fmt_ptr, uint64_t modifier);
 struct wlr_drm_format *wlr_drm_format_dup(const struct wlr_drm_format *format);
 /**

render/allocator/gbm.c

@@ -7,6 +7,7 @@
 #include <wlr/util/log.h>
 #include <xf86drm.h>
 #include "render/allocator/gbm.h"
+#include "render/drm_format_set.h"
 
 static const struct wlr_buffer_impl buffer_impl;
 
@@ -86,17 +87,20 @@ static struct wlr_gbm_buffer *create_buffer(struct wlr_gbm_allocator *alloc,
 		int width, int height, const struct wlr_drm_format *format) {
 	struct gbm_device *gbm_device = alloc->gbm_device;
 
-	struct gbm_bo *bo = NULL;
+	assert(format->len > 0);
+
 	bool has_modifier = true;
-	if (format->len > 0) {
-		bo = gbm_bo_create_with_modifiers(gbm_device, width, height,
-			format->format, format->modifiers, format->len);
-	}
+	struct gbm_bo *bo = gbm_bo_create_with_modifiers(gbm_device, width, height,
+		format->format, format->modifiers, format->len);
 	if (bo == NULL) {
 		uint32_t usage = GBM_BO_USE_SCANOUT | GBM_BO_USE_RENDERING;
 		if (format->len == 1 &&
 				format->modifiers[0] == DRM_FORMAT_MOD_LINEAR) {
 			usage |= GBM_BO_USE_LINEAR;
+		} else if (!wlr_drm_format_has(format, DRM_FORMAT_MOD_INVALID)) {
+			// If the format doesn't accept an implicit modifier, bail out.
+			wlr_log(WLR_ERROR, "gbm_bo_create_with_modifiers failed");
+			return NULL;
 		}
 		bo = gbm_bo_create(gbm_device, width, height, format->format, usage);
 		has_modifier = false;

render/drm_format_set.c

@@ -43,18 +43,7 @@ bool wlr_drm_format_set_has(const struct wlr_drm_format_set *set,
 	if (!fmt) {
 		return false;
 	}
-
-	if (modifier == DRM_FORMAT_MOD_INVALID) {
-		return true;
-	}
-
-	for (size_t i = 0; i < fmt->len; ++i) {
-		if (fmt->modifiers[i] == modifier) {
-			return true;
-		}
-	}
-
-	return false;
+	return wlr_drm_format_has(fmt, modifier);
 }
 
 bool wlr_drm_format_set_add(struct wlr_drm_format_set *set, uint32_t format,
@@ -106,18 +95,21 @@ struct wlr_drm_format *wlr_drm_format_create(uint32_t format) {
 	return fmt;
 }
 
-bool wlr_drm_format_add(struct wlr_drm_format **fmt_ptr, uint64_t modifier) {
-	struct wlr_drm_format *fmt = *fmt_ptr;
-
-	if (modifier == DRM_FORMAT_MOD_INVALID) {
-		return true;
-	}
-
+bool wlr_drm_format_has(const struct wlr_drm_format *fmt, uint64_t modifier) {
 	for (size_t i = 0; i < fmt->len; ++i) {
 		if (fmt->modifiers[i] == modifier) {
 			return true;
 		}
 	}
+	return false;
+}
+
+bool wlr_drm_format_add(struct wlr_drm_format **fmt_ptr, uint64_t modifier) {
+	struct wlr_drm_format *fmt = *fmt_ptr;
+
+	if (wlr_drm_format_has(fmt, modifier)) {
+		return true;
+	}
 
 	if (fmt->len == fmt->cap) {
 		size_t cap = fmt->cap ? fmt->cap * 2 : 4;
@@ -152,16 +144,6 @@ struct wlr_drm_format *wlr_drm_format_intersect(
 		const struct wlr_drm_format *a, const struct wlr_drm_format *b) {
 	assert(a->format == b->format);
 
-	// Special case: if a format only supports LINEAR and the other doesn't
-	// support any modifier, force LINEAR. This will force the allocator to
-	// create a buffer with a LINEAR layout instead of an implicit modifier.
-	if (a->len == 0 && b->len == 1 && b->modifiers[0] == DRM_FORMAT_MOD_LINEAR) {
-		return wlr_drm_format_dup(b);
-	}
-	if (b->len == 0 && a->len == 1 && a->modifiers[0] == DRM_FORMAT_MOD_LINEAR) {
-		return wlr_drm_format_dup(a);
-	}
-
 	size_t format_cap = a->len < b->len ? a->len : b->len;
 	size_t format_size = sizeof(struct wlr_drm_format) +
 		format_cap * sizeof(a->modifiers[0]);
@@ -184,9 +166,9 @@ struct wlr_drm_format *wlr_drm_format_intersect(
 		}
 	}
 
-	// If both formats support modifiers, but the intersection is empty, then
-	// the formats aren't compatible with each other
-	if (format->len == 0 && a->len > 0 && b->len > 0) {
+	// If the intersection is empty, then the formats aren't compatible with
+	// each other.
+	if (format->len == 0) {
 		free(format);
 		return NULL;
 	}

render/egl.c

@@ -119,11 +119,19 @@ static void init_dmabuf_formats(struct wlr_egl *egl) {
 
 		has_modifiers = has_modifiers || modifiers_len > 0;
 
+		// EGL always supports implicit modifiers
+		wlr_drm_format_set_add(&egl->dmabuf_texture_formats, fmt,
+			DRM_FORMAT_MOD_INVALID);
+		wlr_drm_format_set_add(&egl->dmabuf_render_formats, fmt,
+			DRM_FORMAT_MOD_INVALID);
+
 		if (modifiers_len == 0) {
+			// Asume the linear layout is supported if the driver doesn't
+			// explicitly say otherwise
 			wlr_drm_format_set_add(&egl->dmabuf_texture_formats, fmt,
-				DRM_FORMAT_MOD_INVALID);
+				DRM_FORMAT_MOD_LINEAR);
 			wlr_drm_format_set_add(&egl->dmabuf_render_formats, fmt,
-				DRM_FORMAT_MOD_INVALID);
+				DRM_FORMAT_MOD_LINEAR);
 		}
 
 		for (int j = 0; j < modifiers_len; j++) {

types/wlr_linux_dmabuf_v1.c

@@ -9,6 +9,7 @@
 #include <wlr/types/wlr_linux_dmabuf_v1.h>
 #include <wlr/util/log.h>
 #include "linux-dmabuf-unstable-v1-protocol.h"
+#include "render/drm_format_set.h"
 #include "util/signal.h"
 
 #define LINUX_DMABUF_VERSION 3
@@ -413,7 +414,9 @@ static const struct zwp_linux_dmabuf_v1_interface linux_dmabuf_impl = {
 static void linux_dmabuf_send_modifiers(struct wl_resource *resource,
 		const struct wlr_drm_format *fmt) {
 	if (wl_resource_get_version(resource) < ZWP_LINUX_DMABUF_V1_MODIFIER_SINCE_VERSION) {
-		zwp_linux_dmabuf_v1_send_format(resource, fmt->format);
+		if (wlr_drm_format_has(fmt, DRM_FORMAT_MOD_INVALID)) {
+			zwp_linux_dmabuf_v1_send_format(resource, fmt->format);
+		}
 		return;
 	}
 
@@ -422,10 +425,6 @@ static void linux_dmabuf_send_modifiers(struct wl_resource *resource,
 		zwp_linux_dmabuf_v1_send_modifier(resource, fmt->format,
 			mod >> 32, mod & 0xFFFFFFFF);
 	}
-
-	// We always support buffers with an implicit modifier
-	zwp_linux_dmabuf_v1_send_modifier(resource, fmt->format,
-		DRM_FORMAT_MOD_INVALID >> 32, DRM_FORMAT_MOD_INVALID & 0xFFFFFFFF);
 }
 
 static void linux_dmabuf_send_formats(struct wlr_linux_dmabuf_v1 *linux_dmabuf,