servo / euclid

Geometry primitives (basic linear algebra) for Rust
Other
463 stars 103 forks source link

Proposal: implement strongly typed units the way Gecko does #144

Closed nical closed 8 years ago

nical commented 8 years ago

Euclid currently implements strongly typed units by wrapping the Point/Rect/Matrix/etc.'s scalar type in a wrapper that implements the same type of arithmetic operations as the basic types (f32, i32, etc). I am referring to the Length and ScaleFactor types. This approach lets most of the vector and matrix types not have to deal with units, which is nice. However, I believe that the way gecko implements strongly typed units has several advantages over euclid's current approach.

I put together a little experiment here: https://github.com/nical/misc/tree/master/math_api_experiments/src

In v1.rs I implement a very small subset of an hypothetical vector math library (Matrix4x4 and Point3D structs with just a handful of methods), and in v2.rs the same feature set using euclid's approach. One of the features implemented in both cases does not currently exist in euclid: It is the ability to express that a matrix transforms points from a space to another (for example multiplying a Matrix<WorldSpace, ScreenSpace> with a Point3D would return a Point3D. Likewise multiplying matrices use the source and destination spaces so that Matrix<Foo, Bar> * Matrix<Bar, Baz> = Matrix<Foo, Baz>. I believe that this is really useful feature to have, we can come back to why I think that is if there is any disagreement there.

Back to the experiment linked above, it is interesting to compare the two implementations. My goal here being to convince people that v1.rs is the best approach, I invite you to help me the other approach as much as possible to provide a fair comparison:

Advantages of gecko's approach (v1.rs):

Advantage(s) of eculid's approach (v2.rs):

When I brought this up on irc, one interesting concern was that Gecko's approach puts the units in the vector and matrix types themselves, but some users of euclid may not want to use the strongly typed units feature. In reality for these users, the experience is exactly the same either way (see the examples in v1.rs and v2.rs). Since rust has default generic parameters, using Point3D is equivalent to Point3D<f32, Untyped> and thus the API is identical with both approaches for users of the library that don't use units.

Having hacked on Gecko for years I am pretty happy with the way Moz2D's vectors work. My experience trying to use euclid with units in my pet projects has been less pleasant (arguably I haven't used it nearly as much).

Sorry, this is a bit of a long piece. I'll be happy to help with converting euclid if you guys are interested in this.

Ms2ger commented 8 years ago

There is a number of linear algebra libraries in the Rust ecosystem; if we're going to rewrite ours, we should at least consider if we could get any of those on board.

metajack commented 8 years ago

cc @mbrubeck who wrote the original implementation in euclid

@ms2ger What are the other ecosystem competitors and what do they do?

@nical Your new approach seems fine to me at a glance. I'm slightly curious about some of the advantages of matrices knowing their input and output spaces.

The bar for changing this is probably convincing a few stake holders this is a useful improvement and then doing the conversion and updating Servo. You'd probably also want to look at https://crates.io/crates/euclid/reverse_dependencies to see what other crates using euclid are doing and if conversion of those would be straightforward.

nical commented 8 years ago

@metajack Having both input and output spaces encoded in matrices helps with using units without having to cast/transmute points and sizes manually to the new space after they are transformed into a new space. Simply put, casting points manually is a bit like the unsafe keyword: when you use it the type system doesn't have your back.

It may sound trivial but in scenarios where there's a lot of back-and-forth transformations between two spaces, it's easy to either overlook things and cast at the wrong place, or to just be annoyed by the friction that comes with having to manually cast points and decide to not use units with your vectors at all. If the transformation does the unit conversion automatically, you can manipulate vectors without thinking about spaces since the compiler will let you know when you do it wrong.

It can also help with preventing you from building a transformation that does not make sense. When multiplying two matrices, if important transformations are described with units it becomes impossible to have them applied twice (which would make no sense), even if you don't know where the matrices come from, because whether that projection has already happened is part of the type.

At the end of the day it is the same argument as having units for points and sizes, except that the latter two describe a quantity in one space so they have one unit parameter, whereas a matrix is by definition a transformation from a space to another and thus is defined by its both its source and destination space.

The part about matrices is only one example, but the main point here is that the way euclid manages units looks nice at first because it separates expressing vectors and expressing units into two separate things, but in doing so it makes dealing with typed vectors much less ergonomic than non-typed ones, and completely abstracting away the scalar type makes the implementation of the lib itself more complicated than it could be in the end (compare v1.rs with v2.rs).

nical commented 8 years ago

The reverse dependency chain looks a lot less scary than I thought it would! I know that nox is considering changing the transposition of euclid's matrices to match the web's convention (a much bigger breaking change). If that is going to happen, now is a good time to consider any other breaking change.

metajack commented 8 years ago

