search

alex-courtis / way-displays

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

#138 always round scales to nearest 1/8 to support fractional scaling and other recent compositor changes; use --log-threshold debug to see details #145

Closed fberg closed 4 months ago

fberg commented 7 months ago

fixes #138

Attempts to fix #138 by using a different "scaling base" when the compositor supports the new fractional-scale-v1 protocol.

With this PR, I observed no gaps or any overlap between the two outputs (each 3840x2160 pixels) at various tested scales. The issue with "flipping" scales (see discussion in #138) is also gone.

Tested on the following compositors:

I wanted to test on Hyprland 0.32.3 and Wayfire master as well (they would have fractional-scale-v1), but way-displays (even the released 0.9.0) fails to setup the layout when a fractional scale is used, so there is probably something else going on.

fberg commented 7 months ago

@alex-courtis I've included the commits from your branch (138-fix-overlapping-outputs, #144) in this PR since the behavior without fractional-scale-v1 is the same as in 53fef6873cd58df1abf081a163ba385f958d9120, so the updated tests were needed. Hope that's okay.

fberg commented 7 months ago

I wanted to test on Hyprland 0.32.3 and Wayfire master as well

The issues there were probably due to my graphics driver or something, it works again now.

However, it appears that both Hyprland and Wayfire behave different than Sway here: the output sizes after configuration are the same as computed with this PR's code but without the extra fractional-scale-v1 logic. So, perhaps this is a Sway bug after all and #144 would be sufficient? Or do the other compositors have it wrong? :confused: Will have to investigate more.

Edit: here is the relevant Sway code.

alex-courtis commented 7 months ago

Wow... you do not waste time!

Testing / reviewing now.

alex-courtis commented 7 months ago

However, it appears that both Hyprland and Wayfire behave different than Sway here: the output sizes after configuration are the same as computed with this PR's code but without the extra fractional-scale-v1 logic. So, perhaps this is a Sway bug after all and #144 would be sufficient? 😕 Will have to investigate more.

144 was just a hope. It covered river but not sway. We need a complete solution - this is the right way.

For completeness I think we'll need to listen for the actual sizes after the scale is set. I'll branch an experiment off this branch.

alex-courtis commented 7 months ago

I've created a branch https://github.com/alex-courtis/way-displays/tree/138-retrieve-output-dimensions which adds xdg-output-unstable-v1 and listens to output events.

It's not hooked up to anything yet, however it is very useful for debugging, as it shows the actual logical x/y/w/h

Edit: above branch now warns if calculated size does not match actual e.g. WARNING: HDMI-A-1: Logical size 2194x1234 does not match scaled size 2194x1235

fberg commented 7 months ago

I've created a branch which adds xdg-output-unstable-v1 and listens to output events.

Great, that'll be very useful. It seems to me that taking an output's size after it was configured (and configuring them one after the other) is the only real way forward here, seeing as my changes don't work on some compositors.

Of course, one could still clamp the scale to multiples of 0.125 as an intermediate fix.

alex-courtis commented 6 months ago

I've created a branch which adds xdg-output-unstable-v1 and listens to output events.

Great, that'll be very useful. It seems to me that taking an output's size after it was configured (and configuring them one after the other) is the only real way forward here, seeing as my changes don't work on some compositors.

Of course, one could still clamp the scale to multiples of 0.125 as an intermediate fix.

This PR is a bugfix that should resolve the specific issues with fractional scaling compositors and doesn't affect existing functionality.

Clamping scales can be considered a feature (#40) and it doesn't seem to be needed to resolve this case.

Listening to xdg-output may not ever be necessary and adds a lot of complexity i.e. an extra set step.

Providing this branch passes all the test cases as per https://github.com/alex-courtis/way-displays/pull/145#issuecomment-1837655119 we should just merge this and release as a PATCH.

alex-courtis commented 6 months ago

Apologies for the sporadic replies; I should have mentioned that I'm only able to work on this project <1 day per week.

alex-courtis commented 6 months ago

Alternative: just clamp scales by implementing #40

Do you think that would resolve this entire fractional scaling compositor issue?

It would still be useful to have the ability to determine if the compositor does fractional scaling, as it's likely that we will need that some time in the future.

fberg commented 6 months ago

This PR is a bugfix that should resolve the specific issues with fractional scaling compositors and doesn't affect existing functionality.

While it fixes a bug for users of an unreleased Sway version, it does introduce the same problems for Hyprland though. So I unfortunately don't see it as a clear improvement, but I'd be okay with merging it if you think this should be done.

Do you think that [scale clamping] would resolve this entire fractional scaling compositor issue?

Yes, restricting allowed scales to multiples of 1/8ths should resolve all issues that I encountered in my tests.

alex-courtis commented 6 months ago

Yes, restricting allowed scales to multiples of 1/8ths should resolve all issues that I encountered in my tests.

It seems that is the best solution here.

It's disappointing; I like the "fractional scaling choice" and I have a feeling we'll need it in the future.

Options:

  1. create a new PR with hardcoded 1/8ths
  2. rework this PR with hardcoded 1/8ths and keep the fractional-scale-v1 detection
  3. create a new PR to address #40 with a minimum of 1/8
fberg commented 6 months ago

Sorry for my infrequent responses. I'm currently travelling.

I'd be happy to rework my PR to implement option 2, but will only be able to do so by end of January at the earliest. The other options also sound good to me though, and I wouldn't be mad if my PR ends up not being merged. :)

alex-courtis commented 6 months ago

Sorry for my infrequent responses. I'm currently travelling.

I'd be happy to rework my PR to implement option 2, but will only be able to do so by end of January at the earliest. The other options also sound good to me though, and I wouldn't be mad if my PR ends up not being merged. :)

