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

Make PhysicalPosition integer, make dedicated Input* types as necessary #1135

Open
kennylevinsen opened this issue Aug 30, 2019 · 7 comments
Labels
C - needs discussion Direction must be ironed out S - api Design and usability S - meta Project governance

Comments

@kennylevinsen
Copy link

kennylevinsen commented Aug 30, 2019

Splitting out from a comment in #939.

As part of the DPI overhaul, PhysicalSize has been converted to an integer type to better represent the physical pixels it is designed to map to.

PhysicalPosition was left as a float to allow certain types of high resolution input to have sub-pixel resolution. However, as certain types of devices also report input geometries, I believe that serving sub-pixel resolution input devices require fully floating types.

Considering that using floating types for physical pixel addressing is awkward and confusing, I believe that these are simply two problems that need individual solutions. Therefore, I propose converting PhysicalPosition to integer as well, and introducing separate types for sub-pixel resolution input as necessary.

This leaves us with clean types that have clearly defined purposes, rather that having a situation where we are hammering everything into the same shape.

@goddessfreya goddessfreya added C - needs discussion Direction must be ironed out S - api Design and usability S - meta Project governance labels Aug 30, 2019
@goddessfreya
Copy link
Contributor

goddessfreya commented Aug 30, 2019

Personally, I was suprized to learn that PhysicalPosition wasn't an integer and thought that it was something overlooked when drafting the dpi overhaul. It's not obvious to me, and I imagine others, that PhysicalPosition sometimes could report decimal numbers, and so I think splitting PhysicalPosition would be a good idea. I suggest we also make the respective InputSize struct, or whatever we want to name it, just for consistency's sake, even if unused in the codebase, but beyond that I fully support this idea.

I imagine you folks would be interested in this discussion: @Osspial @mtak- @murarth @aleksijuvani @ryanisaacg

@goddessfreya
Copy link
Contributor

Ah, and @mb64 and @philip-alldredge, since you two appear to be working on the android backend, which I imagine reports touch related stuff with ssub-pixel resolution.

@mtak-
Copy link
Contributor

mtak- commented Aug 31, 2019

That makes sense to me. I'm seeing non-integral values for majorRadius on iOS.

@ghost
Copy link

ghost commented Aug 31, 2019

It feels a bit iffy to me to introduce new types when we could be just using floats everywhere. It's not surprising to me that floats would be cast towards zero where sub-pixel accuracy is not available. That said, I don't feel particularly strongly about this.

@Osspial
Copy link
Contributor

Osspial commented Sep 5, 2019

I support introducing integer types. However, I'm not a huge fan of introducing separate Input* types, since that name doesn't make it clear if they're in a physical or logical coordinate space. I'd rather make all the Size/Position types generic over their number type (PhysicalPosition becomes PhysicalPosition<N>{x: N, y: N}), which would avoid that confusion.

@aleksijuvani IMO using floats in the API implies that the decimal places are going to be used, which I found confusing when getting started with the Winit API. Rust has separate types for integer and decimal types, and it feels un-Rusty to not take advantage of that.

@mb64
Copy link

mb64 commented Sep 22, 2019

While Android does report touch events using floating point data, one pixel on a typical new phone is less than 0.1mm, which is more than enough accuracy for using the touch screen. As such, I think it would be fine to just have the integral version.

@kennylevinsen
Copy link
Author

Note that touch screens are not isolated to smartphones. Tablets, large laptops, and even desktop through external touch tracking devices (IR tracking, for example) have touch input, and these usually have much larger pixels.

Furthermore, input types such as drawing tablets have resolutions along the lines of ~5000 lines per inch, making integer pixels of even a 4k monitor look like a 9x9 dot-matrix display in comparison.

Thus, I think it is sensible to have non-integral physical position for input. However, it may not need to be available immediately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C - needs discussion Direction must be ironed out S - api Design and usability S - meta Project governance
Development

No branches or pull requests

5 participants