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

Add VertexMember implementations for cgmath types #1326

Closed
wants to merge 2 commits into from
Closed

Add VertexMember implementations for cgmath types #1326

wants to merge 2 commits into from

Conversation

jmi2k
Copy link
Contributor

@jmi2k jmi2k commented Feb 15, 2020

  • Added an entry to CHANGELOG_VULKANO.md or CHANGELOG_VK_SYS.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes - in this repository
  • Updated documentation to reflect any user-facing changes - PR to the guide that fixes existing documentation invalidated by this PR.

Still very WIP, it fulfills the same needs of #1133. I've implemented it only for cgmath because it's what I'm using currently, but I'd have no problem extending this to, for example, nalgebra types.

The former PR replaces vertex members of the examples with types from cgmath. I think they should be self-contained, and should run without more external dependencies, so I kept them as-is.

@AustinJ235 AustinJ235 added this to the 0.18.1 milestone Mar 12, 2020
@louiidev
Copy link

this would be great for https://github.com/rustsim/nalgebra as well

@jmi2k
Copy link
Contributor Author

jmi2k commented Apr 11, 2020

I have no prior experience with nalgebra, but I'll give it a try next week. I was waiting for #1327 to be merged, but it depends on a Rust issue to be solved first. I'll go on with this PR, and hold the other back until there is a reasonable way of implementing it without resorting to undefined behavior.

@louiidev
Copy link

@jmi2k that's fine, I think getting cgmath would be a huge win for now anyway

@jmi2k
Copy link
Contributor Author

jmi2k commented Apr 18, 2020

It seems that nalgebra types can have various storage types, and their layout is undefined: there is no restriction on their contents (they can be structs with metadata) or their shape (they can be non-contiguous, or not ordered).

I believe they have a subset of storage types which can guarantee being only a wrapper of primitive types, and the implementation could be parametized, but I don't have any prior experience with nalgebra and their type-level machinery is a bit convoluted, so I'll leave it out of this PR. @louisgjohnson open an issue once this is merged, nalgebra looks very complete and packed with useful features, definitely worth supporting. The nalgebra-specific code from #1133 can be extracted as a separate PR, I don't know how far did it get.

@AustinJ235
Copy link
Member

I am not personally sure how well maintained cgmath is. I have switched my projects over to nalgebra personally. cgmath last release was in January of 2019.

@AustinJ235
Copy link
Member

I think I am not going to go through with this pull request due to cgmath being a bit out of date. I don't know if you want to open another for nalgebra or use this one, up to you. We should also remove cgmath use from the examples and replace it with the more favorable nalgebra.

@AustinJ235 AustinJ235 removed this from the 0.18.1 milestone May 8, 2020
@AustinJ235
Copy link
Member

Although support for other libraries is nice to have, sadly cgmath seems to have gone inactive and this would bring in soon to be dead code to the project that will end up being removed.

I am open to this if you decide to implement nalgebra types, but since this pull request changes seem all related to cgmath I am closing this one. You are welcome to implement this for nalegbra and that'll likey go through. nalgebra seems to be the way forward.

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

Successfully merging this pull request may close these issues.

3 participants