From e40532a3761da8b3aef484f1d435ef5f50d94574 Mon Sep 17 00:00:00 2001 From: Romain Vimont Date: Fri, 17 Apr 2020 18:44:24 +0200 Subject: [PATCH] Manually position and scale the content Position and scale the content "manually" instead of relying on the renderer "logical size". This avoids possible rounding differences between the computed window size and the content size, causing one row or column of black pixels on the bottom or on the right. This also avoids HiDPI scale issues, by computing the scaling manually. This will also enable to draw items at their expected size on the screen (unscaled). Fixes #15 --- app/src/input_manager.c | 61 ++++++++------------ app/src/scrcpy.c | 2 +- app/src/screen.c | 119 ++++++++++++++++++++++++++++------------ app/src/screen.h | 20 ++++++- 4 files changed, 125 insertions(+), 77 deletions(-) diff --git a/app/src/input_manager.c b/app/src/input_manager.c index 14ee7e400b..e8c8d68ebf 100644 --- a/app/src/input_manager.c +++ b/app/src/input_manager.c @@ -7,33 +7,6 @@ #include "util/lock.h" #include "util/log.h" -// Convert window coordinates (as provided by SDL_GetMouseState() to renderer -// coordinates (as provided in SDL mouse events) -// -// See my question: -// -static void -convert_to_renderer_coordinates(SDL_Renderer *renderer, int *x, int *y) { - SDL_Rect viewport; - float scale_x, scale_y; - SDL_RenderGetViewport(renderer, &viewport); - SDL_RenderGetScale(renderer, &scale_x, &scale_y); - *x = (int) (*x / scale_x) - viewport.x; - *y = (int) (*y / scale_y) - viewport.y; -} - -static struct point -get_mouse_point(struct screen *screen) { - int x; - int y; - SDL_GetMouseState(&x, &y); - convert_to_renderer_coordinates(screen->renderer, &x, &y); - return (struct point) { - .x = x, - .y = y, - }; -} - static const int ACTION_DOWN = 1; static const int ACTION_UP = 1 << 1; @@ -487,11 +460,17 @@ convert_touch(const SDL_TouchFingerEvent *from, struct screen *screen, to->inject_touch_event.pointer_id = from->fingerId; to->inject_touch_event.position.screen_size = screen->frame_size; + + int ww; + int wh; + SDL_GL_GetDrawableSize(screen->window, &ww, &wh); + // SDL touch event coordinates are normalized in the range [0; 1] - float x = from->x * screen->content_size.width; - float y = from->y * screen->content_size.height; + int32_t x = from->x * ww; + int32_t y = from->y * wh; to->inject_touch_event.position.point = screen_convert_to_frame_coords(screen, x, y); + to->inject_touch_event.pressure = from->pressure; to->inject_touch_event.buttons = 0; return true; @@ -508,13 +487,6 @@ input_manager_process_touch(struct input_manager *im, } } -static bool -is_outside_device_screen(struct input_manager *im, int x, int y) -{ - return x < 0 || x >= im->screen->content_size.width || - y < 0 || y >= im->screen->content_size.height; -} - static bool convert_mouse_button(const SDL_MouseButtonEvent *from, struct screen *screen, struct control_msg *to) { @@ -552,10 +524,15 @@ input_manager_process_mouse_button(struct input_manager *im, action_home(im->controller, ACTION_DOWN | ACTION_UP); return; } + // double-click on black borders resize to fit the device screen if (event->button == SDL_BUTTON_LEFT && event->clicks == 2) { - bool outside = - is_outside_device_screen(im, event->x, event->y); + int32_t x = event->x; + int32_t y = event->y; + screen_hidpi_scale_coords(im->screen, &x, &y); + SDL_Rect *r = &im->screen->rect; + bool outside = x < r->x || x >= r->x + r->w + || y < r->y || y >= r->y + r->h; if (outside) { screen_resize_to_fit(im->screen); return; @@ -579,9 +556,15 @@ input_manager_process_mouse_button(struct input_manager *im, static bool convert_mouse_wheel(const SDL_MouseWheelEvent *from, struct screen *screen, struct control_msg *to) { + + // mouse_x and mouse_y are expressed in pixels relative to the window + int mouse_x; + int mouse_y; + SDL_GetMouseState(&mouse_x, &mouse_y); + struct position position = { .screen_size = screen->frame_size, - .point = get_mouse_point(screen), + .point = screen_convert_to_frame_coords(screen, mouse_x, mouse_y), }; to->type = CONTROL_MSG_TYPE_INJECT_SCROLL_EVENT; diff --git a/app/src/scrcpy.c b/app/src/scrcpy.c index e56ad84de2..7f8d3c7552 100644 --- a/app/src/scrcpy.c +++ b/app/src/scrcpy.c @@ -136,7 +136,7 @@ event_watcher(void *data, SDL_Event *event) { && event->window.event == SDL_WINDOWEVENT_RESIZED) { // In practice, it seems to always be called from the same thread in // that specific case. Anyway, it's just a workaround. - screen_render(&screen); + screen_render(&screen, true); } return 0; } diff --git a/app/src/screen.c b/app/src/screen.c index 566b2d992a..967cf5d7d5 100644 --- a/app/src/screen.c +++ b/app/src/screen.c @@ -156,6 +156,43 @@ get_initial_optimal_size(struct size content_size, uint16_t req_width, return window_size; } +static void +screen_update_content_rect(struct screen *screen) { + int dw; + int dh; + SDL_GL_GetDrawableSize(screen->window, &dw, &dh); + + struct size content_size = screen->content_size; + // The drawable size is the window size * the HiDPI scale + struct size drawable_size = {dw, dh}; + + SDL_Rect *rect = &screen->rect; + + if (is_optimal_size(drawable_size, content_size)) { + rect->x = 0; + rect->y = 0; + rect->w = drawable_size.width; + rect->h = drawable_size.height; + return; + } + + bool keep_width = content_size.width * drawable_size.height + > content_size.height * drawable_size.width; + if (keep_width) { + rect->x = 0; + rect->w = drawable_size.width; + rect->h = drawable_size.width * content_size.height + / content_size.width; + rect->y = (drawable_size.height - rect->h) / 2; + } else { + rect->y = 0; + rect->h = drawable_size.height; + rect->w = drawable_size.height * content_size.width + / content_size.height; + rect->x = (drawable_size.width - rect->w) / 2; + } +} + void screen_init(struct screen *screen) { *screen = (struct screen) SCREEN_INITIALIZER; @@ -245,13 +282,6 @@ screen_init_rendering(struct screen *screen, const char *window_title, const char *renderer_name = r ? NULL : renderer_info.name; LOGI("Renderer: %s", renderer_name ? renderer_name : "(unknown)"); - if (SDL_RenderSetLogicalSize(screen->renderer, content_size.width, - content_size.height)) { - LOGE("Could not set renderer logical size: %s", SDL_GetError()); - screen_destroy(screen); - return false; - } - // starts with "opengl" screen->use_opengl = renderer_name && !strncmp(renderer_name, "opengl", 6); if (screen->use_opengl) { @@ -295,6 +325,8 @@ screen_init_rendering(struct screen *screen, const char *window_title, return false; } + screen_update_content_rect(screen); + return true; } @@ -365,18 +397,12 @@ screen_set_rotation(struct screen *screen, unsigned rotation) { struct size new_content_size = get_rotated_size(screen->frame_size, rotation); - if (SDL_RenderSetLogicalSize(screen->renderer, - new_content_size.width, - new_content_size.height)) { - LOGE("Could not set renderer logical size: %s", SDL_GetError()); - return; - } - set_content_size(screen, new_content_size); + screen->rotation = rotation; LOGI("Display rotation set to %u", rotation); - screen_render(screen); + screen_render(screen, true); } // recreate the texture and resize the window if the frame size has changed @@ -384,21 +410,17 @@ static bool prepare_for_frame(struct screen *screen, struct size new_frame_size) { if (screen->frame_size.width != new_frame_size.width || screen->frame_size.height != new_frame_size.height) { - struct size new_content_size = - get_rotated_size(new_frame_size, screen->rotation); - if (SDL_RenderSetLogicalSize(screen->renderer, - new_content_size.width, - new_content_size.height)) { - LOGE("Could not set renderer logical size: %s", SDL_GetError()); - return false; - } - // frame dimension changed, destroy texture SDL_DestroyTexture(screen->texture); - set_content_size(screen, new_content_size); screen->frame_size = new_frame_size; + struct size new_content_size = + get_rotated_size(new_frame_size, screen->rotation); + set_content_size(screen, new_content_size); + + screen_update_content_rect(screen); + LOGI("New texture: %" PRIu16 "x%" PRIu16, screen->frame_size.width, screen->frame_size.height); screen->texture = create_texture(screen); @@ -439,15 +461,19 @@ screen_update_frame(struct screen *screen, struct video_buffer *vb) { update_texture(screen, frame); mutex_unlock(vb->mutex); - screen_render(screen); + screen_render(screen, false); return true; } void -screen_render(struct screen *screen) { +screen_render(struct screen *screen, bool update_content_rect) { + if (update_content_rect) { + screen_update_content_rect(screen); + } + SDL_RenderClear(screen->renderer); if (screen->rotation == 0) { - SDL_RenderCopy(screen->renderer, screen->texture, NULL, NULL); + SDL_RenderCopy(screen->renderer, screen->texture, NULL, &screen->rect); } else { // rotation in RenderCopyEx() is clockwise, while screen->rotation is // counterclockwise (to be consistent with --lock-video-orientation) @@ -457,12 +483,14 @@ screen_render(struct screen *screen) { SDL_Rect *dstrect = NULL; SDL_Rect rect; if (screen->rotation & 1) { - struct size size = screen->content_size; - rect.x = (size.width - size.height) / 2; - rect.y = (size.height - size.width) / 2; - rect.w = size.height; - rect.h = size.width; + rect.x = screen->rect.x + (screen->rect.w - screen->rect.h) / 2; + rect.y = screen->rect.y + (screen->rect.h - screen->rect.w) / 2; + rect.w = screen->rect.h; + rect.h = screen->rect.w; dstrect = ▭ + } else { + assert(screen->rotation == 2); + dstrect = &screen->rect; } SDL_RenderCopyEx(screen->renderer, screen->texture, NULL, dstrect, @@ -485,7 +513,7 @@ screen_switch_fullscreen(struct screen *screen) { } LOGD("Switched to %s mode", screen->fullscreen ? "fullscreen" : "windowed"); - screen_render(screen); + screen_render(screen, true); } void @@ -523,10 +551,10 @@ screen_handle_window_event(struct screen *screen, const SDL_WindowEvent *event) { switch (event->event) { case SDL_WINDOWEVENT_EXPOSED: - screen_render(screen); + screen_render(screen, true); break; case SDL_WINDOWEVENT_SIZE_CHANGED: - screen_render(screen); + screen_render(screen, true); break; case SDL_WINDOWEVENT_MAXIMIZED: screen->maximized = true; @@ -552,6 +580,13 @@ screen_convert_to_frame_coords(struct screen *screen, int32_t x, int32_t y) { int32_t w = screen->content_size.width; int32_t h = screen->content_size.height; + + screen_hidpi_scale_coords(screen, &x, &y); + + x = (int64_t) (x - screen->rect.x) * w / screen->rect.w; + y = (int64_t) (y - screen->rect.y) * h / screen->rect.h; + + // rotate struct point result; switch (rotation) { case 0: @@ -574,3 +609,15 @@ screen_convert_to_frame_coords(struct screen *screen, int32_t x, int32_t y) { } return result; } + +void +screen_hidpi_scale_coords(struct screen *screen, int32_t *x, int32_t *y) { + // take the HiDPI scaling (dw/ww and dh/wh) into account + int ww, wh, dw, dh; + SDL_GetWindowSize(screen->window, &ww, &wh); + SDL_GL_GetDrawableSize(screen->window, &dw, &dh); + + // scale for HiDPI (64 bits for intermediate multiplications) + *x = (int64_t) *x * dw / ww; + *y = (int64_t) *y * dh / wh; +} diff --git a/app/src/screen.h b/app/src/screen.h index 7703b5a230..aa6218f7d2 100644 --- a/app/src/screen.h +++ b/app/src/screen.h @@ -29,6 +29,8 @@ struct screen { // client rotation: 0, 1, 2 or 3 (x90 degrees counterclockwise) unsigned rotation; + // rectangle of the content (excluding black borders) + struct SDL_Rect rect; bool has_frame; bool fullscreen; bool maximized; @@ -56,6 +58,12 @@ struct screen { .height = 0, \ }, \ .rotation = 0, \ + .rect = { \ + .x = 0, \ + .y = 0, \ + .w = 0, \ + .h = 0, \ + }, \ .has_frame = false, \ .fullscreen = false, \ .maximized = false, \ @@ -89,8 +97,11 @@ bool screen_update_frame(struct screen *screen, struct video_buffer *vb); // render the texture to the renderer +// +// Set the update_content_rect flag if the window or content size may have +// changed, so that the content rectangle is recomputed void -screen_render(struct screen *screen); +screen_render(struct screen *screen, bool update_content_rect); // switch the fullscreen mode void @@ -117,4 +128,11 @@ screen_handle_window_event(struct screen *screen, const SDL_WindowEvent *event); struct point screen_convert_to_frame_coords(struct screen *screen, int32_t x, int32_t y); +// Convert coordinates from window to drawable. +// Events are expressed in window coordinates, but content is expressed in +// drawable coordinates. They are the same if HiDPI scaling is 1, but differ +// otherwise. +void +screen_hidpi_scale_coords(struct screen *screen, int32_t *x, int32_t *y); + #endif