# Objective
Continue improving the user experience of our UI Node API in the
direction specified by [Bevy's Next Generation Scene / UI
System](https://github.com/bevyengine/bevy/discussions/14437)
## Solution
As specified in the document above, merge `Style` fields into `Node`,
and move "computed Node fields" into `ComputedNode` (I chose this name
over something like `ComputedNodeLayout` because it currently contains
more than just layout info. If we want to break this up / rename these
concepts, lets do that in a separate PR). `Style` has been removed.
This accomplishes a number of goals:
## Ergonomics wins
Specifying both `Node` and `Style` is now no longer required for
non-default styles
Before:
```rust
commands.spawn((
Node::default(),
Style {
width: Val::Px(100.),
..default()
},
));
```
After:
```rust
commands.spawn(Node {
width: Val::Px(100.),
..default()
});
```
## Conceptual clarity
`Style` was never a comprehensive "style sheet". It only defined "core"
style properties that all `Nodes` shared. Any "styled property" that
couldn't fit that mold had to be in a separate component. A "real" style
system would style properties _across_ components (`Node`, `Button`,
etc). We have plans to build a true style system (see the doc linked
above).
By moving the `Style` fields to `Node`, we fully embrace `Node` as the
driving concept and remove the "style system" confusion.
## Next Steps
* Consider identifying and splitting out "style properties that aren't
core to Node". This should not happen for Bevy 0.15.
---
## Migration Guide
Move any fields set on `Style` into `Node` and replace all `Style`
component usage with `Node`.
Before:
```rust
commands.spawn((
Node::default(),
Style {
width: Val::Px(100.),
..default()
},
));
```
After:
```rust
commands.spawn(Node {
width: Val::Px(100.),
..default()
});
```
For any usage of the "computed node properties" that used to live on
`Node`, use `ComputedNode` instead:
Before:
```rust
fn system(nodes: Query<&Node>) {
for node in &nodes {
let computed_size = node.size();
}
}
```
After:
```rust
fn system(computed_nodes: Query<&ComputedNode>) {
for computed_node in &computed_nodes {
let computed_size = computed_node.size();
}
}
```
# Objective
Currently text is recomputed unnecessarily on any changes to its color,
which is extremely expensive.
## Solution
Split up `TextStyle` into two separate components `TextFont` and
`TextColor`.
## Testing
I added this system to `many_buttons`:
```rust
fn set_text_colors_changed(mut colors: Query<&mut TextColor>) {
for mut text_color in colors.iter_mut() {
text_color.set_changed();
}
}
```
reports ~4fps on main, ~50fps with this PR.
## Migration Guide
`TextStyle` has been renamed to `TextFont` and its `color` field has
been moved to a separate component named `TextColor` which newtypes
`Color`.
**Ready for review. Examples migration progress: 100%.**
# Objective
- Implement https://github.com/bevyengine/bevy/discussions/15014
## Solution
This implements [cart's
proposal](https://github.com/bevyengine/bevy/discussions/15014#discussioncomment-10574459)
faithfully except for one change. I separated `TextSpan` from
`TextSpan2d` because `TextSpan` needs to require the `GhostNode`
component, which is a `bevy_ui` component only usable by UI.
Extra changes:
- Added `EntityCommands::commands_mut` that returns a mutable reference.
This is a blocker for extension methods that return something other than
`self`. Note that `sickle_ui`'s `UiBuilder::commands` returns a mutable
reference for this reason.
## Testing
- [x] Text examples all work.
---
## Showcase
TODO: showcase-worthy
## Migration Guide
TODO: very breaking
### Accessing text spans by index
Text sections are now text sections on different entities in a
hierarchy, Use the new `TextReader` and `TextWriter` system parameters
to access spans by index.
Before:
```rust
fn refresh_text(mut query: Query<&mut Text, With<TimeText>>, time: Res<Time>) {
let text = query.single_mut();
text.sections[1].value = format_time(time.elapsed());
}
```
After:
```rust
fn refresh_text(
query: Query<Entity, With<TimeText>>,
mut writer: UiTextWriter,
time: Res<Time>
) {
let entity = query.single();
*writer.text(entity, 1) = format_time(time.elapsed());
}
```
### Iterating text spans
Text spans are now entities in a hierarchy, so the new `UiTextReader`
and `UiTextWriter` system parameters provide ways to iterate that
hierarchy. The `UiTextReader::iter` method will give you a normal
iterator over spans, and `UiTextWriter::for_each` lets you visit each of
the spans.
---------
Co-authored-by: ickshonpe <david.curthoys@googlemail.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
# Objective
Several of our APIs (namely gizmos and bounding) use isometries on
current Bevy main. This is nicer than separate properties in a lot of
cases, but users have still expressed usability concerns.
One problem is that in a lot of cases, you only care about e.g.
translation, so you end up with this:
```rust
gizmos.cross_2d(
Isometry2d::from_translation(Vec2::new(-160.0, 120.0)),
12.0,
FUCHSIA,
);
```
The isometry adds quite a lot of length and verbosity, and isn't really
that relevant since only the translation is important here.
It would be nice if you could use the translation directly, and only
supply an isometry if both translation and rotation are needed. This
would make the following possible:
```rust
gizmos.cross_2d(Vec2::new(-160.0, 120.0), 12.0, FUCHSIA);
```
removing a lot of verbosity.
## Solution
Implement `From<Vec2>` and `From<Rot2>` for `Isometry2d`, and
`From<Vec3>`, `From<Vec3A>`, and `From<Quat>` for `Isometry3d`. These
are lossless conversions that fit the semantics of `From`.
This makes the proposed API possible! The methods must now simply take
an `impl Into<IsometryNd>`, and this works:
```rust
gizmos.cross_2d(Vec2::new(-160.0, 120.0), 12.0, FUCHSIA);
```
# Objective
Yet another PR for migrating stuff to required components. This time,
cameras!
## Solution
As per the [selected
proposal](https://hackmd.io/tsYID4CGRiWxzsgawzxG_g#Combined-Proposal-1-Selected),
deprecate `Camera2dBundle` and `Camera3dBundle` in favor of `Camera2d`
and `Camera3d`.
Adding a `Camera` without `Camera2d` or `Camera3d` now logs a warning,
as suggested by Cart [on
Discord](https://discord.com/channels/691052431525675048/1264881140007702558/1291506402832945273).
I would personally like cameras to work a bit differently and be split
into a few more components, to avoid some footguns and confusing
semantics, but that is more controversial, and shouldn't block this core
migration.
## Testing
I ran a few 2D and 3D examples, and tried cameras with and without
render graphs.
---
## Migration Guide
`Camera2dBundle` and `Camera3dBundle` have been deprecated in favor of
`Camera2d` and `Camera3d`. Inserting them will now also insert the other
components required by them automatically.
# Objective
- Fixes https://github.com/bevyengine/bevy/issues/14593.
## Solution
- Add `ViewportConversionError` and return it from viewport conversion
methods on Camera.
## Testing
- I successfully compiled and ran all changed examples.
## Migration Guide
The following methods on `Camera` now return a `Result` instead of an
`Option` so that they can provide more information about failures:
- `world_to_viewport`
- `world_to_viewport_with_depth`
- `viewport_to_world`
- `viewport_to_world_2d`
Call `.ok()` on the `Result` to turn it back into an `Option`, or handle
the `Result` directly.
---------
Co-authored-by: Lixou <82600264+DasLixou@users.noreply.github.com>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Zachary Harrold <zac@harrold.com.au>
# Objective
- Solves the last bullet in and closes#14319
- Make better use of the `Isometry` types
- Prevent issues like #14655
- Probably simplify and clean up a lot of code through the use of Gizmos
as well (i.e. the 3D gizmos for cylinders circles & lines don't connect
well, probably due to wrong rotations)
## Solution
- go through the `bevy_gizmos` crate and give all methods a slight
workover
## Testing
- For all the changed examples I run `git switch main && cargo rr
--example <X> && git switch <BRANCH> && cargo rr --example <X>` and
compare the visual results
- Check if all doc tests are still compiling
- Check the docs in general and update them !!!
---
## Migration Guide
The gizmos methods function signature changes as follows:
- 2D
- if it took `position` & `rotation_angle` before ->
`Isometry2d::new(position, Rot2::radians(rotation_angle))`
- if it just took `position` before ->
`Isometry2d::from_translation(position)`
- 3D
- if it took `position` & `rotation` before ->
`Isometry3d::new(position, rotation)`
- if it just took `position` before ->
`Isometry3d::from_translation(position)`
# Objective
Previously, our cubic spline constructors would produce
`CubicCurve`/`RationalCurve` output with no data when they themselves
didn't hold enough control points to produce a well-formed curve.
Attempting to sample the resulting empty "curves" (e.g. by calling
`CubicCurve::position`) would crash the program (😓).
The objectives of this PR are:
1. Ensure that the curve output of `bevy_math`'s spline constructions
are never invalid as data.
2. Provide a type-level guarantee that `CubicCurve` and `RationalCurve`
actually function as curves.
## Solution
This has a few pieces. Firstly, the curve generator traits
`CubicGenerator`, `CyclicCubicGenerator`, and `RationalGenerator` are
now fallible — they have associated error types, and the
curve-generation functions are allowed to fail:
```rust
/// Implement this on cubic splines that can generate a cubic curve from their spline parameters.
pub trait CubicGenerator<P: VectorSpace> {
/// An error type indicating why construction might fail.
type Error;
/// Build a [`CubicCurve`] by computing the interpolation coefficients for each curve segment.
fn to_curve(&self) -> Result<CubicCurve<P>, Self::Error>;
}
```
All existing spline constructions use this together with errors that
indicate when they didn't have the right control data and provide curves
which have at least one segment whenever they return an `Ok` variant.
Next, `CubicCurve` and `RationalCurve` have been blessed with a
guarantee that their internal array of segments (`segments`) is never
empty. In particular, this field is no longer public, so that invalid
curves cannot be built using struct instantiation syntax. To compensate
for this shortfall for users (in particular library authors who might
want to implement their own generators), there is a new method
`from_segments` on these for constructing a curve from a list of
segments, failing if the list is empty:
```rust
/// Create a new curve from a collection of segments. If the collection of segments is empty,
/// a curve cannot be built and `None` will be returned instead.
pub fn from_segments(segments: impl Into<Vec<CubicSegment<P>>>) -> Option<Self> { //... }
```
All existing methods on `CyclicCurve` and `CubicCurve` maintain the
invariant, so the direct construction of invalid values by users is
impossible.
## Testing
Run unit tests from `bevy_math::cubic_splines`. Additionally, run the
`cubic_splines` example and try to get it to crash using small numbers
of control points: it uses the fallible constructors directly, so if
invalid data is ever constructed, it is basically guaranteed to crash.
---
## Migration Guide
The `to_curve` method on Bevy's cubic splines is now fallible (returning
a `Result`), meaning that any existing calls will need to be updated by
handling the possibility of an error variant.
Similarly, any custom implementation of `CubicGenerator` or
`RationalGenerator` will need to be amended to include an `Error` type
and be made fallible itself.
Finally, the fields of `CubicCurve` and `RationalCurve` are now private,
so any direct constructions of these structs from segments will need to
be replaced with the new `CubicCurve::from_segments` and
`RationalCurve::from_segments` methods.
---
## Design
The main thing to justify here is the choice for the curve internals to
remain the same. After all, if they were able to cause crashes in the
first place, it's worth wondering why safeguards weren't put in place on
the types themselves to prevent that.
My view on this is that the problem was really that the internals of
these methods implicitly relied on the assumption that the value they
were operating on was *actually a curve*, when this wasn't actually
guaranteed. Now, it's possible to make a bunch of small changes inside
the curve struct methods to account for that, but I think that's worse
than just guaranteeing that the data is valid upstream — sampling is
about as hot a code path as we're going to get in this area, and hitting
an additional branch every time it happens just to check that the struct
contains valid data is probably a waste of resources.
Another way of phrasing this is that even if we're only interested in
solving the crashes, the curve's validity needs to be checked at some
point, and it's almost certainly better to do this once at the point of
construction than every time the curve is sampled.
In cases where the control data is supplied dynamically, users would
already have to deal with empty curve outputs basically not working.
Anecdotally, I ran into this while writing the `cubic_splines` example,
and I think the diff illustrates the improvement pretty nicely — the
code no longer has to anticipate whether the output will be good or not;
it just has to handle the `Result`.
The cost of all this, of course, is that we have to guarantee that the
new invariant is actually maintained whenever we extend the API.
However, for the most part, I don't expect users to want to do much
surgery on the internals of their curves anyway.
# Objective
Fill a gap in the functionality of our curve constructions by allowing
users to easily build cyclic curves from control data.
## Solution
Here I opted for something lightweight and discoverable. There is a new
`CyclicCubicGenerator` trait with a method `to_curve_cyclic` which uses
splines' control data to create curves that are cyclic. For now, its
signature is exactly like that of `CubicGenerator` — `to_curve_cyclic`
just yields a `CubicCurve`:
```rust
/// Implement this on cubic splines that can generate a cyclic cubic curve from their spline parameters.
///
/// This makes sense only when the control data can be interpreted cyclically.
pub trait CyclicCubicGenerator<P: VectorSpace> {
/// Build a cyclic [`CubicCurve`] by computing the interpolation coefficients for each curve segment.
fn to_curve_cyclic(&self) -> CubicCurve<P>;
}
```
This trait has been implemented for `CubicHermite`,
`CubicCardinalSpline`, `CubicBSpline`, and `LinearSpline`:
<img width="753" alt="Screenshot 2024-07-01 at 8 58 27 PM"
src="https://github.com/bevyengine/bevy/assets/2975848/69ae0802-3b78-4fb9-b73a-6f842cf3b33c">
<img width="628" alt="Screenshot 2024-07-01 at 9 00 14 PM"
src="https://github.com/bevyengine/bevy/assets/2975848/2992175a-a96c-40fc-b1a1-5206c3572cde">
<img width="606" alt="Screenshot 2024-07-01 at 8 59 36 PM"
src="https://github.com/bevyengine/bevy/assets/2975848/9e99eb3a-dbe6-42da-886c-3d3e00410d03">
<img width="603" alt="Screenshot 2024-07-01 at 8 59 01 PM"
src="https://github.com/bevyengine/bevy/assets/2975848/d037bc0c-396a-43af-ab5c-fad9a29417ef">
(Each type pictured respectively with the control points rendered as
green spheres; tangents not pictured in the case of the Hermite spline.)
These curves are all parametrized so that the output of `to_curve` and
the output of `to_curve_cyclic` are similar. For instance, in
`CubicCardinalSpline`, the first output segment is a curve segment
joining the first and second control points in each, although it is
constructed differently. In the other cases, the segments from
`to_curve` are a subset of those in `to_curve_cyclic`, with the new
segments appearing at the end.
## Testing
I rendered cyclic splines from control data and made sure they looked
reasonable. Existing tests are intact for splines where previous code
was modified. (Note that the coefficient computation for cyclic spline
segments is almost verbatim identical to that of their non-cyclic
counterparts.)
The Bezier benchmarks also look fine.
---
## Changelog
- Added `CyclicCubicGenerator` trait to `bevy_math::cubic_splines` for
creating cyclic curves from control data.
- Implemented `CyclicCubicGenerator` for `CubicHermite`,
`CubicCardinalSpline`, `CubicBSpline`, and `LinearSpline`.
- `bevy_math` now depends on `itertools`.
---
## Discussion
### Design decisions
The biggest thing here is just the approach taken in the first place:
namely, the cyclic constructions use new methods on the same old
structs. This choice was made to reduce friction and increase
discoverability but also because creating new ones just seemed
unnecessary: the underlying data would have been the same, so creating
something like "`CyclicCubicBSpline`" whose internally-held control data
is regarded as cyclic in nature doesn't really accomplish much — the end
result for the user is basically the same either way.
Similarly, I don't presently see a pressing need for `to_curve_cyclic`
to output something other than a `CubicCurve`, although changing this in
the future may be useful. See below.
A notable omission here is that `CyclicCubicGenerator` is not
implemented for `CubicBezier`. This is not a gap waiting to be filled —
`CubicBezier` just doesn't have enough data to join its start with its
end without just making up the requisite control points wholesale. In
all the cases where `CyclicCubicGenerator` has been implemented here,
the fashion in which the ends are connected is quite natural and follows
the semantics of the associated spline construction.
### Future direction
There are two main things here:
1. We should investigate whether we should do something similar for
NURBS. I just don't know that much about NURBS at the moment, so I
regarded this as out of scope for the PR.
2. We may eventually want to change the output type of
`CyclicCubicGenerator::to_curve_cyclic` to a type which reifies the
cyclic nature of the curve output. This wasn't done in this PR because
I'm unsure how much value a type-level guarantee of cyclicity actually
has, but if some useful features make sense only in the case of cyclic
curves, this might be worth pursuing.
