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

Release 0.18? #499

Closed
aloucks opened this issue Apr 13, 2020 · 26 comments · Fixed by #520
Closed

Release 0.18? #499

aloucks opened this issue Apr 13, 2020 · 26 comments · Fixed by #520
Labels

Comments

@aloucks
Copy link
Collaborator

aloucks commented Apr 13, 2020

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 kvark added the question label Apr 13, 2020
@kvark
Copy link
Collaborator

kvark commented Apr 13, 2020

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
Copy link
Collaborator Author

aloucks commented Apr 13, 2020

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 kvark/mint#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
Copy link
Collaborator

kvark commented Apr 13, 2020

See/review kvark/mint#53 please

@kvark
Copy link
Collaborator

kvark commented Apr 13, 2020

@aloucks mint-0.5.5 is up

@aloucks
Copy link
Collaborator Author

aloucks commented Apr 14, 2020

@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":

#448
#390
#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:
#434

Low hanging fruit:

Slerp result needs normalized:
#498

Matrix3::from_translation:
#484

normalize can produce vector of NaN (add a note in the documentation that the result needs normalized if the magnitude > 0):
#450

Run rustfmt on project:
#415

Doc formatting (is this still an issue?)
#373

@kvark
Copy link
Collaborator

kvark commented Apr 19, 2020

Handedness proposal looks good to me.
#434 is something we perhaps not need at the moment.

@kvark
Copy link
Collaborator

kvark commented Apr 19, 2020

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
Copy link
Collaborator

kvark commented May 15, 2020

Looks like #501 should also be addressed before we release

@nstoddard
Copy link
Contributor

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
Copy link
Collaborator

kvark commented Jun 3, 2020

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

@aloucks
Copy link
Collaborator Author

aloucks commented Jun 8, 2020

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
Copy link
Contributor

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
Copy link
Collaborator

kvark commented Aug 12, 2020

@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
Copy link
Contributor

I said non-uniform scale.

@kvark
Copy link
Collaborator

kvark commented Aug 12, 2020

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

@hayashi-stl
Copy link
Contributor

hayashi-stl commented Aug 12, 2020

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
Copy link
Collaborator

kvark commented Aug 12, 2020

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
Copy link
Contributor

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

@nstoddard
Copy link
Contributor

Any updates on this? Is something still blocking 0.18?

@kvark
Copy link
Collaborator

kvark commented Nov 20, 2020

Looks like progress got stale. There is a few issues in the list of #499 (comment) that are still unresolved, including this non-uniform scale bug @josh65536 brought up. @aloucks how do you feel about the release?

@aloucks
Copy link
Collaborator Author

aloucks commented Nov 22, 2020

@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
Copy link
Collaborator

kvark commented Nov 23, 2020

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

@aloucks
Copy link
Collaborator Author

aloucks commented Nov 25, 2020

@kvark It's been rebased now.

@kvark
Copy link
Collaborator

kvark commented Dec 5, 2020

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
Copy link
Collaborator Author

aloucks commented Dec 5, 2020

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

@kvark kvark mentioned this issue Dec 5, 2020
@kvark kvark closed this as completed in #520 Jan 3, 2021
@kvark
Copy link
Collaborator

kvark commented Jan 3, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants