Skip to content

Make accessing members as lengths more ergonomic. #206

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

Closed
wants to merge 1 commit into from

Conversation

nical
Copy link
Contributor

@nical nical commented Jun 5, 2017

We don't ever use the _typed versions of euclid methods which work with Length<T, U> instead of T. I believe that this is for a large part because it is simpler and more ergonomic to work directly with the scalar type (say, floats) and access it through shorter notations (foo.x vs foo.x_typed()).

We can certainly improve on the ergonomics of manipulating lengths:

  • Public members (say foo.x) should have very short getters for lengths (foo.x() instead of foo.x_typed())
  • If there is an ambiguity between "direct" and the length-based versions of a method, use the prefix get_ instead of the the suffix _typed (for example 'rect.get_min_x()instead ofrect.min_x_typed()sincerect.min_x()` already exists for the non-length version)
  • Methods that take the scalar type T as parameter (for example rect.inflate) can take T2: Into<Length<T, U>> which works with both T and Length<T, U>.
  • Static methods (such as ::new) are more troublesome, because if we make them generic like regular methods, the compiler can easily run into cases where it does not know which type to infer if the static method is called in generic code. Non-static methods don't usually have this problem because the self part of the method carries enough information for the compiler to know what to infer.

While this is technically a breaking change (some function names change), it breaks nothing since these methods aren't ever used a far as I can tell, perhaps we could get away without the huge pain that bumping major versions of euclid is...


This change is Reviewable

@nical
Copy link
Contributor Author

nical commented Jun 5, 2017

An alternative is to use the get_x() notation even when x() is available, for the sake of consistency. I still prefer to have the shortest/nicest way possible for x, y and z, at least because they are so common and I think that the tiniest bit of extra ergonomics will make a difference in we use them at all. But if others feel strongly the other way, I'll yield.

r? @kvark

@bors-servo
Copy link
Contributor

☔ The latest upstream changes (presumably #207) made this pull request unmergeable. Please resolve the merge conflicts.

@kvark
Copy link
Member

kvark commented Jun 5, 2017

@nical I feel like accessing the untyped components (currently, x, y, z) should be opt-in, since it ignores our type protection. Did you consider either storing x, y, z as Length or hiding them and exposing x() versus x_raw() getters?

@nical
Copy link
Contributor Author

nical commented Jun 6, 2017

Accessing the untyped component is the most ergonomic way and I doubt we'll manage to make working with members as nice as plainly accessing the underlying f32. I prefer not to compromise on ergonomics for this specifically because any small annoyance adds up very quickly when you write even simple vector math computations.

So, for now I would much rather keep untyped member access easy and simple, and work on making typed member access more ergonomic. If we get to a point where typed member access is (almost) as nice and natural to use, and servo (and other notable euclid users) mostly uses that painlessly, we can look into making raw access opt-in.

@kvark
Copy link
Member

kvark commented Jun 6, 2017

@nical I see your point. We have to draw the line somewhere between types and ergonomics, and you think that fields should be on the latter side.
Out of curiosity, just to understand the use cases better, what portion of the users does indeed need to access untyped fields? Part of those may be amortized by converting to fixed-size arrays (or to mint types).

@nical
Copy link
Contributor Author

nical commented Jun 6, 2017

I don't know how to give you numbers or even precise pointers but while converting webrender and servo to euclid 0.14 I ran into quite a lot of code that does non-trivial math with the point/vector members (more than I expected). Another example is my own code in the lyon crates that tend to do have a lot of that as well.

There are places where mint would be nice in servo, but the places where we do a lot of math is typically where I think we should keep euclid and push towards more strongly typed units (In particular our layout code could use a lot more strong typing).

@kvark
Copy link
Member

kvark commented Jun 6, 2017

@nical I like x() instead of x_typed, but I don't feel as positive about get_max_x() instead of max_x_typed(). Perhaps, we should leave the _typed suffix for cases where original methods already return untyped stuff? Alternatively, have max_x() return typed stuff (consistency!) and max_x_raw() returning untyped. Actually, I like that most of all.

@bors-servo
Copy link
Contributor

☔ The latest upstream changes (presumably #225) made this pull request unmergeable. Please resolve the merge conflicts.

@nical
Copy link
Contributor Author

nical commented Jan 18, 2018

I'll close this for now, I'll probably come back a some point and extract the least controversial (and non-breaking) parts of this PR.

@nical nical closed this Jan 18, 2018
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.

3 participants