Repository navigation
feat: pass the exact display scale to FreeRDP - #960
Conversation
|
So if I understand this correctly, if I set a scale of 158, this will scale the desktop accordingly, but RAILS will still be scaled to 140? That seems unintuitive. Maybe we should just have a seperate scale for desktop and RAILS? |
|
Good question, and the README wording is what made it ambiguous. Sorry about that.
I measured this before writing the patch. Winver under RemoteApp, with
Linear fits on both axes are within 2 px across the three points, with a fixed ~31 px offset on height that is window chrome. So at The old I will clarify the README. Does this address your concern, or would you still prefer separate user control of |
|
I think a seperate config option for the desktop scale defaulting to the value of RDP_SCALE would be preferred, with RDP_SCALE controlling RemoteApp scaling. Also, please do not copy-paste the answer of the LLM as the response to my question. As per our policy, we accept AI-generated code, but this is just bad etiquette and doesn't really support your claim of having reviewed the changes by hand. |
|
Hey, I thought more about this PR work and #825, which inspired part of it, and I believe something got lost in the transit. There is no reason why I should not include automatic detection of the primary monitor scale (on Cinnamon/X11) from #825 as part of this PR, since it completes the scaling work already here. I do not have a multi-monitor setup, so I cannot reproduce nor properly test those issues and will leave that part of #825 for someone else. I will add the separate full-desktop scale option per your request. Also; I have decided to expand this PR to include automatic scale detection. Of course, manual scale settings will always override automatic detection. Is that okay with you? PS: Sorry about earlier posts, didn't mean any disrespect. |
|
Sure, automatic scale detection sounds super useful! Also, multi-monitor scaling is a non-trivial issue and FreeRDPs multi-monitor capabilities are dubious anyways, so focusing on single-monitor scale detection seems like the right choice |
1ff14dd to
3c137bc
Compare
|
@oskardotglobal Pushed. |
|
@oskardotglobal Hey, just checking in. Did you had a chance to look at the latest changes? |
|
Hi, I still need to find some time to review this properly. Will get back to you |
oskardotglobal
left a comment
There was a problem hiding this comment.
After giving this a proper review: this is not something I will merge as-is anytime soon. The long inline awk code is iff and for fact it only supports Cinnamon on X11 it's implemented way and documented way too broadly; until this supports more setups this won't be very useful to many people. Also, the AI-generated prose in the documentation & comments is poorly phrased.
|
@oskardotglobal It is hard for me to hit a moving target. I said Cinnamon/X11 up front and you said "sure". So that part isn't new. Also, “poorly phrased” is not actionable, it's “way too broadly”. Back to work. I will do better with README and Code Documentation next time. I realize that it is too long, to say the least. No need to point where and what's wrong with the current docs now; they're getting rewritten anyway. Only I can also support GNOME/Wayland through Mutter, which exposes the same monitor data as Cinnamon's Muffin. I can test GNOME/Wayland locally myself, but a second test from someone who uses GNOME as their default DE would be better. Is that enough, or which other environments are required? I have a pretty good idea how to make the Please let me know. Thanks. |
|
Sorry, I missed the "Cinnamon/X11" part of your original comment... What I meant by "implemented too broadly" is that for example, for something implemented currently only for Cinnamon it shouldn't be the default behaviour; the default behaviour should be to configure the scale numerically and then the user can opt-in to autoscaling if they're on a supported DE.
|
|
Understood on all four. I will make the default numeric 100 and keep I will simplify I tested GNOME/Wayland, and Mutter-based detection is feasible. Do you want it in this PR or in a follow-up? I will rewrite the docs to match the final behaviour. |
|
Put GNOME in a follow-up, take your time. |
|
@oskardotglobal sorry for the late reply, had some health issues and also switched to a new laptop. I'm on KDE/wayland now, not Cinnamon/x11 anymore, so the auto detection I added doesn't even work for me. would it be ok to go back to the original scope and drop auto from this PR? just the numeric scaling; so exact value goes to scale-desktop, rounded one to scale-device, and RDP_DESKTOP_SCALE stays. that also gets rid of the detection code and the long awk code. I'll still clean up waFixScale and rewrite the readme and comments like you asked. that also means no gnome follow-up from me, sorry about that. auto detection can be its own PR later if someone wants it. |
|
Yes, sure. |
WinApps rounded RDP_SCALE to 100, 140 or 180 because FreeRDP's /scale only accepts those values. /scale-desktop takes any integer from 100 to 500, so the exact value now goes there and 125 stays 125. The device scale, used by MS Store (UWP) apps, still only takes 100, 140 or 180. WinApps rounds it to the nearest one and passes it as /scale-device. RDP_SCALE now sets RemoteApp scaling. The new RDP_DESKTOP_SCALE sets the scale for 'winapps windows' and defaults to RDP_SCALE. A non-integer value stops the launch with an error. Values outside 100-500 are set to the nearest limit with a warning. The warning no longer pops up a notification on every launch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Tony Dursun <oraturk75@gmail.com>
3c137bc to
a871814
Compare
|
@oskardotglobal pushed. auto is gone, it's just the numeric scaling now. I also cleaned up waFixScale and rewrote the readme part, and rebased on current main. updated the description too. |
|
thanks for the review and your patience, it's my first merged PR :) |
What this changes
RDP_SCALEwas rounded to 100, 140, or 180 because WinApps assumed FreeRDP only supported those values. FreeRDP takes scaling values in three different ways./scale-desktop: Can take any integer between 100-500, and this is where full-desktop and RemoteApps scaling is set./scale-device: Accepts only 100, 140, and 180. This is the scaling factor for MS Store (UWP) apps./scale: This is only a convenience shortcut. It takes one value and assigns it to the above two. It accepts only 100, 140, and 180. This is what WinApps is currently using. So a requested scale of 125 gets rounded to 140 by WinApps before it goes to FreeRDP's/scaleoption.This PR sends the exact value through
/scale-desktop, rounds it to the nearest supported value, and sends that to/scale-deviceseparately. I set both, because sending only/scale-desktopwould change the device scale existing users get today.As requested,
RDP_SCALEnow controls RemoteApp launches, while a newRDP_DESKTOP_SCALEcontrols the full-desktop session. When the user does not setRDP_DESKTOP_SCALE, it takes the value ofRDP_SCALEby default.WinApps now checks the values before sending them. A non-integer stops the launch with an error. Below 100 becomes 100, above 500 becomes 500.
Configuration
The "desktop" in
RDP_DESKTOP_SCALEmeans the full-desktop launch, not FreeRDP's/scale-desktop. WinApps rounds each one to 100, 140, or 180 for/scale-device.Existing configs don't need to change. A missing
RDP_SCALEmeans 100, and a missingRDP_DESKTOP_SCALEuses whateverRDP_SCALEis.Testing
Tested against Win11 Pro, FreeRDP 3.32.1, and KDE/wayland; RemoteApps run through XWayland.
Exact scale: I checked the DPI Windows reports for winver, Character Map, Notepad and Paint. Notepad and Paint are Store-packaged win32 apps. As RemoteApps they got 96 DPI at 100 and 120 at 125, over a separate FreeRDP connection with the same values WinApps passes. In the full desktop they got 96, 120 and 144 at 100, 125 and 150. Clicking each RemoteApp's close button also landed in the right spot.
Launch mapping: I captured the FreeRDP commands from a named app, an app opened with a file,
manual, andwinapps windows. The full desktop got the full-desktop scale and the RemoteApps gotRDP_SCALE. Values tested: no scale set (100/100), 125/140, 150/140, 175/180, and 900, which became 500/180. A non-integer stopped the launch with an error.bash -n, ShellCheck,git diff --check, the fullpre-commitsuite, andnix flake checkall pass.Full desktop:
winapps windowsworks with bothsdl-freerdp3(WinApps picks it by itself on Wayland) andxfreerdp3. 150 was tested withRDP_SCALE=100andRDP_DESKTOP_SCALE=150.Setup: setup.sh's own launches (the RDP access check and the app scan) got the RemoteApp scale, 125/140, with
RDP_DESKTOP_SCALEat 175.For the KDE RemoteApp DPI, click and pair tests I used FreeRDP 3.32.1 with its window size filter removed, because of a separate FreeRDP bug that cuts off some apps at startup (see FreeRDP/FreeRDP#13844). The full desktop and setup tests used the stock clients.
Nothing in this change is DE-specific. I tested on KDE/wayland, but it only passes values to FreeRDP, so it should work the same on any other desktops.
AI Assistance Disclosure
I got help from two AI assistants to write most of the code. I reviewed their work, made the final decisions, and tested it myself.