wlr_drm_format: Rework wlr_drm_format_intersect

Now it takes a reference to a destination format

backend/drm/drm.c

@@ -503,18 +503,16 @@ static bool drm_connector_state_update_primary_fb(struct wlr_drm_connector *conn
 
 	struct wlr_buffer *local_buf;
 	if (drm->parent) {
-		struct wlr_drm_format *format =
-			drm_plane_pick_render_format(plane, &drm->mgpu_renderer);
-		if (format == NULL) {
+		struct wlr_drm_format format = {0};
+		if (!drm_plane_pick_render_format(plane, &format, &drm->mgpu_renderer)) {
 			wlr_log(WLR_ERROR, "Failed to pick primary plane format");
 			return false;
 		}
 
 		// TODO: fallback to modifier-less buffer allocation
 		bool ok = init_drm_surface(&plane->mgpu_surf, &drm->mgpu_renderer,
-			source_buf->width, source_buf->height, format);
-		wlr_drm_format_finish(format);
-		free(format);
+			source_buf->width, source_buf->height, &format);
+		wlr_drm_format_finish(&format);
 		if (!ok) {
 			return false;
 		}
@@ -945,17 +943,15 @@ static bool drm_connector_set_cursor(struct wlr_output *output,
 
 		struct wlr_buffer *local_buf;
 		if (drm->parent) {
-			struct wlr_drm_format *format =
-				drm_plane_pick_render_format(plane, &drm->mgpu_renderer);
-			if (format == NULL) {
+			struct wlr_drm_format format = {0};
+			if (!drm_plane_pick_render_format(plane, &format, &drm->mgpu_renderer)) {
 				wlr_log(WLR_ERROR, "Failed to pick cursor plane format");
 				return false;
 			}
 
 			bool ok = init_drm_surface(&plane->mgpu_surf, &drm->mgpu_renderer,
-				buffer->width, buffer->height, format);
-			wlr_drm_format_finish(format);
-			free(format);
+				buffer->width, buffer->height, &format);
+			wlr_drm_format_finish(&format);
 			if (!ok) {
 				return false;
 			}

backend/drm/renderer.c

@@ -136,50 +136,48 @@ void drm_plane_finish_surface(struct wlr_drm_plane *plane) {
 	finish_drm_surface(&plane->mgpu_surf);
 }
 
-struct wlr_drm_format *drm_plane_pick_render_format(
-		struct wlr_drm_plane *plane, struct wlr_drm_renderer *renderer) {
+bool drm_plane_pick_render_format(struct wlr_drm_plane *plane,
+		struct wlr_drm_format *fmt, struct wlr_drm_renderer *renderer) {
 	const struct wlr_drm_format_set *render_formats =
 		wlr_renderer_get_render_formats(renderer->wlr_rend);
 	if (render_formats == NULL) {
 		wlr_log(WLR_ERROR, "Failed to get render formats");
-		return NULL;
+		return false;
 	}
 
 	const struct wlr_drm_format_set *plane_formats = &plane->formats;
 
-	uint32_t fmt = DRM_FORMAT_ARGB8888;
-	if (!wlr_drm_format_set_get(&plane->formats, fmt)) {
+	uint32_t format = DRM_FORMAT_ARGB8888;
+	if (!wlr_drm_format_set_get(&plane->formats, format)) {
 		const struct wlr_pixel_format_info *format_info =
-			drm_get_pixel_format_info(fmt);
+			drm_get_pixel_format_info(format);
 		assert(format_info != NULL &&
 			format_info->opaque_substitute != DRM_FORMAT_INVALID);
-		fmt = format_info->opaque_substitute;
+		format = format_info->opaque_substitute;
 	}
 
 	const struct wlr_drm_format *render_format =
-		wlr_drm_format_set_get(render_formats, fmt);
+		wlr_drm_format_set_get(render_formats, format);
 	if (render_format == NULL) {
-		wlr_log(WLR_DEBUG, "Renderer doesn't support format 0x%"PRIX32, fmt);
-		return NULL;
+		wlr_log(WLR_DEBUG, "Renderer doesn't support format 0x%"PRIX32, format);
+		return false;
 	}
 
 	const struct wlr_drm_format *plane_format =
-		wlr_drm_format_set_get(plane_formats, fmt);
+		wlr_drm_format_set_get(plane_formats, format);
 	if (plane_format == NULL) {
 		wlr_log(WLR_DEBUG, "Plane %"PRIu32" doesn't support format 0x%"PRIX32,
-			plane->id, fmt);
-		return NULL;
+			plane->id, format);
+		return false;
 	}
 
-	struct wlr_drm_format *format =
-		wlr_drm_format_intersect(plane_format, render_format);
-	if (format == NULL) {
+	if (!wlr_drm_format_intersect(fmt, plane_format, render_format)) {
 		wlr_log(WLR_DEBUG, "Failed to intersect plane and render "
-			"modifiers for format 0x%"PRIX32, fmt);
-		return NULL;
+			"modifiers for format 0x%"PRIX32, format);
+		return false;
 	}
 
-	return format;
+	return true;
 }
 
 void drm_fb_clear(struct wlr_drm_fb **fb_ptr) {

include/backend/drm/renderer.h

@@ -50,8 +50,8 @@ struct wlr_drm_fb *drm_fb_lock(struct wlr_drm_fb *fb);
 struct wlr_buffer *drm_surface_blit(struct wlr_drm_surface *surf,
 	struct wlr_buffer *buffer);
 
-struct wlr_drm_format *drm_plane_pick_render_format(
-		struct wlr_drm_plane *plane, struct wlr_drm_renderer *renderer);
+bool drm_plane_pick_render_format(struct wlr_drm_plane *plane,
+	struct wlr_drm_format *fmt, struct wlr_drm_renderer *renderer);
 void drm_plane_finish_surface(struct wlr_drm_plane *plane);
 
 #endif

include/render/drm_format_set.h

@@ -8,13 +8,14 @@ bool wlr_drm_format_has(const struct wlr_drm_format *fmt, uint64_t modifier);
 bool wlr_drm_format_add(struct wlr_drm_format *fmt, uint64_t modifier);
 bool wlr_drm_format_copy(struct wlr_drm_format *dst, const struct wlr_drm_format *src);
 /**
- * Intersect modifiers for two DRM formats.
+ * Intersect modifiers for two DRM formats. The `dst` must be zeroed or initialized
+ * with other state being replaced.
  *
  * Both arguments must have the same format field. If the formats aren't
  * compatible, NULL is returned. If either format doesn't support any modifier,
  * a format that doesn't support any modifier is returned.
  */
-struct wlr_drm_format *wlr_drm_format_intersect(
+bool wlr_drm_format_intersect(struct wlr_drm_format *dst,
 	const struct wlr_drm_format *a, const struct wlr_drm_format *b);
 
 bool wlr_drm_format_set_copy(struct wlr_drm_format_set *dst, const struct wlr_drm_format_set *src);

include/types/wlr_output.h

@@ -7,8 +7,9 @@
 void output_pending_resolution(struct wlr_output *output,
 	const struct wlr_output_state *state, int *width, int *height);
 
-struct wlr_drm_format *output_pick_format(struct wlr_output *output,
-	const struct wlr_drm_format_set *display_formats, uint32_t format);
+bool output_pick_format(struct wlr_output *output,
+	const struct wlr_drm_format_set *display_formats,
+	struct wlr_drm_format *format, uint32_t fmt);
 void output_clear_back_buffer(struct wlr_output *output);
 bool output_ensure_buffer(struct wlr_output *output,
 	const struct wlr_output_state *state, bool *new_back_buffer);

render/drm_format_set.c

@@ -183,7 +183,7 @@ bool wlr_drm_format_set_copy(struct wlr_drm_format_set *dst, const struct wlr_dr
 	return true;
 }
 
-struct wlr_drm_format *wlr_drm_format_intersect(
+bool wlr_drm_format_intersect(struct wlr_drm_format *dst,
 		const struct wlr_drm_format *a, const struct wlr_drm_format *b) {
 	assert(a->format == b->format);
 
@@ -210,20 +210,9 @@ struct wlr_drm_format *wlr_drm_format_intersect(
 		}
 	}
 
-	// If the intersection is empty, then the formats aren't compatible with
-	// each other.
-	if (fmt.len == 0) {
-		wlr_drm_format_finish(&fmt);
-		return NULL;
-	}
-
-	struct wlr_drm_format *format = calloc(1, sizeof(*format));
-	if (!format) {
-		wlr_drm_format_finish(&fmt);
-		return NULL;
-	}
-	*format = fmt;
-	return format;
+	wlr_drm_format_finish(dst);
+	*dst = fmt;
+	return true;
 }
 
 bool wlr_drm_format_set_intersect(struct wlr_drm_format_set *dst,
@@ -242,12 +231,24 @@ bool wlr_drm_format_set_intersect(struct wlr_drm_format_set *dst,
 				// When the two formats have no common modifier, keep
 				// intersecting the rest of the formats: they may be compatible
 				// with each other
-				struct wlr_drm_format *format =
-					wlr_drm_format_intersect(a->formats[i], b->formats[j]);
-				if (format != NULL) {
+				struct wlr_drm_format *format = calloc(1, sizeof(*format));
+				if (!format) {
+					wlr_drm_format_set_finish(&out);
+					return false;
+				}
+
+				if (!wlr_drm_format_intersect(format, a->formats[i], b->formats[j])) {
+					wlr_drm_format_set_finish(&out);
+					return false;
+				}
+
+				if (format->len == 0) {
+					wlr_drm_format_finish(format);
+				} else {
 					out.formats[out.len] = format;
 					out.len++;
 				}
+
 				break;
 			}
 		}

types/output/cursor.c

@@ -242,7 +242,8 @@ static void output_cursor_update_visible(struct wlr_output_cursor *cursor) {
 		wlr_box_intersection(&intersection, &output_box, &cursor_box);
 }
 
-static struct wlr_drm_format *output_pick_cursor_format(struct wlr_output *output) {
+static bool output_pick_cursor_format(struct wlr_output *output,
+		struct wlr_drm_format *format) {
 	struct wlr_allocator *allocator = output->allocator;
 	assert(allocator != NULL);
 
@@ -252,11 +253,11 @@ static struct wlr_drm_format *output_pick_cursor_format(struct wlr_output *outpu
 			output->impl->get_cursor_formats(output, allocator->buffer_caps);
 		if (display_formats == NULL) {
 			wlr_log(WLR_DEBUG, "Failed to get cursor display formats");
-			return NULL;
+			return false;
 		}
 	}
 
-	return output_pick_format(output, display_formats, DRM_FORMAT_ARGB8888);
+	return output_pick_format(output, display_formats, format, DRM_FORMAT_ARGB8888);
 }
 
 static struct wlr_buffer *render_cursor_buffer(struct wlr_output_cursor *cursor) {
@@ -289,18 +290,16 @@ static struct wlr_buffer *render_cursor_buffer(struct wlr_output_cursor *cursor)
 	if (output->cursor_swapchain == NULL ||
 			output->cursor_swapchain->width != width ||
 			output->cursor_swapchain->height != height) {
-		struct wlr_drm_format *format =
-			output_pick_cursor_format(output);
-		if (format == NULL) {
+		struct wlr_drm_format format = {0};
+		if (!output_pick_cursor_format(output, &format)) {
 			wlr_log(WLR_DEBUG, "Failed to pick cursor format");
 			return NULL;
 		}
 
 		wlr_swapchain_destroy(output->cursor_swapchain);
 		output->cursor_swapchain = wlr_swapchain_create(allocator,
-			width, height, format);
-		wlr_drm_format_finish(format);
-		free(format);
+			width, height, &format);
+		wlr_drm_format_finish(&format);
 		if (output->cursor_swapchain == NULL) {
 			wlr_log(WLR_ERROR, "Failed to create cursor swapchain");
 			return NULL;

types/output/output.c

@@ -602,15 +602,13 @@ static bool output_basic_test(struct wlr_output *output,
 
 		const struct wlr_drm_format_set *display_formats =
 			wlr_output_get_primary_formats(output, allocator->buffer_caps);
-		struct wlr_drm_format *format = output_pick_format(output, display_formats,
-			state->render_format);
-		if (format == NULL) {
+		struct wlr_drm_format format = {0};
+		if (!output_pick_format(output, display_formats, &format, state->render_format)) {
 			wlr_log(WLR_ERROR, "Failed to pick primary buffer format for output");
 			return false;
 		}
 
-		wlr_drm_format_finish(format);
-		free(format);
+		wlr_drm_format_finish(&format);
 	}
 
 	bool enabled = output->enabled;

types/output/render.c

@@ -179,9 +179,9 @@ void wlr_output_lock_attach_render(struct wlr_output *output, bool lock) {
 		output->attach_render_locks);
 }
 
-struct wlr_drm_format *output_pick_format(struct wlr_output *output,
+bool output_pick_format(struct wlr_output *output,
 		const struct wlr_drm_format_set *display_formats,
-		uint32_t fmt) {
+		struct wlr_drm_format *format, uint32_t fmt) {
 	struct wlr_renderer *renderer = output->renderer;
 	struct wlr_allocator *allocator = output->allocator;
 	assert(renderer != NULL && allocator != NULL);
@@ -190,43 +190,37 @@ struct wlr_drm_format *output_pick_format(struct wlr_output *output,
 		wlr_renderer_get_render_formats(renderer);
 	if (render_formats == NULL) {
 		wlr_log(WLR_ERROR, "Failed to get render formats");
-		return NULL;
+		return false;
 	}
 
 	const struct wlr_drm_format *render_format =
 		wlr_drm_format_set_get(render_formats, fmt);
 	if (render_format == NULL) {
 		wlr_log(WLR_DEBUG, "Renderer doesn't support format 0x%"PRIX32, fmt);
-		return NULL;
+		return false;
 	}
 
-	struct wlr_drm_format *format = NULL;
 	if (display_formats != NULL) {
 		const struct wlr_drm_format *display_format =
 			wlr_drm_format_set_get(display_formats, fmt);
 		if (display_format == NULL) {
 			wlr_log(WLR_DEBUG, "Output doesn't support format 0x%"PRIX32, fmt);
-			return NULL;
+			return false;
 		}
-		format = wlr_drm_format_intersect(display_format, render_format);
-	} else {
-		format = calloc(1, sizeof(*format));
-		if (!format) {
+		if (!wlr_drm_format_intersect(format, display_format, render_format)) {
+			wlr_log(WLR_DEBUG, "Failed to intersect display and render "
+				"modifiers for format 0x%"PRIX32 " on output %s",
+				fmt, output->name);
 			return false;
 		}
-
+	} else {
 		// The output can display any format
-		wlr_drm_format_copy(format, render_format);
-	}
-
-	if (format == NULL) {
-		wlr_log(WLR_DEBUG, "Failed to intersect display and render "
-			"modifiers for format 0x%"PRIX32 " on output %s",
-			fmt, output->name);
-		return NULL;
+		if (!wlr_drm_format_copy(format, render_format)) {
+			return false;
+		}
 	}
 
-	return format;
+	return true;
 }
 
 uint32_t wlr_output_preferred_read_format(struct wlr_output *output) {

types/output/swapchain.c

@@ -16,34 +16,35 @@ static struct wlr_swapchain *create_swapchain(struct wlr_output *output,
 
 	const struct wlr_drm_format_set *display_formats =
 		wlr_output_get_primary_formats(output, allocator->buffer_caps);
-	struct wlr_drm_format *format = output_pick_format(output, display_formats,
-		output->render_format);
-	if (format == NULL) {
+	struct wlr_drm_format format = {0};
+	if (!output_pick_format(output, display_formats, &format, output->render_format)) {
 		wlr_log(WLR_ERROR, "Failed to pick primary buffer format for output '%s'",
 			output->name);
 		return NULL;
 	}
 
-	char *format_name = drmGetFormatName(format->format);
+	char *format_name = drmGetFormatName(format.format);
 	wlr_log(WLR_DEBUG, "Choosing primary buffer format %s (0x%08"PRIX32") for output '%s'",
-		format_name ? format_name : "<unknown>", format->format, output->name);
+		format_name ? format_name : "<unknown>", format.format, output->name);
 	free(format_name);
 
-	if (!allow_modifiers && (format->len != 1 || format->modifiers[0] != DRM_FORMAT_MOD_LINEAR)) {
-		if (!wlr_drm_format_has(format, DRM_FORMAT_MOD_INVALID)) {
+	if (!allow_modifiers && (format.len != 1 || format.modifiers[0] != DRM_FORMAT_MOD_LINEAR)) {
+		if (!wlr_drm_format_has(&format, DRM_FORMAT_MOD_INVALID)) {
 			wlr_log(WLR_DEBUG, "Implicit modifiers not supported");
-			wlr_drm_format_finish(format);
-			free(format);
+			wlr_drm_format_finish(&format);
 			return NULL;
 		}
 
-		format->len = 0;
-		wlr_drm_format_add(format, DRM_FORMAT_MOD_INVALID);
+		format.len = 0;
+		if (!wlr_drm_format_add(&format, DRM_FORMAT_MOD_INVALID)) {
+			wlr_log(WLR_DEBUG, "Failed to add implicit modifier to format");
+			wlr_drm_format_finish(&format);
+			return NULL;
+		}
 	}
 
-	struct wlr_swapchain *swapchain = wlr_swapchain_create(allocator, width, height, format);
-	wlr_drm_format_finish(format);
-	free(format);
+	struct wlr_swapchain *swapchain = wlr_swapchain_create(allocator, width, height, &format);
+	wlr_drm_format_finish(&format);
 	return swapchain;
 }