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

feature: Add an into() bevy mesh, example viewer. #76

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

shanecelis
Copy link

Add a cargo feature "into_bevy_mesh" that will provide a From implementation so generators like Cube can be turned into a bevy Mesh. This doesn't add any dependencies on the default features. It does add dependencies on bevy_render and glam when the feature is enabled.

Add an example called "viewer" that uses this feature to view the generators. It's a small bevy app with some camera control. I did discover a bug in a cone normal so plus one for visualization.

Add a cargo feature "into_bevy_mesh" that will provide a `From`
implementation so generators like `Cube` can be turned into a bevy `Mesh`.
This doesn't add any dependencies on the default features. It does add
dependencies on `bevy_render` and `glam` when the feature is enabled.

Add an example called "viewer" that uses this feature to view the
generators. It's a small bevy app with some camera control. I did
discover a bug in a cone normal so plus one for visualization.
@shanecelis
Copy link
Author

Here are some screenshots of the viewer and the cone with and without the normal fix.

cone-normal-bug
cone-normal-fix

@kvark
Copy link
Member

kvark commented Apr 28, 2023

Thank you for the PR!
I think this functionality is useful, but it should probably not live in genmesh itself. Problem is genmesh depending on bevy. This problem is two-fold:

  1. Bevy should be a consumer of genmesh, and thus depend on it, not the other way around
  2. Bevy version becomes a part of genmesh public API. E.g. once Bevy-0.11 is released, this code in genmesh becomes dead weight, and only pushes genmesh to publish another breaking release, which we'd want as little as possible.

@shanecelis
Copy link
Author

Thanks for looking it over. I'll consider adding a PR to bevy with a "genmesh" feature. It's really the orphan rules that force this kind of feature to live in genmesh or bevy, but I agree bevy would be the right place for it to live.

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.

2 participants