#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

fberg · 2023-12-03T18:11:33Z

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:

Sway 1.8.1
Sway master (has fractional-scale-v1)
labwc 0.6.6

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.

…evert to assert_float_equal

fberg · 2023-12-03T18:15:40Z

@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 53fef68, so the updated tests were needed. Hope that's okay.

fberg · 2023-12-04T00:17:31Z

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? 😕 Will have to investigate more.

Edit: here is the relevant Sway code.

alex-courtis · 2023-12-04T00:34:50Z

src/head.c

+		 * where the factor 120 comes from. It does not occur on Sway 1.8.1 (which doesn't yet have support for the
+		 * fractional scaling protocol).
+		 *
+		 * TODO: Can we "prove" that this is the right thing to do by looking at protocol specifications?


I couldn't find anything official; it looks like truncation however it's really not clear.

alex-courtis · 2023-12-04T00:35:32Z

Wow... you do not waste time!

Testing / reviewing now.

alex-courtis · 2023-12-04T00:37:58Z

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

This is looking great! Many thanks for your speedy work...

alex-courtis · 2023-12-04T00:39:22Z

inc/head.h

@@ -26,6 +27,8 @@ struct HeadState {

 struct Head {

+	struct Displ *displ;


Once we are happy we can probably refactor this away. I'm not a fan of back references ;)

Removed this in d032ac6 in favor of a scaling_base integer in Head.

alex-courtis · 2023-12-04T00:41:45Z

src/head.c

+		 *
+		 * Note that this is currently only *observed* behavior in a Sway revision with fractional-scale-v1, which is
+		 * where the factor 120 comes from. It does not occur on Sway 1.8.1 (which doesn't yet have support for the
+		 * fractional scaling protocol).


Whichever direction we go, this quantization should be appropriate in all situations.

alex-courtis · 2023-12-04T05:48:48Z

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 · 2023-12-05T23:24:37Z

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 · 2023-12-10T00:52:24Z

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 #145 (comment) we should just merge this and release as a PATCH.

alex-courtis · 2023-12-10T00:53:01Z

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 · 2023-12-10T00:57:06Z

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 · 2023-12-12T17:53:47Z

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 · 2023-12-18T00:38:15Z

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:

create a new PR with hardcoded 1/8ths
rework this PR with hardcoded 1/8ths and keep the fractional-scale-v1 detection
create a new PR to address Feature: SCALE_ROUND #40 with a minimum of 1/8

fberg · 2023-12-24T09:05:43Z

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 · 2023-12-24T23:32:07Z

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.

…ng compositors

fberg · 2024-01-28T20:47:03Z

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

alex-courtis · 2024-01-30T01:14:40Z

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.

Less granular scales are desirable anyway: Feature: SCALE_ROUND #40 , we would simply be adding an option to increase HEAD_FRACTIONAL_SCALING_BASE
Fractional scaling is coming; this change will be forced. We may as well "break" it now.
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 · 2024-01-30T01:17:42Z

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.

Now uses a default scaling base of 8.

fberg · 2024-02-02T19:20:22Z

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 · 2024-02-03T01:12:36Z

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

This is fantastic, the calculated scales for 1/120 1/256 always produce correct logical positions and dimensions.

I hope you don't mind, I enhanced the logging and added some tests.

alex-courtis · 2024-02-05T02:57:18Z

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.

alex-courtis and others added 4 commits November 28, 2023 16:41

alex-courtis#138 truncate scaled dimensions to match wayland

53fef68

alex-courtis#138 assert_double_equal is not available on CI cmocka, r…

098f4be

…evert to assert_float_equal

alex-courtis#138 deal with compositors that support fractional-scale-v1

f014ab2

alex-courtis#138 add tests

a30ad84

alex-courtis#138 tests with integral scales

f201f07

fberg marked this pull request as draft December 4, 2023 00:18

alex-courtis reviewed Dec 4, 2023

View reviewed changes

This was referenced Dec 4, 2023

#138 truncate scaled dimensions to match wayland #144

Closed

Feature: SCALE_ROUND #40

Open

alex-courtis#138 don't keep a Displ* in Head

d032ac6

alex-courtis#138 round scales to multiples of 1/8 on fractional scali…

d7890e8

…ng compositors

fix typo

03d78ec

alex-courtis#138 disable check for fractional-scale-v1 entirely

b40ab4a

Now uses a default scaling base of 8.

Merge branch 'master' into 138-handle-fractional-scale

db8e90b

alex-courtis added 4 commits February 5, 2024 13:47

alex-courtis#138 more scale rounding debug and test

4f8af6e

alex-courtis#138 iwyu fixes

9df7dec

alex-courtis#138 tidy fractional scaling message

e6cfffd

alex-courtis#138 remove test harness

57e1c58

alex-courtis approved these changes Feb 5, 2024

View reviewed changes

alex-courtis marked this pull request as ready for review February 5, 2024 02:57

alex-courtis changed the title ~~Account for different output sizes when the compositor supports fractional-scale-v1~~ #138 always round scales to nearest 1/8 to support fractional scaling and other recent compositor changes; use --log-threshold debug to see details Feb 5, 2024

alex-courtis merged commit 0a5d199 into alex-courtis:master Feb 5, 2024
1 check passed

alex-courtis mentioned this pull request Jul 7, 2024

Wrong position when aligning screens artizirk/wdisplays#20

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#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

#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

fberg commented Dec 3, 2023 •

edited by alex-courtis

Loading

fberg commented Dec 3, 2023

fberg commented Dec 4, 2023 •

edited

Loading

alex-courtis Dec 4, 2023

alex-courtis commented Dec 4, 2023

alex-courtis commented Dec 4, 2023 •

edited

Loading

alex-courtis left a comment

alex-courtis Dec 4, 2023

fberg Dec 5, 2023

alex-courtis Dec 4, 2023

alex-courtis commented Dec 4, 2023 •

edited

Loading

fberg commented Dec 5, 2023

alex-courtis commented Dec 10, 2023

alex-courtis commented Dec 10, 2023

alex-courtis commented Dec 10, 2023

fberg commented Dec 12, 2023

alex-courtis commented Dec 18, 2023

fberg commented Dec 24, 2023 •

edited

Loading

alex-courtis commented Dec 24, 2023

fberg commented Jan 28, 2024 •

edited

Loading

alex-courtis commented Jan 30, 2024

alex-courtis commented Jan 30, 2024

fberg commented Feb 2, 2024

alex-courtis commented Feb 3, 2024

alex-courtis left a comment

alex-courtis commented Feb 5, 2024

		@@ -26,6 +27,8 @@ struct HeadState {

		struct Head {

		struct Displ *displ;

#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

#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

Conversation

fberg commented Dec 3, 2023 • edited by alex-courtis Loading

fberg commented Dec 3, 2023

fberg commented Dec 4, 2023 • edited Loading

alex-courtis Dec 4, 2023

Choose a reason for hiding this comment

alex-courtis commented Dec 4, 2023

alex-courtis commented Dec 4, 2023 • edited Loading

alex-courtis left a comment

Choose a reason for hiding this comment

alex-courtis Dec 4, 2023

Choose a reason for hiding this comment

fberg Dec 5, 2023

Choose a reason for hiding this comment

alex-courtis Dec 4, 2023

Choose a reason for hiding this comment

alex-courtis commented Dec 4, 2023 • edited Loading

fberg commented Dec 5, 2023

alex-courtis commented Dec 10, 2023

alex-courtis commented Dec 10, 2023

alex-courtis commented Dec 10, 2023

fberg commented Dec 12, 2023

alex-courtis commented Dec 18, 2023

fberg commented Dec 24, 2023 • edited Loading

alex-courtis commented Dec 24, 2023

fberg commented Jan 28, 2024 • edited Loading

alex-courtis commented Jan 30, 2024

alex-courtis commented Jan 30, 2024

fberg commented Feb 2, 2024

alex-courtis commented Feb 3, 2024

alex-courtis left a comment

Choose a reason for hiding this comment

alex-courtis commented Feb 5, 2024

fberg commented Dec 3, 2023 •

edited by alex-courtis

Loading

fberg commented Dec 4, 2023 •

edited

Loading

alex-courtis commented Dec 4, 2023 •

edited

Loading

alex-courtis commented Dec 4, 2023 •

edited

Loading

fberg commented Dec 24, 2023 •

edited

Loading

fberg commented Jan 28, 2024 •

edited

Loading