mirror of
https://github.com/bevyengine/bevy
synced 2024-12-22 02:53:07 +00:00
44f677a38a
# Objective This PR's first aim is to fix a mistake in `HalfSpace`'s documentation. When defining a `Frustum` myself in bevy_basic_portals, I realised that the "distance" of the `HalfSpace` is not, as the current doc defines, the "distance from the origin along the normal", but actually the opposite of that. See the example I gave in this PR. This means one of two things: 1. The documentation about `HalfSpace` is wrong (it is either way because of the `n.p + d > 0` formula given later anyway, which is how it behaves, but in that formula `d` is indeed the opposite of the "distance from the origin along the normal", otherwise it should be `n.p > d`) 2. The distance is supposed to be the "distance from the origin along the normal" but when used in a Frustum it's used as the opposite, and it is a mistake 3. Same as 2, but it is somehow intended Since I think `HalfSpace` is only used for `Frustum`, and it's easier to fix documentation than code, I assumed for this PR we're in case number 1. If we're in case number 3, the documentation of `Frustum` needs to change, and in case number 2, the code needs to be fixed. While I was at it, I also : - Tried to improve the documentation for `Frustum`, `Aabb`, and `VisibilitySystems`, among others, since they're all related to `Frustum`. - Fixed documentation about frustum culling not applying to 2d objects, which is not true since https://github.com/bevyengine/bevy/pull/7885 ## Remarks and questions - What about a `HalfSpace` with an infinite distance, is it allowed and does it represents the whole space? If so it should probably be mentioned. - I referenced the `update_frusta` system in `bevy_render::view::visibility` directly instead of referencing its system set, should I reference the system set instead? It's a bit annoying since it's in 3 sets. - `visibility_propagate` is not public for some reason, I think it probably should be, but for now I only documented its system set, should I make it public? I don't think that would count as a breaking change? - Why is `Aabb` inserted by a system, with `NoFrustumCulling` as an opt-out, instead of having it inserted by default in `PbrBundle` for example and then the system calculating it when it's added? Is it because there is still no way to have an optional component inside a bundle? --------- Co-authored-by: SpecificProtagonist <vincentjunge@posteo.net> Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> |
||
|---|---|---|
| .. | ||
| src | ||
| Cargo.toml | ||