avnc icon indicating copy to clipboard operation
avnc copied to clipboard

Feature Request: setting default zoom level

Open PureIncompetence opened this issue 2 years ago • 9 comments

Right now the default zoom-state on an opened connection in portrait mode seems to be determined by the width (a.k.a. the height in landscape mode) of the phone's screen, so it uses only ~ 50% of the phone's screen in portrait mode. Therefore everytime after connecting I need to zoom in at least until the height of the remote screen matches the height of the phone screen to be able to see something on the remote screen. (I'm gonna attach a screen record to show it.)

https://user-images.githubusercontent.com/66217283/167184119-d2911206-7eda-4afe-8cf2-d88ae18793de.mp4

In my opinion it'd make sense to either be able to set a default zoom level (which might need to be different in landscape mode than it would be in portrait mode) or - which would probably be easier and more convenient to most users - to automatically align the default zoom-level for portrait mode with the screen height of portrait mode.

PureIncompetence avatar May 06 '22 17:05 PureIncompetence

Sorry for late response @PureIncompetence.

Instead of having a default zoom preference, would "remembering" the last zoom level of a connection solve this issue? This provides a more generic solution, without any effort from user's side. And users don't have to think about which particular zoom value will fill the screen.

@user8446, IMO, Remembering the zoom level also removes the need to allow Minimum Zoom above 100%, as we were discussing in #66.

gujjwal00 avatar Jul 18 '22 18:07 gujjwal00

Great idea... remembering the last zoom level would solve both issues

user8446 avatar Jul 18 '22 20:07 user8446

While I personally would prefer being able to set a default zoom level I'm also okay with AVNC simply remembering the last used level. Since you're worried for the simplicity of the settings pages this absolutely makes sense to me. However, for this function to be helpful, it needs to store two different values - one for landscape and one for portrait.

PureIncompetence avatar Jul 28 '22 08:07 PureIncompetence

I don't mind adding a preference, but I beleive in this case remembering last level is more simple and effective. Single preference might not fit servers with different resolution/aspect-ratio. It also forces us to link different zoom levels (min, max, defaults) together so they can't be set to contradictory values.

I mainly worry about the Input Settings. For AVNC, I have worked hardest on input handling (followed by layout issues like zooming and panning), and I am kind of proud of what AVNC can do in this area. I mean simplicity in terms of being able to easily grasp & navigate, not in terms of pref count, although having fewer prefs does help. So I like to have fewer prefs, but not at the cost of user personalization. Furthermore, this is just a goal, not something set in stone. Its not like users fiddle with settings everyday 😀.

Yes, it would be per-orientation, but I am thinking of having a 'Separate zoom level for each orientation' preference to allow current behavior of single zoom level.

gujjwal00 avatar Jul 28 '22 15:07 gujjwal00

It also forces us to link different zoom levels (min, max, defaults) together so they can't be set to contradictory values.

That indeed sounds like a good reason against it which I wasn't aware of as I'm not coding myself.

Single preference might not fit servers with different resolution/aspect-ratio.

That might be true too. The concrete problem I tried to show with my video was the fact that when trying to remotely control a system with a 16:9 screen while on a mobile phone in portrait mode AVNC never uses the whole screen size unless zoomed in manually. To solve that specific problem it would be enough to simply adjust the zoom level to the height of the phone's screen, but obviously your approach (storing the latest zoom-level on a per-server-basis) fits much more use cases than just mine.

For AVNC, I have worked hardest on input handling (followed by layout issues like zooming and panning), and I am kind of proud of what AVNC can do in this area.

And you should be. Considering the other FOSS-VNC-Clients (and even RealVNC-Viewer - afaik the top dog of the closed source ones) I used so far AVNC to me is the most complete / usable one.

Yes, it would be per-orientation, but I am thinking of having a 'Separate zoom level for each orientation' preference to allow current behavior of single zoom level.

I'm not sure I got this right. Are you proposing a seperate zoom per orientation setting within the respective host settings or a general one within the main settings? I'm asking because the latter would contradict your above argument (seperate hosts might need seperate zoom levels).

PureIncompetence avatar Jul 31 '22 13:07 PureIncompetence

The concrete problem I tried to show with my video was the fact that when trying to remotely control a system with a 16:9 screen while on a mobile phone in portrait mode AVNC never uses the whole screen size unless zoomed in manually. To solve that specific problem it would be enough to simply adjust the zoom level to the height of the phone's screen, but obviously your approach (storing the latest zoom-level on a per-server-basis) fits much more use cases than just mine.

I think I got sidetracked a bit, and missed your original suggestion about handling portrait mode automatically. AVNC can indeed optimize for portrait mode without any user intervention. It already does many other optimizations (see base scale in FrameState).

Please test the following APK: app-debug.zip

It will automatically resize to 90% of available vertical space in portrait mode. Let me know if this works as you want.

=> 90% is just a limit, and can be changed/removed.

gujjwal00 avatar Aug 01 '22 12:08 gujjwal00

I did test your apk and it does work in a way I expected it to. Thank you.

Regarding the 90%-limit: If possible it imho would make sense to base that height on either (in fullscreen mode) the complete availible display height or (if navbar + notification bar are shown) on the availible space between them.

Your proposal (to save / restore the latest zoom level) does sound promising nonetheless, as would certainly solve more use cases than my approach, which only focusses on the portrait mode zoom level.

PureIncompetence avatar Aug 03 '22 06:08 PureIncompetence

Regarding the 90%-limit: If possible it imho would make sense to base that height on either (in fullscreen mode) the complete availible display height or (if navbar + notification bar are shown) on the availible space between them.

It already works like that.

Your proposal (to save / restore the latest zoom level) does sound promising nonetheless, as would certainly solve more use cases than my approach, which only focusses on the portrait mode zoom level

Yes, but now we don't have to remember it per-orientation. Because base scale is different for each orientation, single zoom level works optimally in both orientations, and it really simplifies the implementation.

And just a thought, but if optimizing for portrait mode solves the issue, is remembering zoom level really required anymore? Because I believe @user8446 was having the same issue as yours where initial zoom level was insufficient.

gujjwal00 avatar Aug 04 '22 12:08 gujjwal00

It already works like that.

Either I misunderstand you, or it doesn't work that way - at least on my device. When using the apk you linked above on my device, the default zoom level seems to be 90% of the availible height between navbar and notification bar if they are shown and 90% of the complete availible screen height when in fullscreen mode. With the sentence you quoted I meant to say that imo it would make more sense to use 100% instead of only 90% of the space availible at any given time.

And just a thought, but if optimizing for portrait mode solves the issue, is remembering zoom level really required anymore?

Not for me. I'm perfectly okay with a sane default level for each orientation.

PureIncompetence avatar Aug 04 '22 14:08 PureIncompetence

I have thought a lot about this issue, and tried multiple solutions, but none of them seemed to properly satisfy everyone (including me).

  • Remembering previous zoom backfires sometimes. If I was zoomed-in on something before closing the session, next time the zoom is too big or too small, and I have to immediately adjust it.
  • Automatically increasing the frame size in portrait mode is also problematic. Even if using 90% height, the frame is slightly pixelated/blurry. And only about 20% of remote screen is visible for me (phone is 2400x1080, remote is 1920x1080), which means I have to zoom-out or constantly pan around.
  • Having a single global default zoom is unlikely to be fit for different servers with different resolutions.
  • I tried having having a default zoom field in advanced server options, but the user experience is not optimal because it usually takes multiple attempts to find the correct default. It also turned out to be surprisingly complex to handle per-orientation default zoom with this approach.

But I think my latest attempt will solve all issues:

  1. Extend the toolbar drawer to provide an option to save current zoom. 'Reset zoom' button will be replaced by a 'Zoom options' button. When clicked, it will show additional options to reset and save zoom. 'Reset zoom' will work as before and set zoom level to 100%. 'Save zoom' will save current zoom to this server, and it will be used as default for this server.

    screenshot3

  2. Add an option named Separate zoom for each orientation to 'Settings => Viewer => Zoom'. When enabled, AVNC will use separate zoom levels for each orientation, and zooming in one orientation won't affect zoom level in other orientation. Currently, enabling it will also cause the frame to be automatically enlarged to about 80% of the height, but I am on the edge about this particular behavior.

  3. Just as a convenience, and to somewhat mimic the old functionality, long-pressing the 'Zoom options' button will also reset the zoom.

Test APK: app-debug.zip

There are still some polishing needed (like providing feedback when saving zoom level), but please test it out and let me know if you have any feedback.

gujjwal00 avatar Oct 19 '22 16:10 gujjwal00

@gujjwal00 I've been testing, nice work here!!

I think the zoom save feature is spot on! Each user can save their own default in each orientation. Easy.

As for the 80% default height when on, my opinion is I don't think it's needed, who knows what each user wants. I think just a simple toggle to either save or not save would do the trick to make it simple for the users.

user8446 avatar Oct 21 '22 18:10 user8446

Thanks for testing!

As for the 80% default height when on, my opinion is I don't think it's needed, who knows what each user wants.

Yeah, I agree. Because setting default zoom is so trivial now, I don't think we need to be extra clever here.

I think just a simple toggle to either save or not save would do the trick to make it simple for the users.

Can you please explain a bit more here? Are you talking about another option in Settings?

gujjwal00 avatar Oct 22 '22 08:10 gujjwal00

Can you please explain a bit more here? Are you talking about another option in Settings?

Thinking about this more, Separate zoom for each orientation would probably be best in the toolbar drawer zoom options to group zoom settings together.

Also I'm not sure if it was intended or not but toggling Separate zoom for each orientation off turns off the ability to save altogether.

user8446 avatar Oct 22 '22 23:10 user8446

Thinking about this more, Separate zoom for each orientation would probably be best in the toolbar drawer zoom options to group zoom settings together.

The reason here is that Separate zoom for each orientation works globally, and is not specific to a particular server. So it resides with other global zoom settings like min/max. 'Save zoom' on the other hand is per-server, it only changes default zoom for currently connected server. Both of these features work independently of each other.

Also I'm not sure if it was intended or not but toggling Separate zoom for each orientation off turns off the ability to save altogether.

I am not able to reproduce this. I can save the zoom irrespective of whether Separate zoom for each orientation is on/off. One thing to note here is that 'Save zoom' is available only for saved servers. It won't be available for connections started using top urlbar or discovered servers.

gujjwal00 avatar Oct 23 '22 01:10 gujjwal00

I tested again and it does work exactly how you described. Thank you for the explanation!

user8446 avatar Oct 23 '22 18:10 user8446

Implemented: https://github.com/gujjwal00/avnc/commit/c715fdfec6ce43450e740cfc885e3f51b74a1494 It will be available in next release.

gujjwal00 avatar Oct 26 '22 14:10 gujjwal00

Looking forward to it, thank you!

user8446 avatar Oct 26 '22 23:10 user8446