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

Mint flavour integration #418

Merged
merged 3 commits into from
Jul 30, 2017
Merged

Mint flavour integration #418

merged 3 commits into from
Jul 30, 2017

Conversation

kvark
Copy link
Collaborator

@kvark kvark commented Jun 6, 2017

Includes #417
r? @brendanzab

Notice how matrix and Euler conversions have stronger types (to ColumnMatrix*<> and EulerAngles<, IntraXYZ> respectively).

@brendanzab
Copy link
Collaborator

Why the 👎 @tomaka? Would like to know your reasoning on this!

#[cfg(feature = "mint")]
mint_conversions!(Matrix3 { x, y, z }, ColumnMatrix3);
#[cfg(feature = "mint")]
mint_conversions!(Matrix4 { x, y, z, w }, ColumnMatrix4);
Copy link
Collaborator

Choose a reason for hiding this comment

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

It is nice that mint has a ColumnMatrix type! I think that is potentially a good distinction to make... so many folks get tripped up by it. :(

@kvark
Copy link
Collaborator Author

kvark commented Jul 1, 2017

@brendanzab if you don't mind some reading, here is the IRC discussion: https://botbot.me/mozilla/rust-gamedev/2017-06-14/?msg=87250505&page=4

@brendanzab
Copy link
Collaborator

Cheers, that is very helpful 🤔 ... will have to consider this. What does @sebcrozet think?

@sebcrozet
Copy link
Contributor

@brendanzab I think something like mint, that simplifies the task of converting between types of different linear algebra crates, is useful, especially when one user wants to use one crate depending on, e.g., nalgebra, and another built with, e.g., cgmath.

On the other hand, I am against the use of mint on APIs, i.e., it should not be a public dependency: this would force the user to add boilerplate for conversions. For example, let say I rewrite the nphysics public API to expose only mint types instead of nalgebra types (though nalgebra is still used internally). Let say there is a method that returns a vector of rigid bodies positions (using homogeneous coordinates) Vec<ColumnMatrix4<f64>>. Because no operation are defined on mint types, the user will necessarily have to convert this back to nalgebra or cgmath, (or any other crate) types before doing anything useful (besides reading their components). There is no point in adding this kind of boilerplate, especially if this user needs only crates based on only one linear algebra crate.

So I think mint is useful for converting types locally, when needed, but not to be used on API directly.

@kvark
Copy link
Collaborator Author

kvark commented Jul 5, 2017

@sebcrozet thanks for chiming in and expressing your opinion/concerns!

So I think mint is useful for converting types locally, when needed, but not to be used on API directly.

This is quite opposite to the original goal of mint - to be the public facing API.

Because no operation are defined on mint types, the user will necessarily have to convert this back to nalgebra or cgmath, (or any other crate) types before doing anything useful (besides reading their components).

I'd argue that when you want to do anything with an array of matrices, the conversion cost, if any, can be neglected.

this would force the user to add boilerplate for conversions

Given mint features in both cgmath and nalgebra all the boilerplate would hopefully be reduced to a few into() calls or maybe SomeType::from(mint_value) ones.

@torkleyy
Copy link
Member

torkleyy commented Jul 8, 2017

In addition to what @kvark said, I'd say that you don't have to convert always. Because as soon as you use another library, you can just pass the mint stuff, so it's less boilerplate because you don't have to convert between math libraries (or seen in another way, libraries dealing with math aren't specific to one math library, but rather have it as an internal dependency).

I don't think the conversion is more than a copy most of the time, probably except for quaternions which can be represented as (w, x, y, z) or (x, y, z, w).

@vitvakatu
Copy link

@brendanzab what's your opinion about this PR? I would like to see it merged, but I don't see any discussion lately.

@sebcrozet
Copy link
Contributor

Given mint features in both cgmath and nalgebra all the boilerplate would hopefully be reduced to a few into() calls or maybe SomeType::from(mint_value) ones.`

IMO a few into and from are already too much boilerplate for users that don't care about interop. So I'd rather let them choose explicitly whenever they want to convert between cgmath and nalgebra (or other crates).

Because as soon as you use another library, you can just pass the mint stuff, so it's less boilerplate because you don't have to convert between math libraries (or seen in another way, libraries dealing with math aren't specific to one math library, but rather have it as an internal dependency).

Yes that's less boilerplate in this specific case. But as long as we are not just passing data between libraries, this becomes much more boilerplaty. Also, this adds a lot of boilerplate for the libraries authors because he would have to systematically convert mint types to its own.

I don't think the conversion is more than a copy most of the time, probably except for quaternions which can be represented as (w, x, y, z) or (x, y, z, w).

What about more complex structure that contain a quaternion (e.g. Vec)? The library writer would have to copy/clone the whole structure every time the user calls its api?

I'm aware that the initial goal of mint is to be on public APIs. But I don't think this is the best approach and would rather use it as a conversion tool. That's a matter of taste but I find it odd to have types of a public API that serve no purpose other than being converted to other types you can actually do useful stuff with.

@vitvakatu
Copy link

IMO there is some misunderstanding between us.

IMO a few into and from are already too much boilerplate for users that don't care about interop. So I'd rather let them choose explicitly whenever they want to convert between cgmath and nalgebra (or other crates).

mint is optional feature for cgmath, users will not notice any difference. Imagine the situation - you're creator of game engine. You want to allow your users to use either nalgebra types or cgmath types as input values. How? With mint (being an optional dependency for both nalgebra and cgmath) - it's extremely simple. Just let user to pass mint types to functions and convert it inside to your type (you can use nalgebra or cgmath inside your library as well).

Yes that's less boilerplate in this specific case. But as long as we are not just passing data between libraries, this becomes much more boilerplaty. Also, this adds a lot of boilerplate for the libraries authors because he would have to systematically convert mint types to its own.

Again, mint is optional dependency. You're not forced to use it if you don't need to. I can't see any problems for libraries authors, their aren't forced to usemint (but they will use it if they want to convert types from different math libraries easily)

That's a matter of taste but I find it odd to have types of a public API that serve no purpose other than being converted to other types you can actually do useful stuff with.

Well, what is your suggestion? You're game engine developer, (like Amethyst, we have actually the same problem), so you need to allow your users to pass any math types in your API. The question is - how?

@sebcrozet
Copy link
Contributor

Well, what is your suggestion? You're game engine developer, (like Amethyst, we have actually the same problem), so you need to allow your users to pass any math types in your API. The question is - how?

An alternative is to make Amethyst just stick with one math lib and let the user convert types from other math libs (using something like mint for example). But that's just a matter of taste; I understand that having mint types on the API would avoid those conversions.

But a mint API is at the cost of requiring the user to convert mint type back to cgmath/nalgebra/etc. whenever he wants to do any computation with them outside of Amethyst.

By the way, none of my arguments are against the merge of this PR. I'm just discussing about the use of mint types on public APIs.

@vitvakatu
Copy link

@sebcrozet I see your point.

But a mint API is at the cost of requiring the user to convert mint type back to cgmath/nalgebra/etc. whenever he wants to do any computation with them outside of Amethyst.

Yes, that is the downside of this approach. But I'm sure compiler can (and would) optimize all those conversations well, most of them aren't more than simple conversion OneType(x, y) -> AnotherType(x, y)

@kvark
Copy link
Collaborator Author

kvark commented Jul 25, 2017

FWIW, I made a nalgebra PR here - dimforge/nalgebra#270
All major math libs now have PRs pending.

@milibopp
Copy link
Contributor

milibopp commented Jul 26, 2017

Since the discussion is going on in here, I am just going to voice my doubts here to keep in one place. I am not sure, what use case mint is trying to solve.

You are trying to provide a common language for APIs dealing with linear algebra in terms of data structures. Why do we need a new crate for this instead of just using plain old Rust arrays? If all things would be equal between those two approaches, I would much rather opt for not having to interface with a new crate, because it is simpler and therefore easier to maintain.

Both nalgebra and cgmath implement the conversion traits for them already, that easily convert these types (which should effectively be no-ops most of the time). As a library author, who wants to support both, I can just use arrays in the public API. For internal computations, I can then convert those into the data structures of the library I want to use. This seems to me the same you propose to do with mint.

Am I missing something here?

edit: reading up on the IRC discussion, I mostly agree with @tomaka on the issue of adding dependencies. writing a bit of boilerplate here and there is usually an acceptable price for flatter dependency trees. mostly that is, because it is trivial to maintain, if it is under your control, but much more of a hassle, if it is an external dependency.

@kvark
Copy link
Collaborator Author

kvark commented Jul 26, 2017

@aepsil0n thanks for chiming in!

Mint has been rigorously discussed on IRC and elsewhere, but I don't mind re-iterating some of the arguments here.

Why do we need a new crate for this instead of just using plain old Rust arrays?

The most important thing about interop between libraries/users is preservation of semantics:

  • [f32; 4] can be a vector, or a quaternion laid out as "sxyz", or as "xyzs"
  • [[f32; 2]; 2] can be a column-major or a row-major matrix
  • [f32; 3] can be a vector, or a set of Euler coordinates in one of the 12 possible interpretations... (even nalgebra gets confusing API there - from_euler_angles has wrong documentation? dimforge/nalgebra#269)

Both nalgebra and cgmath implement the conversion traits for them already

You don't have any guarantee that these libraries would agree on how exactly they represent their types in fixed-size arrays.

As a library author, who wants to support both

Increased maintenance burden is a valid concern, even though there is not much code. I do see more value in semantically strong mint conversions than fixed-array ones though, especially since mint -> fixed-arrays is available.

Having a separate crate dependency in Rust is really easy, as long as it doesn't have much logic in it, thus compiling fast. Users don't even need to use mint, they can just use their math library of choice with enabled mint feature.

@milibopp
Copy link
Contributor

@kvark thanks for the explanation, I have not been following IRC much, but only read up on the discussion between @tomaka and you. Thus the summary is much appreciated. :)

Boiling it down to one sentence, mint is about having a common set of semantically refined types for linear algebra up to 4D. That makes sense as a motivation and I can see how that would relieve some mental burden when working with multiple libraries. Actually I would love to see some end users comment on this, to get an idea about this is a common use case.

I am still not entirely convinced that the benefits are worth the cost of having an extra dependency. Apparently we have a different intuitive estimate of that cost and I guess it is hard to argue about it, since it involves speculation about uncertain data and future developments. I guess, my worry that this might be a very theoretical issue and 99% of applications are going use exactly one library at the end.

Nonetheless, if there is demand for this, I would not object to adding it as an optional feature like in your PR. This way the stability of the library without the feature would not be affected.

One other thought: do the orphan rules prevent mint to provide the conversion implementations on its own (one feature per library)? If that were possible, this discussion would be much easier, as no code would be required in the math libraries and potential "dependency hell"-type issues would be limited to those use cases, where the generality is actually required.

Compared to something like serde having the dependency that way around would actually be okay, as it is specifically designed to be a compatibility layer on top of actual implementations instead of providing its own algorithms.

@kvark
Copy link
Collaborator Author

kvark commented Jul 26, 2017

@aepsil0n I'm glad to hear that mint started to make some sense to you :)

I am still not entirely convinced that the benefits are worth the cost of having an extra dependency.

Good thing is - mint is not tightly integrated. Removing support for it, if needed eventually, or declaring mint feature unstable/unsupported is really trivial.

One other thought: do the orphan rules prevent mint to provide the conversion implementations on its own (one feature per library)? If that were possible, this discussion would be much easier, as no code would be required in the math libraries and potential "dependency hell"-type issues would be limited to those use cases, where the generality is actually required.

I don't think this is going to work. We don't want mint to be required for an update once a new breaking version of cgmath/nalgebra comes out, and I'm not even sure how to simultaneously support conversions to multiple versions of the same library.... Ideally, the currently published mint-0.4.2 is all we need for a long time.

@milibopp
Copy link
Contributor

I don't think this is going to work. We don't want mint to be required for an update once a new breaking version of cgmath/nalgebra comes out, and I'm not even sure how to simultaneously support conversions to multiple versions of the same library.... Ideally, the currently published mint-0.4.2 is all we need for a long time.

True. I did not think that through.

If that's really all we need for a long time, I guess it will turn into 1.0 comparatively soon then?

@vitvakatu
Copy link

If that's really all we need for a long time, I guess it will turn into 1.0 comparatively soon then?

I think we need to solve a few issues beforehand.

@minecrawler
Copy link

minecrawler commented Jul 27, 2017

@aepsil0n

Actually I would love to see some end users comment

I think that it might be very helpful. For now, the only thing discussed was interoperability between math libraries, but that's not interesting for me at all. I want to write a game, so I will probably just roll with whatever math lib my game engine (Amethyst) uses. However, I might decide to switch out certain parts of Amethyst, or add things which it does not provide. Something like the physics engine, VR motion tracking, etc. those things might use other math libraries or expose entirely custom structures, hence I'd be forced to do the conversion. If all libraries, however, expose a Mint interface (which is easy to implement and add/remove via a feature-switch), then I can plug them together in whatever way I like without thinking much about differences and boilerplate.

Imho, it would be a big win for projects which need to put together multiple solutions to form an end-user product.

@kvark
Copy link
Collaborator Author

kvark commented Jul 27, 2017

@minecrawler here is an example of the interaction of lib - user is going to look like (from three-rs):

impl OrbitControls {
    pub fn new<P>(object: &Object, position: P, target: P) -> OrbitControlsBuilder
where P: Into<mint::Point3<f32>> {..}
}

So the user would then do:

let orbital = OrbitalControl::new(&camera,
    cgmath::Point3::new(0.0, 0.0, 1.0),
    target_position); // supposedly, also `cgmath::Point3`

As you can see, user doesn't need to know about mint, they pick a library of their choice and pass the foreign math primitives directly.

@brendanzab
Copy link
Collaborator

brendanzab commented Jul 30, 2017

This is implemented as a feature, so I'm thinking I will unblock the ecosystem for now and merge.

I also want to experiment with switching to alga for the actual traits though - I think that might be a good idea if we want to get around the costs of conversions between types in the first place. We need far better docs for that though if we want it to be accessible for more developers. I also want to add direct, forwarding methods on the base types, as discussed on #419.

@brendanzab brendanzab merged commit 343361a into rustgd:master Jul 30, 2017
@kvark kvark deleted the mint2 branch July 30, 2017 17:14
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.

7 participants