Once it looks like we have some consensus to do the work proposed in this issue, I suggest posting on dev-servo and contacting the reverse dependency owners to ask what other breaking changes we might need. It might make sense to version the resulting euclid as 1.0.

cc @SimonSapin for his thoughts since he just went through a similar phase with rust-url.

SimonSapin commented 8 years ago

I’ve discussed this is bit offline with nical and nox but I don’t really have an opinion.

metajack commented 8 years ago

@SimonSapin I pinged you about the publish 1.0 thing, not necessarily about the technical contents of this issue. Is it a reasonable idea, and is that a good way to do it?

SimonSapin commented 8 years ago

Ah, sorry. With 0.7.1 being the current version, as far as Cargo is concerned both 0.8.0 and 1.0.0 are fine: Cargo won’t automatically update 0.7.x users to these versions since they indicate breaking changes according to SemVer.

1.0 vs 0.x is purely about what you want to signal to humans. In my opinion there isn’t an objective criteria for which is appropriate, it’s really about how the maintainer(s) feel about the library. A redesign like this happens with experience from using the earlier versions for some time, so I think it’s a good time to signal this kind of confidence.

TL;DR: Yes. 1.0, go for it.

nical commented 8 years ago

@pcwalton told me on irc that he was OK with the approach. I would like to be sure the main interested parties are OK too before I go about writing a big patch. Who else should I talk to? cc @glennw

asajeffrey commented 8 years ago

If you need someone to bounce ideas off of, I know a bit about the design space of typed units. Like @Ms2ger said, it would be nice to know what existing linear algebra crates do, and whether we can adopt one of the pre-existing solutions.

asajeffrey commented 8 years ago

Looooooong IRC converation: http://logs.glob.uno/?c=mozilla%23servo&s=18+Jul+2016&e=18+Jul+2016#c482636

TL;DR: Safety vs ergonomics of interfacing to a software ecosystem is tricky.

brendanzab commented 8 years ago

@nical: Might I suggest implementing it, then using it in servo referencing the git branch to see if the ergonomics hold up? It would be a bit hard to make a decision without seeing it in action.

nical commented 8 years ago

I am in the process of converting euclid, I'll put something up on github soon. In the mean time you don't need to wait for euclid to be converted, you can look at Gecko's code (grep for LayoutDeviceIntPoint, CssIntPoint, ScreenIntPoint, ParentLayerIntPoint, LayerPoint, and more generally everything you see in layout/base/Units.h). What I am proposing is not new, the ergonomics would be exactly what we have had in Gecko for a long while (for points sizes etc. Gecko still lacks proper units for matrices at the moment). You can also look at my old vector math library that implements the same thing in rust (for f32 specifically, though).

Converting servo's use of units to the scheme I propose will actually not give much of a comparison, since servo almost does not use units at all (which illustrates my claim that the current ergonomics discourage devs from using units).

nical commented 8 years ago

I have units integrated in euclid class in this branch: https://github.com/nical/euclid/tree/units Code that does not use units should not be impacted (I have to actually try and verify this claim), code that wants to transition from unitless to units can just add some type aliases like type CssPoint = Point2D<f32, Css>; and refer to that instead of Point2D without changing the members are accessed (as opposed to transitioning to units with Length, in which case msot member access has to be fixed up).

The core idea is there, and we can play around with the details, such as interfacing with Length externally. For example I added fn x_typed(&self) -> Length<T, Unit> to point classes, and we can play around with adding more of these, perhaps.

Interestingly ScaleFactor and Matrix become very similar constructs semantically and in how they interact with points, which they are in theory since ScaleFactor is really just a compact representation of a scale matrix.

Also, mathematicians around will be happy to hear that some things that did not make sense in terms of units have been removed, for example a area() for a Rect<Length<Cm, f32>> was returning a Length<Cm, f32> (now it returns a f32 which is a bit raw but not wrong). I personally don't care but since a lot of the discussion so far has been about "math-ness" correctness vs ergonomics...

nical commented 8 years ago

I stupidly implemented it all on top of an old revision (thinking I was on an up to date master). Working on rebasing/re-implementing it on top of the current tip. In the mean time, any feedback is welcome. I would like to get a better idea of whether this work has a chance of being merged.

asajeffrey commented 8 years ago

@nical am I right in thinking that the choice here is whether p.x is a raw type or a type-with-units? e.g. if p is a Point2D<f32,U> then should p.x be an f32 or a Length<f32,U>?

It seems like there are two designs:

My concern about v2 is that it's going to encourage people to use unitless scalars. Is p.x.raw that bad?

nical commented 8 years ago

