Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Work around for multiple displays with respective scales #245

Closed
wants to merge 3 commits into from
Closed

Work around for multiple displays with respective scales #245

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
48e10b9
Work around for multiple displays with respective scales
wsxyeah Aug 26, 2018
8313e60
Extract out logical size update operation into screen.c
wsxyeah Sep 5, 2018
a5e690d
Turn scale factor into float for better precision
wsxyeah Sep 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions app/src/scrcpy.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ static SDL_bool event_loop(void) {
switch (event.window.event) {
case SDL_WINDOWEVENT_EXPOSED:
case SDL_WINDOWEVENT_SIZE_CHANGED:
if (!screen_update_logical_size_if_needed(&screen)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which even is triggered when the window moves from one screen to another? SDL_WINDOW_EVENT_EXPOSED or SDL_WINDOWEVENT_SIZE_CHANGED?

On my computer (on Linux/XFCE), I get both events when I move the window from one screen to another if and only if I enable "put the windows side-by-side when moved to a screen border" (my own translation). However, otherwise, I get no event at all. So my question: is one of these events always triggered on switching screen (with different scales)? (it may be a good compromise if in practice it works, though).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As tested on my devices(an external display without any scale and mbp's builtin retina display), when I moved window from one screen to another, SDL_WINDOWEVENT_SIZE_CHANGED always gets triggered.
And listening SDL_WINDOWEVENT_EXPOSED event seems unnecessary.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC, for rendering at least, SDL_WINDOWEVENT_EXPOSED was necessary when switching fullscreen on some platforms. I'm not sure it's necessary for detecting scaling changes.

return SDL_FALSE;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The computing should be done in screen.c.

Typically, add a set_logical_size() wrapper for this call and this one.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, but I'm stupid, you need it there so that it changes when you moves the window from/to the secondary screen (it still needs be done in screen.c though).

screen_render(&screen);
break;
}
Expand Down
28 changes: 28 additions & 0 deletions app/src/screen.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,33 @@ static void set_window_size(struct screen *screen, struct size new_size) {
}
}

void get_window_scale(SDL_Window *window, float *scale_x, float *scale_y) {
int win_w, win_h;
SDL_GetWindowSize(window, &win_w, &win_h);
int output_w, output_h;
SDL_GL_GetDrawableSize(window, &output_w, &output_h);
*scale_x = (float)output_w / win_w;
*scale_y = (float)output_h / win_h;
}

SDL_bool screen_update_logical_size_if_needed(struct screen *screen) {
float scale_x, scale_y;
get_window_scale(screen->window, &scale_x, &scale_y);
if (scale_x != screen->scale_x || scale_y != screen->scale_y) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comparing float values for equality is fragile.

Since the "update" must also happen on frame size changes, maybe we could just set the logical size whenever SDL_WINDOW_EVENT_EXPOSED or SDL_WINDOWEVENT_SIZE_CHANGED (the one which works) or when the frame size changes? (without checking if the scale actually changed)

What do you think?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that comparing float values for equality is fragile.

Updating logical size when the frame size changes is ok, but updating on SDL_WINDOWEVENT_SIZE_CHANGED is unnecessary.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, then we can keep the last width and height passed to SDL_RenderSetLogicalSize() (instead of the scale), and call it only when the parameters have changed.

int logical_width, logical_height;
SDL_RenderGetLogicalSize(screen->renderer, &logical_width, &logical_height);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'm nitpicking, but I'm not fan of computing the new value based on the previous scaled rounded value. It should be computed from the non-scaled value IMO.

logical_width *= scale_x / screen->scale_x;
logical_height *= scale_y / screen->scale_y;
screen->scale_x = scale_x;
screen->scale_y = scale_y;
if (SDL_RenderSetLogicalSize(screen->renderer, logical_width, logical_height)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logical size is rewritten on frame size changes (in prepare_for_frame), so it should take the scale into account here too.

LOGE("Could not set renderer logical size: %s", SDL_GetError());
return SDL_FALSE;
}
}
return SDL_TRUE;
}

// get the preferred display bounds (i.e. the screen bounds with some margins)
static SDL_bool get_preferred_display_bounds(struct size *bounds) {
SDL_Rect rect;
Expand Down Expand Up @@ -168,6 +195,7 @@ SDL_bool screen_init_rendering(struct screen *screen, const char *device_name, s
screen_destroy(screen);
return SDL_FALSE;
}
get_window_scale(screen->window, &screen->scale_x, &screen->scale_y);

SDL_Surface *icon = read_xpm(icon_xpm);
if (!icon) {
Expand Down
6 changes: 6 additions & 0 deletions app/src/screen.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ struct screen {
SDL_Renderer *renderer;
SDL_Texture *texture;
struct size frame_size;
float scale_x;
float scale_y;
//used only in fullscreen mode to know the windowed window size
struct size windowed_window_size;
SDL_bool has_frame;
Expand Down Expand Up @@ -66,4 +68,8 @@ void screen_resize_to_fit(struct screen *screen);
// resize window to 1:1 (pixel-perfect)
void screen_resize_to_pixel_perfect(struct screen *screen);

void get_window_scale(SDL_Window *window, float *scale_x, float *scale_y);

SDL_bool screen_update_logical_size_if_needed(struct screen *screen);

#endif