No worries at all. I'm in a similar situation.

fberg commented 5 months ago

Updated the PR to round the scale to a multiple of 1/8 if the compositor has fractional-scale-v1.

alex-courtis commented 5 months ago

Updated the PR to round the scale to a multiple of 1/8 if the compositor has fractional-scale-v1.

I'm really happy with that: I've hardcoded fractional scaling on and the scales are sensible. It's working just fine with sway 1.8.1 and river master (fractional scaling). You've validated that it's resolved your initial failure case on sway master.

  1. Less granular scales are desirable anyway: #40 , we would simply be adding an option to increase HEAD_FRACTIONAL_SCALING_BASE
  2. Fractional scaling is coming; this change will be forced. We may as well "break" it now.
  3. Non HEAD_FRACTIONAL_SCALING_BASE scales do produce a bad/blurry result when integer multiple scaling is used for apps that don't adopt fractional scaling.

∴let's just do this: remove the test for fractional scaling and always use the new codepaths.

alex-courtis commented 5 months ago

We should keep have_fractional_scale_v1. It should be logged at startup time to aid diagnosis of issues.

In a subsequest PR we can add xdg-output-unstable to retrieve the positions, but that will again be information / warning only.

fberg commented 5 months ago

Removed the check for the fractional scaling protocol, scales are now always rounded to the nearest multiple of 1/8th. Let me know if that's not what you intended.

We should keep have_fractional_scale_v1. It should be logged at startup time to aid diagnosis of issues.

There's a debug log message in listener_registry.c.

alex-courtis commented 5 months ago

Removed the check for the fractional scaling protocol, scales are now always rounded to the nearest multiple of 1/8th. Let me know if that's not what you intended.

We should keep have_fractional_scale_v1. It should be logged at startup time to aid diagnosis of issues.

There's a debug log message in listener_registry.c.

Fantastic! I'll review / test this one on Monday.

alex-courtis commented 4 months ago

Real world testing:

ARRANGE: ROW
ALIGN: BOTTOM
ORDER:
  - LG Display 0x05EF
  - M28U
  - Gigabyte M32Q
SCALING: TRUE
AUTO_SCALE: TRUE
SCALE:
  - NAME_DESC: LG Display 0x05EF
    SCALE: 2.3
  - NAME_DESC: M28U
    SCALE: 1.7
  - NAME_DESC: Gigabyte M32Q
    SCALE: 1.9
VRR_OFF:
  - LG Display 0x05EF
  - M28U
  - Gigabyte M32Q

released:

interface: 'zxdg_output_manager_v1',                     version:  3, name: 13
    xdg_output_v1
        output: 47
        name: 'HDMI-A-1'
        description: 'GIGA-BYTE TECHNOLOGY CO., LTD. M28U 21490B006081 (HDMI-A-1)'
        logical_x: 1113, logical_y: 0
        logical_width: 2259, logical_height: 1271
    xdg_output_v1
        output: 46
        name: 'eDP-1'
        description: 'LG Display 0x05EF (eDP-1)'
        logical_x: 0, logical_y: 645
        logical_width: 1112, logical_height: 625
    xdg_output_v1
        output: 45
        name: 'DP-6'
        description: 'GIGA-BYTE TECHNOLOGY CO., LTD. Gigabyte M32Q 0x0000064C (DP-6)'
        logical_x: 3373, logical_y: 512
        logical_width: 1348, logical_height: 758
