From 2a9193c454a19d6c7aa189acbd497de28cbf88a0 Mon Sep 17 00:00:00 2001 From: Justin Huang Date: Mon, 25 Sep 2023 17:30:20 +1000 Subject: [PATCH] vm_tools: Refactors sommelier-output to batch up and group send updates This is a pre-patch for the introduction of xdg-output. By batching up the output/update information inside one function it'll make it easier to integrate the logic of forwarding output and direct scale together. Also added some comments to help explain what all of the various scale numbers are in sl_context and sl_host_output. BUG=b:289158040 TEST=Unittests run, tested on device. Change-Id: Ib43853af6ef5a11ea0d4a6dcd28293333f443140 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/4891392 Reviewed-by: Chloe Pelling Commit-Queue: Justin Huang Tested-by: Justin Huang --- sommelier-ctx.h | 5 +++ sommelier-output-test.cc | 19 -------- sommelier-output.cc | 96 +++++++++++++++++----------------------- sommelier-transform.cc | 2 - sommelier-window.cc | 1 - sommelier.cc | 6 ++- sommelier.h | 40 ++++++++++++++--- 7 files changed, 84 insertions(+), 85 deletions(-) diff --git a/sommelier-ctx.h b/sommelier-ctx.h index 1ec9583..85f542d 100644 --- a/sommelier-ctx.h +++ b/sommelier-ctx.h @@ -67,6 +67,7 @@ enum { ATOM_LAST = ATOM_SOMMELIER_QUIRK_APPLIED, }; +// A series of configurations and objects shared globally. struct sl_context { char** runprog; @@ -144,7 +145,11 @@ struct sl_context { #ifdef GAMEPAD_SUPPORT struct wl_list gamepads; #endif + // The scale provided by the --scale argument to sommelier. double desired_scale; + // The calculated value of desired_scale * default_scale. Default scale is the + // device_scale_factor * preferred_scale (usually preferred_scale = 1) of the + // internal screen (which is assumed to be the first output). double scale; // These scale factors are used for the direct scaling mode. diff --git a/sommelier-output-test.cc b/sommelier-output-test.cc index 641015a..6294fba 100644 --- a/sommelier-output-test.cc +++ b/sommelier-output-test.cc @@ -53,12 +53,10 @@ TEST_F(X11DirectScaleTest, output = ctx.host_outputs[0]; EXPECT_EQ(output->x, 0); EXPECT_EQ(output->virt_x, 0); - EXPECT_EQ(output->virt_y, 0); output = ctx.host_outputs[1]; EXPECT_EQ(output->x, 1920); EXPECT_EQ(output->virt_x, 1920); - EXPECT_EQ(output->virt_y, 0); // Advertise another output to the left. AdvertiseOutputs(xwayland.get(), {{.x = -800, @@ -71,17 +69,14 @@ TEST_F(X11DirectScaleTest, output = ctx.host_outputs[0]; EXPECT_EQ(output->x, -800); EXPECT_EQ(output->virt_x, 0); - EXPECT_EQ(output->virt_y, 0); output = ctx.host_outputs[1]; EXPECT_EQ(output->x, 0); EXPECT_EQ(output->virt_x, 1080); - EXPECT_EQ(output->virt_y, 0); output = ctx.host_outputs[2]; EXPECT_EQ(output->x, 1920); EXPECT_EQ(output->virt_x, 3000); - EXPECT_EQ(output->virt_y, 0); } TEST_F(X11Test, AddingMultipleOutputsPositionsCorrectly) { @@ -102,12 +97,10 @@ TEST_F(X11Test, AddingMultipleOutputsPositionsCorrectly) { output = ctx.host_outputs[0]; EXPECT_EQ(output->x, 0); EXPECT_EQ(output->virt_x, 0); - EXPECT_EQ(output->virt_y, 0); output = ctx.host_outputs[1]; EXPECT_EQ(output->x, 1920); EXPECT_EQ(output->virt_x, 1920); - EXPECT_EQ(output->virt_y, 0); // Act AdvertiseOutputs(xwayland.get(), {{ @@ -123,17 +116,14 @@ TEST_F(X11Test, AddingMultipleOutputsPositionsCorrectly) { output = ctx.host_outputs[0]; EXPECT_EQ(output->x, -800); EXPECT_EQ(output->virt_x, 0); - EXPECT_EQ(output->virt_y, 0); output = ctx.host_outputs[1]; EXPECT_EQ(output->x, 0); EXPECT_EQ(output->virt_x, 1440); - EXPECT_EQ(output->virt_y, 0); output = ctx.host_outputs[2]; EXPECT_EQ(output->x, 1920); EXPECT_EQ(output->virt_x, 3360); - EXPECT_EQ(output->virt_y, 0); } TEST_F(X11Test, OutputsPositionedCorrectlyAfterRemovingLeftOutput) { @@ -159,12 +149,10 @@ TEST_F(X11Test, OutputsPositionedCorrectlyAfterRemovingLeftOutput) { output = ctx.host_outputs[0]; EXPECT_EQ(output->x, 1920); EXPECT_EQ(output->virt_x, 0); - EXPECT_EQ(output->virt_y, 0); output = ctx.host_outputs[1]; EXPECT_EQ(output->x, 2000); EXPECT_EQ(output->virt_x, 540); - EXPECT_EQ(output->virt_y, 0); // outputs has length 2. EXPECT_EQ(ctx.host_outputs.size(), 2u); @@ -193,12 +181,10 @@ TEST_F(X11Test, OutputsPositionedCorrectlyAfterRemovingMiddleOutput) { output = ctx.host_outputs[0]; EXPECT_EQ(output->x, 0); EXPECT_EQ(output->virt_x, 0); - EXPECT_EQ(output->virt_y, 0); output = ctx.host_outputs[1]; EXPECT_EQ(output->x, 2000); EXPECT_EQ(output->virt_x, 1920); - EXPECT_EQ(output->virt_y, 0); // outputs has length 2. EXPECT_EQ(ctx.host_outputs.size(), 2u); @@ -220,7 +206,6 @@ TEST_F(X11Test, OtherOutputUnchangedAfterRemovingRightOutput) { output = ctx.host_outputs[0]; EXPECT_EQ(output->x, 0); EXPECT_EQ(output->virt_x, 0); - EXPECT_EQ(output->virt_y, 0); // outputs has length 1. EXPECT_EQ(ctx.host_outputs.size(), 1u); } @@ -240,13 +225,11 @@ TEST_F(X11Test, RotatingOutputsShiftsNeighbouringOutputs) { // Assert: Output is rotated and next output is shifted left. EXPECT_EQ(output->virt_x, 0); - EXPECT_EQ(output->virt_y, 0); EXPECT_EQ(output->virt_rotated_width, 1080); EXPECT_EQ(output->virt_rotated_height, 1920); output = ctx.host_outputs[1]; EXPECT_EQ(output->virt_x, 1080); - EXPECT_EQ(output->virt_y, 0); } TEST_F(X11Test, MovingOutputsShiftsOutputs) { @@ -266,12 +249,10 @@ TEST_F(X11Test, MovingOutputsShiftsOutputs) { output = ctx.host_outputs[0]; EXPECT_EQ(output->x, -1920); EXPECT_EQ(output->virt_x, 0); - EXPECT_EQ(output->virt_y, 0); output = ctx.host_outputs[1]; EXPECT_EQ(output->x, 0); EXPECT_EQ(output->virt_x, 1920); - EXPECT_EQ(output->virt_y, 0); } } // namespace sommelier diff --git a/sommelier-output.cc b/sommelier-output.cc index 64d0c6a..87d3b93 100644 --- a/sommelier-output.cc +++ b/sommelier-output.cc @@ -276,48 +276,16 @@ void sl_output_get_dimensions_original(struct sl_host_output* host, } // Recalculates the virt_x coordinates of outputs when an output is -// add/removed/changed. skip_host is false if the host is being removed -// (as it should not exist in the list already) and true if it's being -// added/changed. -void sl_output_shift_output_x(struct sl_host_output* host, bool skip_host) { - // Outputs are positioned in a line from left to right ordered base on its - // x position. +// add/removed/changed. +void sl_output_update_output_x(struct sl_context* ctx) { + // Outputs are positioned in a line from left to right based off their x + // position. int next_output_x = 0; - for (auto output : host->ctx->host_outputs) { + for (auto output : ctx->host_outputs) { + // Update the value and mark for sending. if (output->virt_x != next_output_x) { output->virt_x = next_output_x; - // Skipping sending current output's details here if skip host is set - // as they are sent after this method. - if (!skip_host || output != host) { - // virt_physical_width/height may have not been set yet. - if (!output->virt_physical_width || !output->virt_physical_height) { - int scale; - int virt_physical_width; - int virt_physical_height; - int virt_width; - int virt_height; - - if (host->ctx->use_direct_scale) { - sl_output_init_dimensions_direct(host, &scale, &virt_physical_width, - &virt_physical_height, &virt_width, - &virt_height); - } else { - sl_output_get_dimensions_original( - host, &scale, &virt_physical_width, &virt_physical_height, - &virt_width, &virt_height); - } - host->virt_physical_width = virt_physical_width; - host->virt_physical_height = virt_physical_height; - } - - wl_output_send_geometry(output->resource, output->virt_x, - output->virt_y, output->virt_physical_width, - output->virt_physical_height, output->subpixel, - output->make, output->model, output->transform); - if (wl_resource_get_version(output->resource) >= - WL_OUTPUT_DONE_SINCE_VERSION) - wl_output_send_done(output->resource); - } + output->needs_update = true; } next_output_x += output->virt_rotated_width; } @@ -393,7 +361,7 @@ struct sl_host_output* sl_infer_output_for_guest_position( return last; } -void sl_output_send_host_output_state(struct sl_host_output* host) { +void sl_output_calculate_virtual_dimensions(struct sl_host_output* host) { int scale; int virt_physical_width; int virt_physical_height; @@ -410,26 +378,36 @@ void sl_output_send_host_output_state(struct sl_host_output* host) { &virt_height); } + host->scale_factor = scale; + host->virt_width = virt_width; + host->virt_height = virt_height; host->virt_physical_width = virt_physical_width; host->virt_physical_height = virt_physical_height; sl_output_apply_rotation(host->transform, virt_width, virt_height, &host->virt_rotated_width, &host->virt_rotated_height); + host->needs_update = true; +} - // Shift all outputs that are to the right of host to the right if needed. - sl_output_shift_output_x(host, true); - host->virt_y = 0; - wl_output_send_geometry(host->resource, host->virt_x, host->virt_y, - host->virt_physical_width, host->virt_physical_height, - host->subpixel, host->make, host->model, - host->transform); - wl_output_send_mode(host->resource, host->flags | WL_OUTPUT_MODE_CURRENT, - virt_width, virt_height, host->refresh); - if (wl_resource_get_version(host->resource) >= WL_OUTPUT_SCALE_SINCE_VERSION) - wl_output_send_scale(host->resource, scale); - if (wl_resource_get_version(host->resource) >= WL_OUTPUT_DONE_SINCE_VERSION) - wl_output_send_done(host->resource); +// Function which pushes the state of an output to the client. +void sl_output_send_host_output_state(struct sl_host_output* host) { + // Could be more granular, but the current implementation means that if one + // value changes, everything should be impacted. + if (host->needs_update) { + wl_output_send_geometry(host->resource, host->virt_x, 0, + host->virt_physical_width, + host->virt_physical_height, host->subpixel, + host->make, host->model, host->transform); + wl_output_send_mode(host->resource, host->flags | WL_OUTPUT_MODE_CURRENT, + host->virt_width, host->virt_height, host->refresh); + if (wl_resource_get_version(host->resource) >= + WL_OUTPUT_SCALE_SINCE_VERSION) + wl_output_send_scale(host->resource, host->scale_factor); + if (wl_resource_get_version(host->resource) >= WL_OUTPUT_DONE_SINCE_VERSION) + wl_output_send_done(host->resource); + host->needs_update = false; + } } static void sl_output_geometry(void* data, @@ -487,6 +465,7 @@ static void sl_output_mode(void* data, host->width = width; host->height = height; host->refresh = refresh; + host->needs_update = true; } static void sl_output_done(void* data, struct wl_output* output) { @@ -497,6 +476,10 @@ static void sl_output_done(void* data, struct wl_output* output) { if (host->expecting_scale) return; + // Recalculate according to any information that's been modified. + sl_output_calculate_virtual_dimensions(host); + // Shift all outputs that are to the right of host to the right if needed. + sl_output_update_output_x(host->ctx); sl_output_send_host_output_state(host); // Expect scale if aura output exists. @@ -573,7 +556,7 @@ static void sl_destroy_host_output(struct wl_resource* resource) { free(host->make); free(host->model); // Shift all outputs to the right of the deleted output to the left. - sl_output_shift_output_x(host, false); + sl_output_update_output_x(host->ctx); delete host; } @@ -623,10 +606,11 @@ static void sl_bind_host_output(struct wl_client* client, host->aura_output = nullptr; // We assume that first output is internal by default. host->internal = ctx->host_outputs.empty(); + // We'll always need to forward this information. + host->needs_update = true; host->x = 0; host->y = 0; host->virt_x = 0; - host->virt_y = 0; host->logical_x = 0; host->logical_y = 0; host->physical_width = 0; @@ -640,6 +624,8 @@ static void sl_bind_host_output(struct wl_client* client, host->flags = 0; host->width = 1024; host->height = 768; + host->virt_width = 1024; + host->virt_height = 768; host->virt_rotated_width = 0; host->virt_rotated_height = 0; host->logical_width = 1024; diff --git a/sommelier-transform.cc b/sommelier-transform.cc index bab5bc9..464639f 100644 --- a/sommelier-transform.cc +++ b/sommelier-transform.cc @@ -401,7 +401,6 @@ struct sl_host_output* sl_transform_guest_position_to_host_position( // Translate from global to output-relative guest coordinates (*x) -= output->virt_x; - (*y) -= output->virt_y; // Convert to host logical scale sl_transform_guest_to_host(ctx, surface, x, y); @@ -427,7 +426,6 @@ struct sl_host_output* sl_transform_host_position_to_guest_position( // Translate to global guest coordinates (*x) += output->virt_x; - (*y) += output->virt_y; return output; } diff --git a/sommelier-window.cc b/sommelier-window.cc index c7143b3..39640b4 100644 --- a/sommelier-window.cc +++ b/sommelier-window.cc @@ -305,7 +305,6 @@ static void sl_internal_toplevel_configure(struct sl_window* window, window->next_config.values[i++] = output->virt_x + (output->virt_rotated_width - width_in_pixels) / 2; window->next_config.values[i++] = - output->virt_y + (output->virt_rotated_height - height_in_pixels) / 2; } else { window->next_config.values[i++] = diff --git a/sommelier.cc b/sommelier.cc index b2eba17..6d19d60 100644 --- a/sommelier.cc +++ b/sommelier.cc @@ -200,8 +200,7 @@ static void sl_adjust_window_position_for_screen_size( if (output) { window->x = output->virt_x + (output->virt_rotated_width - window->width) / 2; - window->y = - output->virt_y + (output->virt_rotated_height - window->height) / 2; + window->y = (output->virt_rotated_height - window->height) / 2; } else { // Center horizontally/vertically. window->x = ctx->screen->width_in_pixels / 2 - window->width / 2; @@ -3282,6 +3281,9 @@ static void sl_calculate_scale_for_xwayland(struct sl_context* ctx) { ctx->scale = MIN(MAX_SCALE, MAX(MIN_SCALE, scale)); // Scale affects output state. Send updated output state to xwayland. + for (auto output : ctx->host_outputs) + sl_output_calculate_virtual_dimensions(output); + sl_output_update_output_x(ctx); for (auto output : ctx->host_outputs) sl_output_send_host_output_state(output); } diff --git a/sommelier.h b/sommelier.h index 5e2f045..dd1b3de 100644 --- a/sommelier.h +++ b/sommelier.h @@ -242,6 +242,9 @@ struct sl_host_output { struct zxdg_output_v1* zxdg_output; struct zaura_output* aura_output; int internal; + // Whether or not this output has been modified and updated information needs + // to be forwarded. + bool needs_update; // Position in host's global logical space int x; @@ -249,8 +252,9 @@ struct sl_host_output { int physical_width; int physical_height; + // The physical width/height after being scaled by - // sl_output_get_dimensions_original. + // sl_output_get_dimensions_original to adjust the DPI values accordingly. int virt_physical_width; int virt_physical_height; int subpixel; @@ -263,19 +267,41 @@ struct sl_host_output { // https://wayland.app/protocols/wayland#wl_output:event:mode int width; int height; + + // The width/height after being scaled. + int virt_width; + int virt_height; + // The width/height after being scaled by // sl_output_get_dimensions_original and rotated. int virt_rotated_width; int virt_rotated_height; int refresh; + + // The output scale received from the host via wl_output.scale. + // When run without aura-shell and in non-direct-scale mode, + // we forward this scale to clients. int scale_factor; + // The current scale being used which is provided by zaura_output.scale. int current_scale; + // The preferred scale which is provided by zaura_output.scale. On a + // Chromebook, this is the "Default" value in Settings>Device>Display. int preferred_scale; + // The device_scale_factor provided by zaura_output.device_scale_factor. + // Chromebooks come with their own default device scale which is appropriate + // for their screen sizes. int device_scale_factor; int expecting_scale; bool expecting_logical_size; + // xdg_output values. + // Ref: https://wayland.app/protocols/xdg-output-unstable-v1 + int32_t logical_width; + int32_t logical_height; + int32_t logical_x; + int32_t logical_y; + // The scaling factors for direct mode // virt_scale: Used to translate from physical space to virtual space // xdg_scale: Used to translate from virtual space to logical space @@ -293,11 +319,6 @@ struct sl_host_output { double xdg_scale_x; double xdg_scale_y; int virt_x; - int virt_y; - int32_t logical_width; - int32_t logical_height; - int32_t logical_x; - int32_t logical_y; }; MAP_STRUCTS(wl_output, sl_host_output); @@ -505,6 +526,13 @@ struct sl_host_output* sl_infer_output_for_host_position(struct sl_context* ctx, struct sl_host_output* sl_infer_output_for_guest_position( struct sl_context* ctx, int32_t virt_x, int32_t virt_y); +// Generates all the virtual/modified values to be forwarded to the client. +void sl_output_calculate_virtual_dimensions(struct sl_host_output* host); + +// Updates the virt_x of all outputs. +void sl_output_update_output_x(struct sl_context* ctx); + +// Forwards all the available information of an output to the client. void sl_output_send_host_output_state(struct sl_host_output* host); struct sl_global* sl_output_global_create(struct sl_output* output);