Repository navigation
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? |
oskardotglobal
added
triage
|
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. |
RDP_SCALE was rounded to 100, 140 or 180 on the premise that FreeRDP only supports those values. That is true of /scale, but not of /scale-desktop, which accepts any integer from 100 to 500. A user asking for 125 silently got 140. Pass the configured value through as /scale-desktop, and derive the device scale separately as /scale-device. Both factors still get set: /scale set them together, so switching to /scale-desktop alone would have quietly reset DeviceScaleFactor to 100 and made Store apps worse. The device factor genuinely is limited to 100, 140 and 180, so it is still approximated - but only it, and the debug output now says so rather than claiming the whole value was unsupported. The per-launch notify-send is gone. waFixScale runs on every configuration load, so it fired on every launch to report something the user cannot act on. RDP_SCALE is now validated, since the old rounding incidentally sanitised it. A non-integer is fatal; a value outside 100-500 is clamped with a warning rather than rejected, so an existing misconfigured setup keeps working instead of failing to launch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Tony Dursun <oraturk75@gmail.com>
|
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 |
RDP_SCALE now controls RemoteApp launches only. RDP_DESKTOP_SCALE controls the full-desktop session started by 'winapps windows' and defaults to the RDP_SCALE value, as requested in review. Its name refers to the launch type, not to FreeRDP's /scale-desktop option: both settings supply a /scale-desktop value for their own launch type, and each derives its own /scale-device. Both settings also accept 'auto', which is the new default. On Cinnamon under X11 that reads the primary monitor's applied logical scale from Muffin's DisplayConfig and converts it to a percentage. The scale is read as a decimal string and converted with integer arithmetic, so no locale-sensitive float parsing is involved. Detection is best effort and never blocks a launch. gdbus is guarded by command -v; it normally ships with Cinnamon, and without it automatic detection falls back to 100. Every failure falls back to 100 with the reason logged: tool missing, bus unreachable, unparseable reply, no primary monitor, or an out-of-range scale. An explicit number always wins and bypasses detection entirely, and two 'auto' values share a single detection result per invocation. Cinnamon quantises scale per display mode, so a nominal 175% setting can apply as 176%. The applied value is used rather than the number shown in the settings UI. Changing the display resolution is not the same as changing its scale, and only the scale is followed. Not implemented: per-monitor values, rescaling a window moved between monitors, monitor hotplug, and scale changes during a running session. Detection is verified on Cinnamon under X11; every other desktop falls back to 100. An invalid scale value now also reports to stderr, rather than only to the log and a desktop notification. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Tony Dursun <oraturk75@gmail.com>
1ff14dd to
3c137bc
Compare
oraturk75
changed the title
|
@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
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. |
What this changes
RDP_SCALEwas rounded to 100, 140, or 180 because WinApps assumed FreeRDP only supported those values. FreeRDP takes scaling values 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 applications./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. Hence, a user requested scale of 125 is rounded to 140 by WinApps before being sent through FreeRDP’s/scaleoption.This PR now sends the exact scaling value through
/scale-desktop, calculates/rounds the corresponding value, and then sends it to/scale-deviceseparately. I had to set both, not only/scale-desktop, to keep the existing behaviour unchanged.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.Both settings also accept
auto, which is now the default. This implements the automatic host scale detection part of #825 for a single-monitor setup.Configuration
The name
RDP_DESKTOP_SCALErefers to the full-desktop launch type, not to be confused with FreeRDP’s/scale-desktopoption. Both WinApps settings provide a/scale-desktopvalue for their own respective launch type and automatically calculate an acceptable (100, 140, or 180)/scale-devicevalue.Existing configurations do not need to change. If
RDP_SCALEis missing, it usesauto. If the newRDP_DESKTOP_SCALEis missing, it takes the resolvedRDP_SCALEvalue.Automatic detection
On Cinnamon/X11,
autoreads the primary monitor’s applied scale from Muffin’sDisplayConfig.Automatic detection is designed to do its best, but fail safely and never block a launch. If detection is unavailable or fails, WinApps falls back to 100 and records the reason in its own log. The user can always bypass detection by manually setting their own desired value.
I did not implement per-monitor scaling for multi-monitor setups, rescaling an actively running RemoteApp when it moves between monitors, monitor hotplug, or scale changes during an active session.
The detection is kept in a separate function, so support for other desktop environments can be added without changing the configuration or launch code.
WinApps now validates the values before sending them; non-integers fail with a visible error, values below 100 are changed to 100, and values above 500 are changed to 500.
Testing
Tested against Win11 Pro, FreeRDP 3.30.0, and Cinnamon 6.4.13 under X11. No test framework was added.
Exact scale: I tested
winveras a RemoteApp using the geometry reported by my Win11 VM. Scale 100 produced 460x423, 125 produced 575x521, and 150 produced 691x621. The dimensions followed the requested scale within two pixels.Automatic detection: With my host at 100%, WinApps detected 100. After setting my host to 175% in Cinnamon, WinApps detected the applied value of 176. After restoring my host to 100%, WinApps detected 100 again. I also tested automatic detection from Cinnamon’s application menu, and it returned 176.
Detection failures: I tested every detection failure path, including missing
gdbus, a failed D-Bus call, malformed replies, missing primary-monitor information, and invalid detected scales. Every case fell back to 100 without blocking the launch.End-to-end scaling: With my host at 175%, a Windows test script inside the RemoteApp session reported 168 DPI. This confirms that the detected scale reached the Windows session.
Launch mapping: I captured the FreeRDP commands generated by all four WinApps launch paths. The full-desktop launch used the full-desktop scale, while all three RemoteApp launches used the configured
RDP_SCALEvalue. When no separate full-desktop value was configured, the full-desktop launch also usedRDP_SCALEvalue.bash -n, ShellCheck,git diff --check, and the fullpre-commitsuite all pass.Not tested with Wayland. WinApps uses
sdl-freerdp3for full-desktop launches on Wayland, while my environment uses X11.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 personally tested and confirmed the intended functionality.