interface: 'wl_output',                                  version:  4, name: 45
    name: DP-6
    description: GIGA-BYTE TECHNOLOGY CO., LTD. Gigabyte M32Q 0x0000064C (DP-6)
    x: 0, y: 0, scale: 2,
    physical_width: 700 mm, physical_height: 390 mm,
    make: 'GIGA-BYTE TECHNOLOGY CO., LTD.', model: 'Gigabyte M32Q',
    subpixel_orientation: unknown, output_transform: normal,
    mode:
        width: 2560 px, height: 1440 px, refresh: 143.912 Hz,
        flags: current
interface: 'wl_output',                                  version:  4, name: 46
    name: eDP-1
    description: LG Display 0x05EF (eDP-1)
    x: 0, y: 0, scale: 3,
    physical_width: 310 mm, physical_height: 170 mm,
    make: 'LG Display', model: '0x05EF',
    subpixel_orientation: unknown, output_transform: normal,
    mode:
        width: 2560 px, height: 1440 px, refresh: 59.998 Hz,
        flags: current
interface: 'wl_output',                                  version:  4, name: 47
    name: HDMI-A-1
    description: GIGA-BYTE TECHNOLOGY CO., LTD. M28U 21490B006081 (HDMI-A-1)
    x: 0, y: 0, scale: 2,
    physical_width: 630 mm, physical_height: 360 mm,
    make: 'GIGA-BYTE TECHNOLOGY CO., LTD.', model: 'M28U',
    subpixel_orientation: unknown, output_transform: normal,
    mode:
        width: 3840 px, height: 2160 px, refresh: 30.000 Hz,
        flags: current

Logical positions and scales do not add up.

branch:

interface: 'zxdg_output_manager_v1',                     version:  3, name: 13
xdg_output_v1
output: 47
name: 'HDMI-A-1'
description: 'GIGA-BYTE TECHNOLOGY CO., LTD. M28U 21490B006081 (HDMI-A-1)'
logical_x: 1137, logical_y: 0
logical_width: 2194, logical_height: 1234
xdg_output_v1
output: 46
name: 'eDP-1'
description: 'LG Display 0x05EF (eDP-1)'
logical_x: 0, logical_y: 594
logical_width: 1137, logical_height: 640
xdg_output_v1
output: 45
name: 'DP-6'
description: 'GIGA-BYTE TECHNOLOGY CO., LTD. Gigabyte M32Q 0x0000064C (DP-6)'
logical_x: 3331, logical_y: 466
logical_width: 1365, logical_height: 768
interface: 'wl_output',                                  version:  4, name: 45
name: DP-6
description: GIGA-BYTE TECHNOLOGY CO., LTD. Gigabyte M32Q 0x0000064C (DP-6)
x: 0, y: 0, scale: 2,
    physical_width: 700 mm, physical_height: 390 mm,
    make: 'GIGA-BYTE TECHNOLOGY CO., LTD.', model: 'Gigabyte M32Q',
    subpixel_orientation: unknown, output_transform: normal,
    mode:
width: 2560 px, height: 1440 px, refresh: 143.912 Hz,
    flags: current
    interface: 'wl_output',                                  version:  4, name: 46
    name: eDP-1
    description: LG Display 0x05EF (eDP-1)
    x: 0, y: 0, scale: 3,
    physical_width: 310 mm, physical_height: 170 mm,
    make: 'LG Display', model: '0x05EF',
    subpixel_orientation: unknown, output_transform: normal,
    mode:
width: 2560 px, height: 1440 px, refresh: 59.998 Hz,
    flags: current
    interface: 'wl_output',                                  version:  4, name: 47
    name: HDMI-A-1
    description: GIGA-BYTE TECHNOLOGY CO., LTD. M28U 21490B006081 (HDMI-A-1)
    x: 0, y: 0, scale: 2,
    physical_width: 630 mm, physical_height: 360 mm,
    make: 'GIGA-BYTE TECHNOLOGY CO., LTD.', model: 'M28U',
    subpixel_orientation: unknown, output_transform: normal,
    mode:
width: 3840 px, height: 2160 px, refresh: 30.000 Hz,
    flags: current

Everything adds up.