alex-courtis / way-displays

way-displays: Auto Manage Your Wayland Displays
MIT License
239 stars 12 forks source link

Overlapping outputs on some fractional scales #138

Closed fberg closed 4 months ago

fberg commented 7 months ago

With some fractional scales, way-displays creates output layouts that overlap. For instance, using two 3840x2160 displays, and with

ARRANGE: ROW
ALIGN: TOP
SCALING: true
AUTO_SCALE: false
SCALE:
  - NAME_DESC: '!LG Electronics LG ULTRAFINE .*'
    SCALE: 1.8

a layout is created where the outputs have logical width 2133, but the second monitor is positioned at x=2132, according to wayland-info:

interface: 'zxdg_output_manager_v1',                     version:  3, name:  8
    xdg_output_v1
        output: 58
        name: 'DP-1'
        description: 'LG Electronics LG ULTRAFINE ... (DP-1)'
        logical_x: 2132, logical_y: 0
        logical_width: 2133, logical_height: 1200
    xdg_output_v1
        output: 57
        name: 'DP-2'
        description: 'LG Electronics LG ULTRAFINE ... (DP-2)'
        logical_x: 0, logical_y: 0
        logical_width: 2133, logical_height: 1200

(I'm not sure, but it seems that this might also be preventing some Sway key bindings from working (switch to prev/next output), which is how I noticed. From observation, this works when the scale is set such that the layout is not overlapping. In my case, scales of 1.7 and 1.9 both work here. Positioning the output at x=2133 using wlr-randr also resolves this.)

Setup:

The compositor supports the new fractional scaling protocol, maybe that's the issue here? Afaik, this uses scales in increments of 1/120, while wl_fixed_from_double() which is used in computing the desired scale uses a base of 256.

It's even worse with scale 2.01, where logical_x: 1909 (as set by way-displays) and logical_width: 1912. On the other hand, 120 * 2.01 = 241.2 and 3840 * 120/241 = 1912.03, so that's probably where logical_width comes from.

Using scales that are multiples of 0.125 was fine in my tests (logical_x and logical_width matched exactly), which would make sense since 120 and 256 are both divisible by 8.

alex-courtis commented 7 months ago

Thank you so much for the detailed bug report.

With this example it can be definitively reproduced and unit tested.

alex-courtis commented 7 months ago

This will be fixed; sanity checks will be applied. I'll likely get to it next monday.

Perhaps introduced by a move from a manually created wl_fixed_t to wl_fixed_from_double.

The compositor supports the new fractional scaling protocol, maybe that's the issue here?

What's this new protocol? Is it sway specific?

Using scales that are multiples of 0.125 was fine in my tests (logical_x and logical_width matched exactly), which would make sense since 120 and 256 are both divisible by 8.

I've not had good results with non-one-eighth and I don't think it's a good user experience. I might change the suggestion into a rule.

alex-courtis commented 7 months ago

Could you please send your logs? I'll write a unit test specifically for this case - physical width/height, resolution etc.

fberg commented 7 months ago

Thank you for looking into this!

What's this new protocol? Is it sway specific?

I meant fractional-scale-v1 (and here's the MR in the wayland-protocols repo for reference). It's not specific to Sway, and indeed other major compositors like KWin and Mutter have implementations already (Sway doesn't have a release version yet, hence why I built from master). Application/toolkit support has been lagging behind a bit, but FF and Chrome at least have initial implementations, and KDE applications should get it with KDE/Plasma 6, through Qt 6.

I've not had good results with non-one-eighth and I don't think it's a good user experience. I might change the suggestion into a rule.

From my (perhaps limited) understanding, if this protocol is used, the compositor gives clients a buffer and tells them what scale the content should be rendered at. Afterwards, this surface buffer is thrown onto the screen as is, with no additional up or down scaling done by the compositor at all. For instance, the application/client can make sure that fonts and other vector graphics are rendered at the correct (fractional) scale without blurriness. So, if all clients supported this protocol, there should be a perfectly sharp image at any fractional scale.

As for me, restricting to scales that are multiples of 1/8 is fine, and maybe the easiest way to go forward seeing as there seem to be two different ways that outputs may be configured (with Sway at least). But unless I'm wrong, this should no longer be strictly needed with the new protocol. :slightly_smiling_face:

Could you please send your logs?

Here you go: way-displays.log. I've set WAYLAND_DEBUG=1 as well.

I start out with scale 2.0, then switch to 1.8, and finally to 2.01. On scale 2.01, way-displays actually enters a loop where it constantly reconfigures the layout, apparently because it thinks the scale jumps between 2.008 and 2.012. At the very end, I return to scale 2.0

alex-courtis commented 7 months ago

I meant fractional-scale-v1 (and here's the MR in the wayland-protocols repo for reference).

This is fantastic! Thank you for letting me know... I was looking for a wlroots protocol, however wayland itself is a big win.

Hopefully we can do something about X now...

Here you go: way-displays.log. I've set WAYLAND_DEBUG=1 as well.

I start out with scale 2.0, then switch to 1.8, and finally to 2.01.

Many thanks for all the detail. I'll get this sorted out on mainline and sway master.

On scale 2.01, way-displays actually enters a loop where it constantly reconfigures the layout, apparently because it thinks the scale jumps between 2.008 and 2.012. At the very end, I return to scale 2.0

Great find! This can be hardened against or protected against via discrete scales.

alex-courtis commented 7 months ago

Thanks to the info above I was able to replicate with some instrumentation.

It appears that wayland truncates the output width and height when applying the scale, wheras way-displays was rounding it. Changed to truncation.

@fberg I'd be most grateful if you could test a fix for the above, with one of the problematic ones like 1.8

git clone git@github.com:alex-courtis/way-displays.git
cd way-displays
git 138-fix-overlapping-outputs
make
sudo make install

When you are done you can:

sudo make uninstall

I'll add the quantization after that which should prevent all the flipping around.

fberg commented 7 months ago

Thank you! I'll make sure to do some tests on the weekend, when I'll have access to that machine again.

fberg commented 7 months ago

So I spent some time with this today and unfortunately, 53fef6873cd58df1abf081a163ba385f958d9120 doesn't fix the issue for scale 1.8. I still get the second output positioned at x=2132, which is one pixel less than the width of the first output.

I looked into the code myself a bit and found this to work for me: https://github.com/fberg/way-displays/commit/a7165853bbab2a5080bb932ce7d4de408747562b. That commit fixes both the overlapping outputs as well as the scale flipping for scale 2.01 (and all other tested scales). However, it introduces a similar issue of output overlaps for current release versions of Sway (and probably other wlroots compositors). One can probably detect whether the compositor supports the fractional scaling protocol, but I'm not sure if that's all it takes (and I'm way out of my depth here).

Since you wanted to quantize scales to 1/8ths anyway (and this was tested to be okay without any other modifications), I'm not sure if it's worth it to spend too much time on this. The difficulty is predicting how large the output will be after it was configured. Perhaps the best way would be to configure outputs one-by-one and looking at the sizes of the already configured outputs before doing the next? This would probably need a whole lot of rewriting though.

Btw: not sure if it's of interest, but I noticed that way-displays 1.9.0 with Sway 1.8.1 (i.e. not supporting the fractional-scale-v1 yet) can also generate layouts that are not matching up exactly: with scale 1.9 I get x=2023 for the second output, but widths are only 2022. So there's a one pixel "gap" between the outputs, but as far as I can tell this at least doesn't cause any problems with Sway like overlapping outputs do.

alex-courtis commented 7 months ago

I still get the second output positioned at x=2132, which is one pixel less than the width of the first output.

I'm not surprised. The "fix" was... hopeful.

I looked into the code myself a bit and found this to work for me: fberg@a716585. That commit fixes both the overlapping outputs as well as the scale flipping for scale 2.01 (and all other tested scales). However, it introduces a similar issue of output overlaps for current release versions of Sway (and probably other wlroots compositors).

That is fantastic! It looks solid, with good use of fixed rather than double. It looks like the quantization will be "correct" based on observed behaviour.

One can probably detect whether the compositor supports the fractional scaling protocol, but I'm not sure if that's all it takes (and I'm way out of my depth here).

We can most definitely do that: https://github.com/alex-courtis/way-displays/blob/393fdd876646c3e64d5cf018ab28f5ea33d194cc/src/listener_registry.c detects advertised capabilities, allowing us to follow different code paths.

We could also use https://gitlab.freedesktop.org/wayland/wayland-protocols/-/blob/main/unstable/xdg-output/xdg-output-unstable-v1.xml?ref_type=heads#L100 to retrieve the actual sizes and positions following scaling. That would be a multi step process, like mode setting: set the output scales then position them based on their actual logical size. That may be unnecessary.

alex-courtis commented 7 months ago

Proposal: can you please create a PR with your work? We can work on it together to come up with a solution that works for all cases that we can test.