@asajeffrey This is part of the choice. The problem is that when using Point<Length<f32>> it is complicated to abstract over the unit. for example, expressing a function that takes a f32 vector and is generic over the unit might look like fn foo<U>(p: Point2D<Length<U, f32>>) {...} which does not work with the simple Point2D<f32> that everybody uses and is the de facto default. My proposed approach does not change the structure of the point when opting into units, because there is always a unit (be it the default one or a custom one) which makes everything simpler. Fn foo becomes fn foo<U>(p: TypedPoint2D<f32, U>) {...} and accessing the content of p will happen the same way whether or not you take a typed or an untyped vector.

The fact that currently euclid will be used differently with and without units is a problem. p.x.raw isn't that bad (actually for most people it is, but even if we pretend it isn't), it actually does not work. It does not compile with Point2D<f32>. So if I write some code using Point2D<f32> first because it is simpler to use, coming back to the code later and make it use units means fixing up most of the stuff I have written.

Projects using euclid, whether or not they use units, will decide what kind of precision they want and stick to a certain scalar type for floats and another for ints (maybe more for ints, for other reasons). Abstracting away the type of scalar (f32) gets in way. It makes APIs harder to use, and the proliferation of trait bounds for all methods that abstract over the units is discouraging.

As a data point: Gecko does not have double precision float Sizes and Points at all, in fact it only has a double precision float Rect type for legacy reasons, the last uses of which we would be happy to remove if we didn't have better things to do. Servo is no different.

Currently servo by and large does not use units. Glenn said he usually doesn't use them because they get in the way of prototyping. Gecko gained support for units not that long ago (less than 4 years ago since I am pretty sure I was already on board) and units are used a lot more.

I do understand the your concern about encouraging people to access raw members. My experience is that most of the time if you are touching a vector's member directly you want to manipulate a float and not something that is abstracted away, mostly because often what you are doing is some doing some math that was not provided by euclid itself.

If an API is designed to add maximum safety but is not used because of ergonomic issues, it does not protect anyone (This is servo's current relationship with units).

Edit: wrapped <T> stuff in code blocks, I didn't realize github was stripping them out otherwise.

asajeffrey commented 8 years ago

Yes, being generic over units is a bit painful, you end up with something like:

   fn foo<T>(p: Point2D<T>) where T: ... { ... }

but the constraints T:... end up being quite cumbersome, especially if you end up requiring T: To<f32> + From<f32>, and your code has lots of explicit casts.

We could try to get round this with traits, e.g.

  trait Pointy2D<T> {
     fn x(self) : T;
     fn y(self) : T;
     ...
  }

with implementations like:

  impl<T> Pointy2D<T> for Point2D<T> { ... }
  impl<T,U> Pointy2D<T> for Point2D<Length<U,T>> { ... }

so you can write, e.g.:

  fn foo<P>(p: P) where P: Pointy2D<f32> { ... }

but these kinds of overlapping implementations often cause problems for type inference, sigh.

nical commented 8 years ago

I doubt most people would use vectors as traits when they can simply use a concrete Point2D type. But traits for vectors is something that's been on my mind for some time could be useful for other things (like write simple libs that work on top of euclid, cgmath, etc. or write glue between libs without depending on all of them if everybody agrees on a common set of traits). It's a separate discussion I would love to have with the implementors of the major math crates. It doesn't change the question of units, though.

asajeffrey commented 8 years ago

Well, if we had traits for vectors, then the code for both accesses would be p.x() and then the type inference system would work out if p was being used as a Pointy2D<f32> or a Pointy2D<Length<U,f32>>. It does mean that you need to write p.x() rather than p.x though.

nical commented 8 years ago

It not only means that you have to write *p.x() or p.x(); p.set_x(1.0), but also that you force all of your functions manipulating vectors to be generic with a trait bound. When maintaining something a tad complex people won't choose that over a simple unit-less Point2D. What I have in mind for traits is libraries that are either simple or barely don't use vectors (for example some simple bezier math or a crate that only take vectors to forward them to, say, opengl) without having depend on a particular or several vector libraries. I would be willing to make different tradeoffs for simple self-contained libraries like that, than for complex systems (like WebRender, layout, or a path tessellator, which do a lot of complicated vector maths). The traits could also help with making the glue between vector libraries if you end up depending on several of them through indirect dependencies.

It solves a different set of problems. I want units in complex code that could benefit from stronger typing, and this kind of code can't afford to be harder to read and maintain (even by the tiniest margin).

nical commented 8 years ago

I rebased the units branch on top of the tip of euclid's master. The rebase was a bit brutal and the commits don't build independently (need to apply them all) which is not great, I'll look into cleaning the history up (my git-jutsu isn't so good).

nical commented 8 years ago

rust-layers adapted to the proposed units system: https://github.com/nical/rust-layers/commit/1e493d5d0ae229cd9fe0ddb42d0f298941a7b2ff There is room for more "unitness" in there (I left matrices unit-less as they were already). This commit just contains the minimal amount changes (almost).

nical commented 8 years ago

Landed in commit 62051044a723bcd1d3817169734960ca28e6500b