Fix X11 screen size when using fractional "scale-up" mode #694
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CRTCs must also be disabled if the requested screen size is, in any dimension, smaller than what the screen size would be without any xrandr scaling applied. See, e.g., [1, 2]. [1] https://lists.x.org/archives/xorg-devel/2018-March/056203.html [2] https://github.com/linuxmint/muffin/blob/b15de53d7bc43dbcd0136cd7f845eb7ec9d89e6a/src/backends/x11/meta-monitor-manager-xrandr.c#L851
Only call meta_monitor_manager_xrandr_update_screen_size() when the CRTCs have been, if necessary, disabled. It is always safe and sometimes required to call it in this state. Previously, for instance, we would not call it in this state when shrinking the screen size (e.g., when in fractional "scale-up" mode), but it is in fact required to call it in this state when using fractional "scale-up" mode.
In previous commits, we have fixed "scale-up" fractional mode scaling when done via the display settings UI. However, there is still the matter of meta_monitor_manager_xrandr_update_screen_size_derived() which can be called on anyone's change to the xrandr configuration. Prior to this commit, to work around an issue where CRTCs must be disabled, this function *never* set the correct screen size when scaling up. This has some horrible consequences like breaking edges on the sides of monitors, windows appearing completely offscreen, incorrect workspace previews, etc., so we should *never* do this. Let's instead try to set the correct screen size. If it fails, that's fine because we are doing it via XCB and will merely log the error. It's better than always setting the wrong screen size. Plus, there is reason to think that the screen size will already be correct anyways. In previous commits we already fixed *our* handling of xrandr scaling to correctly update the screen size when the necessary CRTCs are disabled. Hopefully all other tools are doing it correctly as well and we never have to update the screen size here. :) If this prediction proves inaccurate, we can later update this function to (1) check if the screen size actually needs changing and, if it does, then (2) disable any CRTCs that need disabling before changing the screen size and (3) re-enable them after the change of screen size. Unfortunately I don't know if there is a simpler way. In general, we have to be careful to not disable CRTCs too eagerly either because disabling a CRTC is typically quite easily noticed by a user and results in the screen turning black for a moment.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These commits aim to fix the X11 screen size getting improperly set if fractional scaling is used in "scale-up" mode. An investigation into why we were improperly setting the screen size in the first place led to the discovery that this was to work around an issue where the X server would throw an error if the requested screen size is, in any dimension, smaller than what the screen size would be without any xrandr scaling applied. See, e.g., [1, 2]. The first two commits aim to fix the way that we set the screen size so that the worked around issue no longer applies.
Finally, with the worked around issue no longer applying, the third commit removes the workaround, setting the correct screen size when using fractional scaling in "scale-up" mode.
[1] https://lists.x.org/archives/xorg-devel/2018-March/056203.html
[2]
muffin/src/backends/x11/meta-monitor-manager-xrandr.c
Line 851 in b15de53