From 41849d3951c89dce619c6cc511de43005e4e2d12 Mon Sep 17 00:00:00 2001 From: itycodes Date: Sun, 27 Oct 2024 09:00:32 +0100 Subject: [PATCH] Revert "Support direct scanout with src crop and dst boxes" Seems to introduce a regression which segfaults sway when switching back to the VT. This reverts commit ac17517a5ca29a95fac88774c284f567446212bc. --- backend/drm/atomic.c | 44 +++++++-------------- backend/drm/legacy.c | 46 ++++++---------------- backend/drm/libliftoff.c | 52 +++++++++---------------- backend/wayland/output.c | 22 ----------- backend/x11/output.c | 21 ---------- include/types/wlr_output.h | 5 --- include/wlr/types/wlr_output.h | 11 ------ types/output/output.c | 70 ++++------------------------------ types/output/state.c | 4 -- types/scene/wlr_scene.c | 16 +++----- 10 files changed, 55 insertions(+), 236 deletions(-) diff --git a/backend/drm/atomic.c b/backend/drm/atomic.c index a3297bda..b7444857 100644 --- a/backend/drm/atomic.c +++ b/backend/drm/atomic.c @@ -3,7 +3,6 @@ #include #include #include -#include #include #include #include @@ -11,7 +10,6 @@ #include "backend/drm/fb.h" #include "backend/drm/iface.h" #include "backend/drm/util.h" -#include "types/wlr_output.h" static char *atomic_commit_flags_str(uint32_t flags) { const char *const l[] = { @@ -358,8 +356,7 @@ static void plane_disable(struct atomic *atom, struct wlr_drm_plane *plane) { static void set_plane_props(struct atomic *atom, struct wlr_drm_backend *drm, struct wlr_drm_plane *plane, struct wlr_drm_fb *fb, uint32_t crtc_id, - const struct wlr_box *dst_box, - const struct wlr_fbox *src_box) { + int32_t x, int32_t y) { uint32_t id = plane->id; const struct wlr_drm_plane_props *props = &plane->props; @@ -369,17 +366,20 @@ static void set_plane_props(struct atomic *atom, struct wlr_drm_backend *drm, return; } + uint32_t width = fb->wlr_buf->width; + uint32_t height = fb->wlr_buf->height; + // The src_* properties are in 16.16 fixed point - atomic_add(atom, id, props->src_x, src_box->x * (1 << 16)); - atomic_add(atom, id, props->src_y, src_box->y * (1 << 16)); - atomic_add(atom, id, props->src_w, src_box->width * (1 << 16)); - atomic_add(atom, id, props->src_h, src_box->height * (1 << 16)); + atomic_add(atom, id, props->src_x, 0); + atomic_add(atom, id, props->src_y, 0); + atomic_add(atom, id, props->src_w, (uint64_t)width << 16); + atomic_add(atom, id, props->src_h, (uint64_t)height << 16); + atomic_add(atom, id, props->crtc_w, width); + atomic_add(atom, id, props->crtc_h, height); atomic_add(atom, id, props->fb_id, fb->id); atomic_add(atom, id, props->crtc_id, crtc_id); - atomic_add(atom, id, props->crtc_x, dst_box->x); - atomic_add(atom, id, props->crtc_y, dst_box->y); - atomic_add(atom, id, props->crtc_w, dst_box->width); - atomic_add(atom, id, props->crtc_h, dst_box->height); + atomic_add(atom, id, props->crtc_x, (uint64_t)x); + atomic_add(atom, id, props->crtc_y, (uint64_t)y); } static bool supports_cursor_hotspots(const struct wlr_drm_plane *plane) { @@ -439,14 +439,8 @@ static void atomic_connector_add(struct atomic *atom, if (crtc->props.vrr_enabled != 0) { atomic_add(atom, crtc->id, crtc->props.vrr_enabled, state->vrr_enabled); } - - struct wlr_fbox src_box; - struct wlr_box dst_box; - output_state_get_buffer_src_box(state->base, &src_box); - output_state_get_buffer_dst_box(state->base, &dst_box); - set_plane_props(atom, drm, crtc->primary, state->primary_fb, crtc->id, - &dst_box, &src_box); + 0, 0); if (crtc->primary->props.fb_damage_clips != 0) { atomic_add(atom, crtc->primary->id, crtc->primary->props.fb_damage_clips, state->fb_damage_clips); @@ -459,18 +453,8 @@ static void atomic_connector_add(struct atomic *atom, } if (crtc->cursor) { if (drm_connector_is_cursor_visible(conn)) { - struct wlr_fbox cursor_src = { - .width = state->cursor_fb->wlr_buf->width, - .height = state->cursor_fb->wlr_buf->height, - }; - struct wlr_box cursor_dst = { - .x = conn->cursor_x, - .y = conn->cursor_y, - .width = state->cursor_fb->wlr_buf->width, - .height = state->cursor_fb->wlr_buf->height, - }; set_plane_props(atom, drm, crtc->cursor, state->cursor_fb, - crtc->id, &cursor_dst, &cursor_src); + crtc->id, conn->cursor_x, conn->cursor_y); if (supports_cursor_hotspots(crtc->cursor)) { atomic_add(atom, crtc->cursor->id, crtc->cursor->props.hotspot_x, conn->cursor_hotspot_x); diff --git a/backend/drm/legacy.c b/backend/drm/legacy.c index 371418c6..0c591f4d 100644 --- a/backend/drm/legacy.c +++ b/backend/drm/legacy.c @@ -7,7 +7,6 @@ #include "backend/drm/fb.h" #include "backend/drm/iface.h" #include "backend/drm/util.h" -#include "types/wlr_output.h" static bool legacy_fb_props_match(struct wlr_drm_fb *fb1, struct wlr_drm_fb *fb2) { @@ -40,41 +39,20 @@ static bool legacy_crtc_test(const struct wlr_drm_connector_state *state, struct wlr_drm_connector *conn = state->connector; struct wlr_drm_crtc *crtc = conn->crtc; - if (state->base->committed & WLR_OUTPUT_STATE_BUFFER) { - // If the size doesn't match, reject buffer (scaling is not supported) - int pending_width, pending_height; - output_pending_resolution(&state->connector->output, state->base, - &pending_width, &pending_height); - if (state->base->buffer->width != pending_width || - state->base->buffer->height != pending_height) { - wlr_log(WLR_DEBUG, "Primary buffer size mismatch"); - return false; - } - // Source crop is also not supported - struct wlr_fbox src_box; - output_state_get_buffer_src_box(state->base, &src_box); - if (src_box.x != 0.0 || src_box.y != 0.0 || - src_box.width != (double)state->base->buffer->width || - src_box.height != (double)state->base->buffer->height) { - wlr_log(WLR_DEBUG, "Source crop not supported in DRM-legacy output"); - return false; + if ((state->base->committed & WLR_OUTPUT_STATE_BUFFER) && !modeset) { + struct wlr_drm_fb *pending_fb = state->primary_fb; + + struct wlr_drm_fb *prev_fb = crtc->primary->queued_fb; + if (!prev_fb) { + prev_fb = crtc->primary->current_fb; } - if (!modeset) { - struct wlr_drm_fb *pending_fb = state->primary_fb; - - struct wlr_drm_fb *prev_fb = crtc->primary->queued_fb; - if (!prev_fb) { - prev_fb = crtc->primary->current_fb; - } - - /* Legacy is only guaranteed to be able to display a FB if it's been - * allocated the same way as the previous one. */ - if (prev_fb != NULL && !legacy_fb_props_match(prev_fb, pending_fb)) { - wlr_drm_conn_log(conn, WLR_DEBUG, - "Cannot change scan-out buffer parameters with legacy KMS API"); - return false; - } + /* Legacy is only guaranteed to be able to display a FB if it's been + * allocated the same way as the previous one. */ + if (prev_fb != NULL && !legacy_fb_props_match(prev_fb, pending_fb)) { + wlr_drm_conn_log(conn, WLR_DEBUG, + "Cannot change scan-out buffer parameters with legacy KMS API"); + return false; } } diff --git a/backend/drm/libliftoff.c b/backend/drm/libliftoff.c index 29752848..a87a4826 100644 --- a/backend/drm/libliftoff.c +++ b/backend/drm/libliftoff.c @@ -4,14 +4,12 @@ #include #include #include -#include #include #include "backend/drm/drm.h" #include "backend/drm/fb.h" #include "backend/drm/iface.h" #include "config.h" -#include "types/wlr_output.h" static void log_handler(enum liftoff_log_priority priority, const char *fmt, va_list args) { enum wlr_log_importance importance = WLR_SILENT; @@ -151,23 +149,25 @@ static bool add_prop(drmModeAtomicReq *req, uint32_t obj, } static bool set_plane_props(struct wlr_drm_plane *plane, - struct liftoff_layer *layer, struct wlr_drm_fb *fb, uint64_t zpos, - const struct wlr_box *dst_box, const struct wlr_fbox *src_box) { + struct liftoff_layer *layer, struct wlr_drm_fb *fb, int32_t x, int32_t y, uint64_t zpos) { if (fb == NULL) { wlr_log(WLR_ERROR, "Failed to acquire FB for plane %"PRIu32, plane->id); return false; } - // The src_* properties are in 16.16 fixed point + uint32_t width = fb->wlr_buf->width; + uint32_t height = fb->wlr_buf->height; + + // The SRC_* properties are in 16.16 fixed point return liftoff_layer_set_property(layer, "zpos", zpos) == 0 && - liftoff_layer_set_property(layer, "SRC_X", src_box->x * (1 << 16)) == 0 && - liftoff_layer_set_property(layer, "SRC_Y", src_box->y * (1 << 16)) == 0 && - liftoff_layer_set_property(layer, "SRC_W", src_box->width * (1 << 16)) == 0 && - liftoff_layer_set_property(layer, "SRC_H", src_box->height * (1 << 16)) == 0 && - liftoff_layer_set_property(layer, "CRTC_X", dst_box->x) == 0 && - liftoff_layer_set_property(layer, "CRTC_Y", dst_box->y) == 0 && - liftoff_layer_set_property(layer, "CRTC_W", dst_box->width) == 0 && - liftoff_layer_set_property(layer, "CRTC_H", dst_box->height) == 0 && + liftoff_layer_set_property(layer, "SRC_X", 0) == 0 && + liftoff_layer_set_property(layer, "SRC_Y", 0) == 0 && + liftoff_layer_set_property(layer, "SRC_W", (uint64_t)width << 16) == 0 && + liftoff_layer_set_property(layer, "SRC_H", (uint64_t)height << 16) == 0 && + liftoff_layer_set_property(layer, "CRTC_X", (uint64_t)x) == 0 && + liftoff_layer_set_property(layer, "CRTC_Y", (uint64_t)y) == 0 && + liftoff_layer_set_property(layer, "CRTC_W", width) == 0 && + liftoff_layer_set_property(layer, "CRTC_H", height) == 0 && liftoff_layer_set_property(layer, "FB_ID", fb->id) == 0; } @@ -331,17 +331,9 @@ static bool add_connector(drmModeAtomicReq *req, if (crtc->props.vrr_enabled != 0) { ok = ok && add_prop(req, crtc->id, crtc->props.vrr_enabled, state->vrr_enabled); } - struct wlr_fbox src_box; - struct wlr_box dst_box; - output_state_get_buffer_src_box(state->base, &src_box); - output_state_get_buffer_dst_box(state->base, &dst_box); - - ok = ok && - set_plane_props(crtc->primary, crtc->primary->liftoff_layer, - state->primary_fb, 0, &dst_box, &src_box); ok = ok && - set_plane_props(crtc->primary, crtc->liftoff_composition_layer, - state->primary_fb, 0, &dst_box, &src_box); + set_plane_props(crtc->primary, crtc->primary->liftoff_layer, state->primary_fb, 0, 0, 0) && + set_plane_props(crtc->primary, crtc->liftoff_composition_layer, state->primary_fb, 0, 0, 0); liftoff_layer_set_property(crtc->primary->liftoff_layer, "FB_DAMAGE_CLIPS", state->fb_damage_clips); liftoff_layer_set_property(crtc->liftoff_composition_layer, @@ -368,19 +360,9 @@ static bool add_connector(drmModeAtomicReq *req, if (crtc->cursor) { if (drm_connector_is_cursor_visible(conn)) { - struct wlr_fbox cursor_src = { - .width = state->cursor_fb->wlr_buf->width, - .height = state->cursor_fb->wlr_buf->height, - }; - struct wlr_box cursor_dst = { - .x = conn->cursor_x, - .y = conn->cursor_y, - .width = state->cursor_fb->wlr_buf->width, - .height = state->cursor_fb->wlr_buf->height, - }; ok = ok && set_plane_props(crtc->cursor, crtc->cursor->liftoff_layer, - state->cursor_fb, wl_list_length(&crtc->layers) + 1, - &cursor_dst, &cursor_src); + state->cursor_fb, conn->cursor_x, conn->cursor_y, + wl_list_length(&crtc->layers) + 1); } else { ok = ok && disable_plane(crtc->cursor); } diff --git a/backend/wayland/output.c b/backend/wayland/output.c index 5b8dfb0f..e672028b 100644 --- a/backend/wayland/output.c +++ b/backend/wayland/output.c @@ -300,28 +300,6 @@ static bool output_test(struct wlr_output *wlr_output, struct wlr_wl_output *output = get_wl_output_from_output(wlr_output); - if (state->committed & WLR_OUTPUT_STATE_BUFFER) { - // If the size doesn't match, reject buffer (scaling is not currently - // supported but could be implemented with viewporter) - int pending_width, pending_height; - output_pending_resolution(wlr_output, state, - &pending_width, &pending_height); - if (state->buffer->width != pending_width || - state->buffer->height != pending_height) { - wlr_log(WLR_DEBUG, "Primary buffer size mismatch"); - return false; - } - // Source crop is also not currently supported - struct wlr_fbox src_box; - output_state_get_buffer_src_box(state, &src_box); - if (src_box.x != 0.0 || src_box.y != 0.0 || - src_box.width != (double)state->buffer->width || - src_box.height != (double)state->buffer->height) { - wlr_log(WLR_DEBUG, "Source crop not supported in wayland output"); - return false; - } - } - uint32_t unsupported = state->committed & ~SUPPORTED_OUTPUT_STATE; if (unsupported != 0) { wlr_log(WLR_DEBUG, "Unsupported output state fields: 0x%"PRIx32, diff --git a/backend/x11/output.c b/backend/x11/output.c index 67f1dae9..b400f3df 100644 --- a/backend/x11/output.c +++ b/backend/x11/output.c @@ -129,27 +129,6 @@ static bool output_test(struct wlr_output *wlr_output, return false; } - if (state->committed & WLR_OUTPUT_STATE_BUFFER) { - // If the size doesn't match, reject buffer (scaling is not supported) - int pending_width, pending_height; - output_pending_resolution(wlr_output, state, - &pending_width, &pending_height); - if (state->buffer->width != pending_width || - state->buffer->height != pending_height) { - wlr_log(WLR_DEBUG, "Primary buffer size mismatch"); - return false; - } - // Source crop is not supported - struct wlr_fbox src_box; - output_state_get_buffer_src_box(state, &src_box); - if (src_box.x != 0.0 || src_box.y != 0.0 || - src_box.width != (double)state->buffer->width || - src_box.height != (double)state->buffer->height) { - wlr_log(WLR_DEBUG, "Source crop not supported in X11 output"); - return false; - } - } - // All we can do to influence adaptive sync on the X11 backend is set the // _VARIABLE_REFRESH window property like mesa automatically does. We don't // have any control beyond that, so we set the state to enabled on creating diff --git a/include/types/wlr_output.h b/include/types/wlr_output.h index 2dc979c6..bd095d8f 100644 --- a/include/types/wlr_output.h +++ b/include/types/wlr_output.h @@ -26,9 +26,4 @@ void output_defer_present(struct wlr_output *output, struct wlr_output_event_pre bool output_prepare_commit(struct wlr_output *output, const struct wlr_output_state *state); void output_apply_commit(struct wlr_output *output, const struct wlr_output_state *state); -void output_state_get_buffer_src_box(const struct wlr_output_state *state, - struct wlr_fbox *out); -void output_state_get_buffer_dst_box(const struct wlr_output_state *state, - struct wlr_box *out); - #endif diff --git a/include/wlr/types/wlr_output.h b/include/wlr/types/wlr_output.h index 1cc7baec..3ed99b9a 100644 --- a/include/wlr/types/wlr_output.h +++ b/include/wlr/types/wlr_output.h @@ -17,7 +17,6 @@ #include #include #include -#include enum wlr_output_mode_aspect_ratio { WLR_OUTPUT_MODE_ASPECT_RATIO_NONE, @@ -95,16 +94,6 @@ struct wlr_output_state { enum wl_output_subpixel subpixel; struct wlr_buffer *buffer; - // Source crop for the buffer. If all zeros then no crop is applied. - // As usual with source crop, this is in buffer coordinates. - // Double-buffered by WLR_OUTPUT_STATE_BUFFER along with `buffer`. - struct wlr_fbox buffer_src_box; - // Destination rect to scale the buffer to (after source crop). If width - // and height are zero then the buffer is displayed at native size. The - // offset is relative to the origin of this output. Double-buffered by - // WLR_OUTPUT_STATE_BUFFER along with `buffer`. - struct wlr_box buffer_dst_box; - /* Request a tearing page-flip. When enabled, this may cause the output to * display a part of the previous buffer and a part of the current buffer at * the same time. The backend may reject the commit if a tearing page-flip diff --git a/types/output/output.c b/types/output/output.c index f8ed118f..e52803fc 100644 --- a/types/output/output.c +++ b/types/output/output.c @@ -548,37 +548,14 @@ static uint32_t output_compare_state(struct wlr_output *output, static bool output_basic_test(struct wlr_output *output, const struct wlr_output_state *state) { if (state->committed & WLR_OUTPUT_STATE_BUFFER) { - struct wlr_fbox src_box; - output_state_get_buffer_src_box(state, &src_box); - - // Source box must be contained within the buffer - if (src_box.x < 0.0 || src_box.y < 0.0 || - src_box.x + src_box.width > state->buffer->width || - src_box.y + src_box.height > state->buffer->height) { - wlr_log(WLR_ERROR, "Tried to commit with invalid buffer_src_box"); - return false; - } - - // Source box must not be empty (but it can be smaller than 1 pixel, - // some DRM devices support sub-pixel crops) - if (wlr_fbox_empty(&src_box)) { - wlr_log(WLR_ERROR, "Tried to commit with an empty buffer_src_box"); - return false; - } - - // Destination box cannot be entirely off-screen (but it also doesn't - // have to be entirely on-screen). This also checks the dst box is - // not empty. + // If the size doesn't match, reject buffer (scaling is not + // supported) int pending_width, pending_height; - output_pending_resolution(output, state, &pending_width, &pending_height); - struct wlr_box output_box = { - .width = pending_width, - .height = pending_height - }; - struct wlr_box dst_box; - output_state_get_buffer_dst_box(state, &dst_box); - if (!wlr_box_intersection(&output_box, &output_box, &dst_box)) { - wlr_log(WLR_ERROR, "Primary buffer is entirely off-screen or 0-sized"); + output_pending_resolution(output, state, + &pending_width, &pending_height); + if (state->buffer->width != pending_width || + state->buffer->height != pending_height) { + wlr_log(WLR_DEBUG, "Primary buffer size mismatch"); return false; } } else { @@ -868,39 +845,6 @@ void output_defer_present(struct wlr_output *output, struct wlr_output_event_pre deferred_present_event_handle_idle, deferred); } -void output_state_get_buffer_src_box(const struct wlr_output_state *state, - struct wlr_fbox *out) { - out->x = state->buffer_src_box.x; - out->y = state->buffer_src_box.y; - // If the source box is unset then default to the whole buffer. - if (state->buffer_src_box.width == 0.0 && - state->buffer_src_box.height == 0.0) { - out->width = (double)state->buffer->width; - out->height = (double)state->buffer->height; - } else { - out->width = state->buffer_src_box.width; - out->height = state->buffer_src_box.height; - } -} - -void output_state_get_buffer_dst_box(const struct wlr_output_state *state, - struct wlr_box *out) { - out->x = state->buffer_dst_box.x; - out->y = state->buffer_dst_box.y; - // If the dst box is unset then default to source crop size (which itself - // defaults to the whole buffer size if unset) - if (state->buffer_dst_box.width == 0 && - state->buffer_dst_box.height == 0) { - struct wlr_fbox src_box; - output_state_get_buffer_src_box(state, &src_box); - out->width = (int)src_box.width; - out->height = (int)src_box.height; - } else { - out->width = state->buffer_dst_box.width; - out->height = state->buffer_dst_box.height; - } -} - void wlr_output_send_request_state(struct wlr_output *output, const struct wlr_output_state *state) { uint32_t unchanged = output_compare_state(output, state); diff --git a/types/output/state.c b/types/output/state.c index 72849c02..465b54ad 100644 --- a/types/output/state.c +++ b/types/output/state.c @@ -142,8 +142,6 @@ bool wlr_output_state_copy(struct wlr_output_state *dst, WLR_OUTPUT_STATE_WAIT_TIMELINE | WLR_OUTPUT_STATE_SIGNAL_TIMELINE); copy.buffer = NULL; - copy.buffer_src_box = (struct wlr_fbox){0}; - copy.buffer_dst_box = (struct wlr_box){0}; pixman_region32_init(©.damage); copy.gamma_lut = NULL; copy.gamma_lut_size = 0; @@ -152,8 +150,6 @@ bool wlr_output_state_copy(struct wlr_output_state *dst, if (src->committed & WLR_OUTPUT_STATE_BUFFER) { wlr_output_state_set_buffer(©, src->buffer); - copy.buffer_src_box = src->buffer_src_box; - copy.buffer_dst_box = src->buffer_dst_box; } if (src->committed & WLR_OUTPUT_STATE_DAMAGE) { diff --git a/types/scene/wlr_scene.c b/types/scene/wlr_scene.c index e4d68654..58ad0e49 100644 --- a/types/scene/wlr_scene.c +++ b/types/scene/wlr_scene.c @@ -1831,7 +1831,6 @@ static bool scene_entry_try_direct_scanout(struct render_list_entry *entry, return false; } - // The native size of the buffer after any transform is applied int default_width = buffer->buffer->width; int default_height = buffer->buffer->height; wlr_output_transform_coords(buffer->transform, &default_width, &default_height); @@ -1840,6 +1839,11 @@ static bool scene_entry_try_direct_scanout(struct render_list_entry *entry, .height = default_height, }; + if (!wlr_fbox_empty(&buffer->src_box) && + !wlr_fbox_equal(&buffer->src_box, &default_box)) { + return false; + } + if (buffer->transform != data->transform) { return false; } @@ -1860,16 +1864,6 @@ static bool scene_entry_try_direct_scanout(struct render_list_entry *entry, return false; } - if (!wlr_fbox_empty(&buffer->src_box) && - !wlr_fbox_equal(&buffer->src_box, &default_box)) { - pending.buffer_src_box = buffer->src_box; - } - - // Translate the location from scene coordinates to output coordinates - pending.buffer_dst_box.x = entry->x - scene_output->x; - pending.buffer_dst_box.y = entry->y - scene_output->y; - scene_node_get_size(node, &pending.buffer_dst_box.width, &pending.buffer_dst_box.height); - wlr_output_state_set_buffer(&pending, buffer->buffer); if (buffer->wait_timeline != NULL) { wlr_output_state_set_wait_timeline(&pending, buffer->wait_timeline, buffer->wait_point);