Skip to content
This repository has been archived by the owner on Nov 1, 2021. It is now read-only.

wlr-output-management: not enough precision for scale #104

Open
AdrianVovk opened this issue May 20, 2021 · 8 comments
Open

wlr-output-management: not enough precision for scale #104

AdrianVovk opened this issue May 20, 2021 · 8 comments

Comments

@AdrianVovk
Copy link

Context:

I'm building a daemon that implements the org.mutter.DisplayConfig protocol on top of wlr-output-management and wf-config

Issue:

Currently, the scale event sends a wl_fixed with the scale. However, wl_fixed does not have enough precision to represent fractional scales correctly. For instance, on my test MacBook one of the ideal scales is 1.6. This is a value that doubles cannot represent exactly; the best they can do is 1.60000...008.... However, since the number of pixels on any axis of the screen are going to be somewhere in the thousands range, a change in scale less than 0.0001 will have no effect on computations.

Trying to represent 1.6 with a wl_fixed_t yields a different result. Trying to convert 1.6 to a wl_fixed and back returns 1.60156.... There is not enough precision to represent values in the 0.0001 range, which is necessary to ensure proper scale values.

Because wlr-output-management uses a wl_fixed to send scale values, it cannot accurately report the current scale to its clients. Any calculations done with that scale will be off.

Potential solution:

The easiest and dirtiest solution would probably be to just multiply the scale by a value like 1000 in the compositor and then have the client divide it by 1000. This could be detailed in the protocol's docs.

The cleanest solution is probably modifying the Wayland wire protocol to be able to send true doubles and use that instead.

Workaround:

In my use case, I compute a list of "good" scales and those are the only scales the user can select in the GUI. Therefore, the scale reported by wlr-output-management should be one of those good scales. Here's the code

@zzag
Copy link
Contributor

zzag commented May 21, 2021

The easiest and dirtiest solution would probably be to just multiply the scale by a value like 1000 in the compositor and then have the client divide it by 1000

For what it's worth, chromeos does something like that. https://chromium.googlesource.com/chromiumos/platform2/+/HEAD/vm_tools/sommelier/protocol/aura-shell.xml#238

However, there's still a problem - the compositor and the clients need to agree on how fractional values are rounded.

@AdrianVovk
Copy link
Author

However, there's still a problem - the compositor and the clients need to agree on how fractional values are rounded.

Anything beyond 0.0001 of precision has no effect on the scaled size, unless I'm doing my napkin math wrong. The compositor could multiply by 10,000 instead and then send it as an integer and not bother with decimals at all. Client would divide by 10,000 and get the scale value it can pass around and use reliably in calculations. I see no need for rounding here. Am I missing something?

Actually, this workaround already has precedent in wlr-output-management: wlr_output_mode's refresh event. It sends the refresh rate in mHz instead of Hz, and it sends the refresh rate as an integer.

@zzag
Copy link
Contributor

zzag commented May 21, 2021

Whether the compositor floor()s or round()s values matters if it wants to compute the surface size from the buffer size. For example,

  • buffer width: 840, scale: 1.5 (or 1500 if multiplied by 1000): 840 / 1.5 = 560
  • buffer width 841, scale: 1.5 (or 1500 if multiplied by 1000): 841 / 1.5 = 560.6

In the last example, it matters if the compositor floors or rounds floating point numbers.

If all arithmetic is done with integers though, i.e. 841 * 1000 / 1500 instead of 841 / 1.5, it means that the compositor floor()s floating point numbers. It's a minor detail, but little details such as this are important.

Edit: On a second read, I think I'm in a wrong thread :D. I thought this was about adding fractional scale factors on Wayland in general, but this seems to be only about fractional scale factors in wlr-output-management

@AdrianVovk
Copy link
Author

@zzag ah I see you meant rounding the scaled sizes. That makes sense. However, for this protocol and use case (output management) the client probably doesn't care. The only thing the client really would be interested in calculating with the scale is the size of the entire output, and whether or not a given scale evenly divides into the resolution of the output. Why/how would an output management client keep track of the sizes of surfaces? At least that's my thinking from my limited perspective

I have nothing against standardizing on floor() or round() whichever makes most sense. Definitely not qualified to contribute an answer to that.

@emersion
Copy link
Member

I'd prefer to

  1. Use int/uint instead of fixed and carefully choose the right unit just like wl_output.mode's refresh does with mHz.
  2. Make it a protocol error if the mode size isn't divisible by the scale factor maybe?

@zzag
Copy link
Contributor

zzag commented May 24, 2021

Make it a protocol error if the mode size isn't divisible by the scale factor maybe?

With this, the compositor will have to forbid some scale factors, won't it? e.g. if mode width is 3840 the following scale factors will be valid

  • 1.0: 3840 / 1.0 = 3840
  • 1.25: 3840 / 1.25 = 3072
  • 1.5: 3840 / 1.5 = 2560
  • 1.75: 3840 / 1.75 = 2194.2 (nope!)
  • 2.0: 3840 / 2.0 = 1920

@emersion
Copy link
Member

emersion commented Jun 3, 2021

With this, the compositor will have to forbid some scale factors, won't it?

Yes. The idea is that half-pixels don't exist, so someone will need to adjust the scale factor. Instead of doing the rounding in the compositor, make the rounding in the client.

@emersion
Copy link
Member

emersion commented Nov 1, 2021

wlr-protocols has migrated to gitlab.freedesktop.org. This issue has been moved to:

https://gitlab.freedesktop.org/wlroots/wlr-protocols/-/issues/104

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

3 participants