rustgd / cgmath

A linear algebra and mathematics library for computer graphics.
https://docs.rs/cgmath
Apache License 2.0
1.13k stars 155 forks source link

Release 0.18? #499

Closed aloucks closed 3 years ago

aloucks commented 4 years ago

Would it be possible to see a new release sometime soon? I'm trying to trim down dependencies and the rand crate was made optional about a year ago. It would be nice to exclude that when it's not needed.

kvark commented 4 years ago

Looks like cgmath needs a maintainer. I've not been doing any math stuff for ~ year, so I don't feed incentivized invest time in it right now. Would you want to help? We'd need:

  1. a pass through the issues to see which of them need to be addressed for 0.18
  2. a pass through the current dependencies, considering removal or bumps
  3. a pass through the API to see which parts need to be updated
aloucks commented 4 years ago

Would you want to help?

Yes.

All of the dependencies on master are now up-to-date. However, I'd like to see a new release of mint that includes https://github.com/kvark/mint/pull/50 (and we'd update cgmath Quaternion to match).

The simd crate is questionable at this point, but it's an optional nightly-only feature. I'd like to see this updated but I'm not sure it's something I'd be comfortable trying to take on right now.

I did a quick scan of the issues and saw a few that I might be good candidates for fixing. I'll take a closer look this evening or later in the week.

kvark commented 4 years ago

See/review https://github.com/kvark/mint/pull/53 please

kvark commented 4 years ago

@aloucks mint-0.5.5 is up

aloucks commented 4 years ago

@kvark I scanned through the issues and these are the ones I'm thinking are good candidates.

These issues are all related to the "handedness" of various functions. I'd like to resolve and clarify them once and for all by adding _rh and _lh variants to all things with "handedness":

https://github.com/rustgd/cgmath/issues/448 https://github.com/rustgd/cgmath/issues/390 https://github.com/rustgd/cgmath/issues/350

Matrix3::look_at_rh
Matrix3::look_at_lh
Matrix4::look_at_rh
Matrix4::look_at_lh
Decomposed::look_at_rh
Decomposed::look_at_lh
perspective_rh
perspective_lh

Nice to have:

Non-uniform scale for Decomposed: https://github.com/rustgd/cgmath/issues/434

Low hanging fruit:

Slerp result needs normalized: https://github.com/rustgd/cgmath/issues/498

Matrix3::from_translation: https://github.com/rustgd/cgmath/issues/484

normalize can produce vector of NaN (add a note in the documentation that the result needs normalized if the magnitude > 0): https://github.com/rustgd/cgmath/issues/450

Run rustfmt on project: https://github.com/rustgd/cgmath/issues/415

Doc formatting (is this still an issue?) https://github.com/rustgd/cgmath/issues/373

kvark commented 4 years ago

Handedness proposal looks good to me.

434 is something we perhaps not need at the moment.

kvark commented 4 years ago

Actually, I think it could be best to have enum Handedness, since this would make up for less entry points, and internally cgmath would do something like that anyway.

Also, there may need to be a enumeration of depth for perspective generation:

enum Depth {
  Signed, // OpenGL style, -1 to 1
  Unsigned, // D3D style, 0 to 1
  UnsignedReverse, // reverse-depth, 0 to 1
}
kvark commented 4 years ago

Looks like #501 should also be addressed before we release

nstoddard commented 4 years ago

Would it be possible to release 0.18 fairly soon, with relatively minimal fixes for issues like #501, putting off larger changes (like the proposal to eliminate Point entirely) until 0.19? I'd like to stop depending on the git version of cgmath, but some of my code depends on new features.

I might be able to help if there's just a few things that need to be done before release.

kvark commented 4 years ago

Yes, I think it's a good idea! So we are only blocked on #501 atm

aloucks commented 4 years ago

I've submitted #508 for feedback on look_at_[lh|rh]. I don't think a Handedness enum is the right path forward.

hayashi-stl commented 4 years ago

Non-uniform scale on Decomposed would be tricky because it would be possible to concat two Decomposeds into a transform that can't be represented losslessly by a Decomposed.

kvark commented 4 years ago

@josh65536 can you explain this, please? My understanding is - if you have any sequence of uniform scales, translations, and rotations, you aren't squeezing the object improportionally, so there is an equivalent Decomposed for it, losslessly.

hayashi-stl commented 4 years ago

I said non-uniform scale.

kvark commented 4 years ago

ok, for non-uniform scale, could you provide an example where you wouldn't be able to combine transformations?

hayashi-stl commented 4 years ago

Imagine scale in Decomposed was a V instead of V::Scalar. Then

let s = Decomposed {
    scale: vec2(2.0, 1.0),
    ..Decomposed::one()
};
let r = Decomposed {
    rot: Rotation2::from_angle(Deg(45.0)),
    ..Decomposed::one()
};
let diamond = s.concat(r);

The equivalent Matrix3 for diamond should be

Matrix3::new(
     2.0f64.sqrt(), 0.5f64.sqrt(), 0.0,
    -2.0f64.sqrt(), 0.5f64.sqrt(), 0.0,
     0.0,           0.0,           1.0,
)

but no single Decomposed would be able to represent that. Because scale is applied before rotation in a single Decomposed (if you assume scale applies after rotation, you could make a similar argument with a different invariant), the basis vectors stay orthogonal, which they are not in the matrix.

kvark commented 4 years ago

I see. So the concat would have to return a result or something, which is highly inconvenient. How about we force Decomposed to always have uniform scale then?

hayashi-stl commented 4 years ago

That's the current status. I'm just responding to the request for non-uniform scale.

nstoddard commented 3 years ago

Any updates on this? Is something still blocking 0.18?

kvark commented 3 years ago

Looks like progress got stale. There is a few issues in the list of https://github.com/rustgd/cgmath/issues/499#issuecomment-613201222 that are still unresolved, including this non-uniform scale bug @josh65536 brought up. @aloucks how do you feel about the release?

aloucks commented 3 years ago

@kvark There were some unresolved questions regarding Matrix2::look_at in #508 and #514. I had thought that #508 would be merged as-is and then work would continue but it was never merged. The quaternion memory layout change in #500 was also never merged.

508 was approved and in a good state but had some unfinished pieces (the Rotation trait had not yet been updated and there were questions around Matrix2). #514 started to build on that PR but I don't think some of those changes are quite right and may intro new function names that will end up being deprecated again.

I'd like to see both #500 and #508 merged, as they should be future-compatible, but I don't think it should hold up a release.

kvark commented 3 years ago

Please rebase #508 (it's not landable right now), and we'll proceed.

aloucks commented 3 years ago

@kvark It's been rebased now.

kvark commented 3 years ago

Looks like we got all the changes for the release now, thank you! One last thing I wish we have is moving the CI to GHA, since we are going to branch out on release, and I don't want to go through this Travis pain again whenever we need to back-port changes. Would somebody be willing to contribute this and move us to Github Actions?

aloucks commented 3 years ago

@kvark I've added github workflows in #519.

kvark commented 3 years ago

https://crates.io/crates/cgmath/0.18.0 almost a year since 0.17!