Require INVALID for implicit format modifiers

See [1] for the motivation.

[1]: https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/75

3 files changed, 29 insertions, 31 deletions

diff --git a/render/allocator/gbm.c b/render/allocator/gbm.c
index b546e412..8f73862a 100644
--- a/render/allocator/gbm.c
+++ b/render/allocator/gbm.c
@@ -10,6 +10,7 @@
 #include <xf86drm.h>
 
 #include "render/allocator/gbm.h"
+#include "render/drm_format_set.h"
 
 static const struct wlr_buffer_impl buffer_impl;
 
@@ -89,17 +90,22 @@ 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);
-	}
+	uint64_t fallback_modifier = DRM_FORMAT_MOD_INVALID;
+	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;
+			fallback_modifier = DRM_FORMAT_MOD_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;
@@ -128,7 +134,7 @@ static struct wlr_gbm_buffer *create_buffer(struct wlr_gbm_allocator *alloc,
 	// don't populate the modifier field: other parts of the stack may not
 	// understand modifiers, and they can't strip the modifier.
 	if (!has_modifier) {
-		buffer->dmabuf.modifier = DRM_FORMAT_MOD_INVALID;
+		buffer->dmabuf.modifier = fallback_modifier;
 	}
 
 	wlr_log(WLR_DEBUG, "Allocated %dx%d GBM buffer (format 0x%"PRIX32", "
diff --git a/render/drm_format_set.c b/render/drm_format_set.c
index ef2929b7..90256098 100644
--- a/render/drm_format_set.c
+++ b/render/drm_format_set.c
@@ -43,11 +43,6 @@ 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;
-	}
-
 	return wlr_drm_format_has(fmt, modifier);
 }
 
@@ -112,10 +107,6 @@ 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 *fmt = *fmt_ptr;
 
-	if (modifier == DRM_FORMAT_MOD_INVALID) {
-		return true;
-	}
-
 	if (wlr_drm_format_has(fmt, modifier)) {
 		return true;
 	}
@@ -153,15 +144,17 @@ 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) {
+	// Special case: if a format only supports LINEAR and the other supports
+	// implicit modifiers, force LINEAR. This will force the allocator to
+	// create a buffer with a linear layout instead of an implicit modifier.
+	if (a->len == 1 && a->modifiers[0] == DRM_FORMAT_MOD_LINEAR &&
+			wlr_drm_format_has(b, DRM_FORMAT_MOD_INVALID)) {
 		return wlr_drm_format_dup(a);
 	}
+	if (b->len == 1 && b->modifiers[0] == DRM_FORMAT_MOD_LINEAR &&
+			wlr_drm_format_has(a, DRM_FORMAT_MOD_INVALID)) {
+		return wlr_drm_format_dup(b);
+	}
 
 	size_t format_cap = a->len < b->len ? a->len : b->len;
 	size_t format_size = sizeof(struct wlr_drm_format) +
@@ -185,9 +178,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;
 	}
diff --git a/render/egl.c b/render/egl.c
index ec23ce8d..a398d586 100644
--- a/render/egl.c
+++ b/render/egl.c
@@ -119,12 +119,11 @@ static void init_dmabuf_formats(struct wlr_egl *egl) {
 
 		has_modifiers = has_modifiers || modifiers_len > 0;
 
-		if (modifiers_len == 0) {
-			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);
-		}
+		// 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);
 
 		for (int j = 0; j < modifiers_len; j++) {
 			wlr_drm_format_set_add(&egl->dmabuf_texture_formats, fmt,