Skip to content

Meshing for Annulus primitive #12734

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

Merged
merged 4 commits into from
Apr 1, 2024
Merged

Conversation

mweatherley
Copy link
Contributor

@mweatherley mweatherley commented Mar 26, 2024

Objective

Related to #10572
Allow the Annulus primitive to be meshed.

Solution

We introduce a Meshable structure, AnnulusMeshBuilder, which allows the Annulus primitive to be meshed, leaving optional configuration of the number of angular sudivisions to the user. Here is a picture of the annulus's UV-mapping:
Screenshot 2024-03-26 at 10 39 48 AM

Other features are essentially identical to the implementations for Circle/Ellipse.


Changelog

  • Introduced AnnulusMeshBuilder
  • Implemented Meshable for Annulus with Output = AnnulusMeshBuilder
  • Implemented From<Annulus> and From<AnnulusMeshBuilder> for Mesh
  • Added impl_reflect! declaration for Annulus and Triangle3d in bevy_reflect

Discussion

Design considerations

The only interesting wrinkle here is that the existing UV-mapping of Ellipse (and hence of Circle and RegularPolygon) is non-radial (it's skew-free, created by situating the mesh in a bounding rectangle), so the UV-mapping of Annulus doesn't limit to that of Circle as its inner radius tends to zero, for instance. I don't see this as a real issue for Annulus, which should almost certainly have this kind of UV-mapping, but I think we ought to at least consider allowing mesh configuration for Circle/Ellipse that performs radial UV-mapping instead. (In these cases in particular, it would be especially easy, since we wouldn't need a different parameter set in the builder.)

@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-Rendering Drawing game state to the screen A-Math Fundamental domain-agnostic mathematical operations M-Needs-Release-Note Work that should be called out in the blog due to impact labels Mar 26, 2024
Copy link
Contributor

You added a new example but didn't add metadata for it. Please update the root Cargo.toml file.

Copy link
Contributor

@Jondolf Jondolf left a comment

Choose a reason for hiding this comment

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

CI is screaming but it seems to be for unrelated reasons (as usual). LGTM!

I agree it'd be nice to add radial/polar UV mapping as a configuration option for Circle and Ellipse (and RegularPolygon?) in a follow-up.

@alice-i-cecile alice-i-cecile added this to the 0.14 milestone Mar 26, 2024
Copy link
Contributor

@Chubercik Chubercik left a comment

Choose a reason for hiding this comment

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

LGTM ^^

@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Mar 27, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Apr 1, 2024
Merged via the queue into bevyengine:main with commit c8aa3ac Apr 1, 2024
@mweatherley mweatherley deleted the annulus-mesh branch April 2, 2024 00:27
github-merge-queue bot pushed a commit that referenced this pull request Apr 2, 2024
# Objective

- Depends on #12734.

Since adding the `Annulus` primitive shape (#12706, #12734), the
`2d_shapes` example has become outdated.

## Solution

This PR adds the annulus shape to the `2d_shapes` example:


![image](https://github.com/bevyengine/bevy/assets/37378746/e620362d-bec6-4660-bf6e-d70babff8179)

---

## Changelog

### Added

- `Annulus` shape to the `2d_shapes` example

(~~as an added bonus, the example now features Newton's
[ROYGBIV](https://en.wikipedia.org/wiki/ROYGBIV) rainbow palette ^^~~ no
it doesn't, but one can shoehorn..)
chompaa pushed a commit to chompaa/bevy that referenced this pull request Apr 5, 2024
# Objective

Related to bevyengine#10572 
Allow the `Annulus` primitive to be meshed.

## Solution

We introduce a `Meshable` structure, `AnnulusMeshBuilder`, which allows
the `Annulus` primitive to be meshed, leaving optional configuration of
the number of angular sudivisions to the user. Here is a picture of the
annulus's UV-mapping:
<img width="1440" alt="Screenshot 2024-03-26 at 10 39 48 AM"
src="https://github.com/bevyengine/bevy/assets/2975848/b170291d-cba7-441b-90ee-2ad6841eaedb">

Other features are essentially identical to the implementations for
`Circle`/`Ellipse`.

---

## Changelog

- Introduced `AnnulusMeshBuilder`
- Implemented `Meshable` for `Annulus` with `Output =
AnnulusMeshBuilder`
- Implemented `From<Annulus>` and `From<AnnulusMeshBuilder>` for `Mesh`
- Added `impl_reflect!` declaration for `Annulus` and `Triangle3d` in
`bevy_reflect`

---

## Discussion

### Design considerations

The only interesting wrinkle here is that the existing UV-mapping of
`Ellipse` (and hence of `Circle` and `RegularPolygon`) is non-radial
(it's skew-free, created by situating the mesh in a bounding rectangle),
so the UV-mapping of `Annulus` doesn't limit to that of `Circle` as its
inner radius tends to zero, for instance. I don't see this as a real
issue for `Annulus`, which should almost certainly have this kind of
UV-mapping, but I think we ought to at least consider allowing mesh
configuration for `Circle`/`Ellipse` that performs radial UV-mapping
instead. (In these cases in particular, it would be especially easy,
since we wouldn't need a different parameter set in the builder.)

---------

Co-authored-by: Alice Cecile <[email protected]>
chompaa pushed a commit to chompaa/bevy that referenced this pull request Apr 5, 2024
# Objective

- Depends on bevyengine#12734.

Since adding the `Annulus` primitive shape (bevyengine#12706, bevyengine#12734), the
`2d_shapes` example has become outdated.

## Solution

This PR adds the annulus shape to the `2d_shapes` example:


![image](https://github.com/bevyengine/bevy/assets/37378746/e620362d-bec6-4660-bf6e-d70babff8179)

---

## Changelog

### Added

- `Annulus` shape to the `2d_shapes` example

(~~as an added bonus, the example now features Newton's
[ROYGBIV](https://en.wikipedia.org/wiki/ROYGBIV) rainbow palette ^^~~ no
it doesn't, but one can shoehorn..)
@alice-i-cecile alice-i-cecile removed the M-Needs-Release-Note Work that should be called out in the blog due to impact label May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Math Fundamental domain-agnostic mathematical operations A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants