Add adaptive virtual display that adjusts to window size.#6705
Add adaptive virtual display that adjusts to window size.#6705TinuraD wants to merge 2 commits intoGenymobile:masterfrom
Conversation
|
P.S. I noticed that someone implemented the same feature in [#6351]. Since it hasn't been merged yet, I decided to reopen my PR too. I hope one of these will be added to scrcpy soon. |
LHLaurini
left a comment
There was a problem hiding this comment.
Thank you for your PR!
I left you some comments. I personally find your implementation cleaner than the other one, although that one works a bit better at the moment.
Your implementation makes the video look a bit blurry, probably because the server does some rounding to the dimensions.
| } catch (Exception e) { | ||
| Ln.w("Virtual display resize failed, fallback to recreate", e); | ||
| displaySizeMonitor.stopAndRelease(); | ||
| virtualDisplay.release(); | ||
| virtualDisplay = null; | ||
| } |
There was a problem hiding this comment.
The whole point of resize is to "to allow applications using virtual displays to adapt to changing conditions without having to tear down and recreate the display" (from the docs). Is this really necessary?
IMO unless this happens in some devices, you should just show a warning and keep going
app/src/screen.c
Outdated
| if (size.width == 0 || size.height == 0) { | ||
| return; | ||
| } | ||
| if (size.width < 64 || size.height < 64) { | ||
| return; | ||
| } |
app/src/screen.c
Outdated
| @@ -811,6 +934,7 @@ sc_screen_handle_event(struct sc_screen *screen, const SDL_Event *event) { | |||
| return true; | |||
| } | |||
| case SC_EVENT_NEW_FRAME: { | |||
| sc_screen_maybe_send_vd_resize(screen); | |||
There was a problem hiding this comment.
You probably only need one sc_screen_maybe_send_vd_resize here. I'd keep the one in SC_EVENT_NEW_FRAME.
There was a problem hiding this comment.
Changed. Kept the one you did recommend.
| { | ||
| .longopt_id = OPT_ADAPTIVE_NEW_DISPLAY, | ||
| .longopt = "adaptive-new-display", | ||
| .argdesc = "[<width>x<height>][/<dpi>]", |
There was a problem hiding this comment.
This dpi value is almost immediately ignored in favor of the one from adaptive-scale
There was a problem hiding this comment.
Since scaling can basically handle dpi I thought it wasn’t needed. But it makes sense to add a known dpi value. So I fixed it.
|
|
||
| static uint16_t | ||
| get_window_dpi(const struct sc_screen *screen) { | ||
| if (screen->adaptive_new_display) { |
There was a problem hiding this comment.
Maybe you meant to check vd_scale here? This is always true.
app/src/screen.c
Outdated
| } | ||
| screen->vd_resize_size = size; | ||
| screen->vd_resize_pending = true; | ||
| screen->vd_resize_deadline_ms = SDL_GetTicks() + 250; |
There was a problem hiding this comment.
Use a constant for this value
| struct sc_size window_size = | ||
| get_initial_optimal_size(screen->content_size, screen->req.width, | ||
| screen->req.height); | ||
| struct sc_size window_size; | ||
| if (screen->adaptive_new_display | ||
| && !screen->req.width && !screen->req.height) { | ||
| struct sc_size bounds; | ||
| if (get_preferred_display_bounds(&bounds)) { | ||
| window_size = get_optimal_size(bounds, screen->content_size, true); | ||
| } else { | ||
| window_size = get_initial_optimal_size(screen->content_size, | ||
| screen->req.width, | ||
| screen->req.height); | ||
| } | ||
| } else { | ||
| window_size = get_initial_optimal_size(screen->content_size, | ||
| screen->req.width, | ||
| screen->req.height); | ||
| } |
There was a problem hiding this comment.
Could you explain the goal of this change?
There was a problem hiding this comment.
Basically, the resizing function isn’t tied to the initial startup, as it’s mostly based on normal new-display behavior. This allows the virtual display to have adaptive window size on initial launch following px to dpi conversion or using custom arguments.
| if (screen->new_display) { | ||
| // For new-display, keep the window size controlled by the user. | ||
| // Do not auto-resize the window to the content size to avoid | ||
| // feedback loops with virtual display resizing. | ||
| screen->content_size = new_content_size; | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
Is this really a problem? Unless I'm mistaken, shouldn't the check in lines 108-110 avoid that?
There was a problem hiding this comment.
This is to avoid issues when Android's rotation causes the virtual display to resize. Some Android apps, like video players, change the rotation while the app is in use, such as when opening a video. Even though the window appears the same, it still makes a very small size change as i noticed which can create loops.
| if (pendingRelaunchOnResize && lastStartedAppPackage != null) { | ||
| pendingRelaunchOnResize = false; | ||
| Ln.i("Relaunching app \"" + lastStartedAppPackage + "\" on resized display " + virtualDisplayId + "..."); | ||
| Device.startApp(lastStartedAppPackage, virtualDisplayId, lastStartedAppForceStop); | ||
| } |
There was a problem hiding this comment.
You shouldn't relaunch the app if you were able to resize the display successfully, as that causes some apps to restart (particularly PWAs)
…plication | dpi value arg added | Avoid relaunching on successful resizes
|
Hey, thanks for reviewing the PR. I’ve checked your comments and updated the required files. |
I noticed some developers have made GUI or companion apps using scrcpy, so being able to open apps in resizable windows would be cool. SO I’ve just added an adaptive virtual display option.
--new-display, you can now also use--adaptive-new-displaytoo, which provides an adaptive virtual display that resizes according to your current window.--adaptive-scale, which allows you to change the scale (default is 1). [Eg: 0.5, 2.0]--new-displaystill works as before.adaptivevd.mp4
adaptiveapp.mp4