Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Precise pointer events for Wayland platform #1854

Merged
merged 6 commits into from
Apr 19, 2021

Conversation

wmww
Copy link
Contributor

@wmww wmww commented Dec 8, 2020

This adds support for floating point pointer motion and scrolling to the Wayland platform. It appears touches were already precise. Fixes #1852. There's still a notable problem, where scrolling events don't appear to be precise because they are arbitrarily multiplied by 10 by the Wayland frontend. This will be fixed in a different PR.

Comment on lines 53 to 54
std::pair<float, float> const& pos,
std::pair<float, float> const& scroll) = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We really ought to have strong types for pos and scroll. Lists of identical types are easy to get in the wrong order and this continues down that "slippery slope".

I know you've mentioned floating point geometry before (#1829) and this seems like an opportunity to do something towards it.

(I'm not convinced that generalizing the current "geometry" classes is the best way forward, but we could think about that.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, following the style here, why isn't this just tuple<float, float, float, float> const& axes ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why isn't this just tuple<float, float, float, float> const& axes ;)

Because that has substantially more ways to induce bugs? As it currently is, the main way to get is wrong is passing parameters out of order (which you can check because they have meaningful names) or mixing up .first and .second which again are at least obviously wrong once you notice. A 4-part tuple has all of those problems, plus making it ambiguous (unless explained in comments) what order things are supposed to be in. A more convincing alternative to me would be float pos_x, float pos_y, float scroll_horz, float scroll_vert, but I'm not sure if that has any particular advantages over std::pair.

Of course you're right that we need a real solution to #1829, but I'm not convinced this needs to be blocked on that. The in-practice maintenance burden here is low.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In part I agree: this doesn't touch any API accessible from downstream.

But if we're passing a generic 'tuple around several interfaces we really ought to have a type. And that internal type could be replaced with a "real solution to #1829".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a generic 'tuple

In this case pair is just a 2-tuple

@wmww wmww force-pushed the wayland-platform-precise-input branch from e60ce90 to 1630616 Compare March 22, 2021 18:37
@wmww
Copy link
Contributor Author

wmww commented Mar 22, 2021

This now uses the new floating point geometry types. Stacked on #1961.

@wmww wmww force-pushed the wayland-platform-precise-input branch from 1630616 to d8b5c99 Compare March 22, 2021 20:58
Copy link
Contributor

@AlanGriffiths AlanGriffiths left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks sensible, doesn't break anything obvious. Let's have it on trunk.

bors r+

@bors
Copy link
Contributor

bors bot commented Apr 19, 2021

Build succeeded:

@bors bors bot merged commit 1a3535d into master Apr 19, 2021
@bors bors bot deleted the wayland-platform-precise-input branch April 19, 2021 13:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

High precision input events for the Wayland platform
2 participants