More reliable pixendensitydetection under Xorg/Randr #5178
Conversation
Physical dimensions are parsed from the monitors EDID instead of Randr. GuessPixelDensity locks to fixed values, caps at 2 and returns 1 if the supplied dimensions are unreasonable.
|
|
src/Avalonia.X11/X11Screens.cs
Outdated
| { | ||
| if(rrOutput == IntPtr.Zero) | ||
| return null; | ||
| var output = Marshal.ReadInt64(rrOutput); |
There was a problem hiding this comment.
Are you sure that it's 64 bit on 32-bit ARM too? Might be better to use ReadIntPtr
There was a problem hiding this comment.
Actually, _XRRMonitorInfo is defined like this:
typedef struct _XRRMonitorInfo {
Atom name;
Bool primary;
Bool automatic;
int noutput;
int x;
int y;
int width;
int height;
int mwidth;
int mheight;
RROutput *outputs;
} XRRMonitorInfo;and RROutput is a XID, so we should make the Outputs property to be IntPtr*
src/Avalonia.X11/X11Screens.cs
Outdated
| if(!hasEDID) | ||
| return null; | ||
| XRRGetOutputProperty(_x11.Display, output, _x11.Atoms.RR_PROPERTY_RANDR_EDID, 0, 128, false, false, _x11.Atoms.AnyPropertyType, out int actualType, out int actualFormat, out int nItems, out _, out IntPtr prop); | ||
| if(actualType != 19) // XA_INTEGER |
There was a problem hiding this comment.
XA_INTEGER is defined in X11Atoms class and Atom enum
src/Avalonia.X11/X11Screens.cs
Outdated
| XFree(new IntPtr(properties)); | ||
| if(edid.Length < 22) | ||
| return null; | ||
| var width = edid[21]; |
There was a problem hiding this comment.
Since those are magic numbers, please, quote the standard, e. g.
// 15h 1 Max. Horizontal Image Size cm.
// 16h 1 Max. Vertical Image Size cm.
src/Avalonia.X11/X11Screens.cs
Outdated
| density = X11Screen.GuessPixelDensity(mon.Width, mon.MWidth); | ||
| } | ||
|
|
||
| var pSize = GetPhysicalMonitorSizeFromEDID(mon.Outputs); |
There was a problem hiding this comment.
I guess we should also check NOutputs to be >= 1. BTW, any idea why it can be more than 1?
There was a problem hiding this comment.
This can be the case if the user creates an monitor with multiple outputs attache by running something like this:
$ xrandr --setmonitor DP-2-1 auto DP-2,HDMI-1
$ xrandr --listmonitors
Monitors: 1
0: DP-2-1 5120/0x1440/1+0+0 DP-2 HDMI-1
So detecting the smallest scaling factor among the attached outputs should be safest way to handle this case.
|
It would be nice to also get the actual monitor name from EDID too, so we could have a more fine-grained override via environment variable (obviously that's outside of the scope of this PR) At some point we might also define a user-wide/system-wide config and a configuration utility like Qt does. |
Change XRRMonitorInfo.Outputs from IntPtr to IntPtr* and iterate over all Outputs to detect the smallest pixeldensity.
What does the pull request do?
Physical monitor dimensions are parsed from the monitors EDID instead of requesting them from Randr.
Automatic pixeldensity detection locks to specific scalingfactors (100%, 125%,150%,175%,200%) and falls back to 100% if internal values are way off (scalingfactor higher than 300%) or no EDID is found.
What is the current behavior?
Xrandr reports unreliable physical monitor dimensions with hacky/invalid EDIDs, e.g. no physical dimension for specific modes.
This can lead to scalingfactors of 1000% and higher.
What is the updated/expected behavior with this PR?
How was the solution implemented (if it's not obvious)?
XRRListOutputPropertiesandXRRGetOutputProperty. Code Reference from ChromiumBasic Display Parameters, as these values should be more likely to be correct than the modespecific dimension.Wikipedia ReferenceChecklist
Breaking changes
Fixed issues
Fixes #5124