Commit graph

12 commits

Author SHA1 Message Date
Matty
74cecb27bb
Disallow empty cubic and rational curves (#14382)
# 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.
2024-07-29 23:25:14 +00:00
BD103
84363f2fab
Remove redundant imports (#12817)
# Objective

- There are several redundant imports in the tests and examples that are
not caught by CI because additional flags need to be passed.

## Solution

- Run `cargo check --workspace --tests` and `cargo check --workspace
--examples`, then fix all warnings.
- Add `test-check` to CI, which will be run in the check-compiles job.
This should catch future warnings for tests. Examples are already
checked, but I'm not yet sure why they weren't caught.

## Discussion

- Should the `--tests` and `--examples` flags be added to CI, so this is
caught in the future?
- If so, #12818 will need to be merged first. It was also a warning
raised by checking the examples, but I chose to split off into a
separate PR.

---------

Co-authored-by: François Mockers <francois.mockers@vleue.com>
2024-04-01 19:59:08 +00:00
Alice Cecile
599e5e4e76
Migrate from LegacyColor to bevy_color::Color (#12163)
# Objective

- As part of the migration process we need to a) see the end effect of
the migration on user ergonomics b) check for serious perf regressions
c) actually migrate the code
- To accomplish this, I'm going to attempt to migrate all of the
remaining user-facing usages of `LegacyColor` in one PR, being careful
to keep a clean commit history.
- Fixes #12056.

## Solution

I've chosen to use the polymorphic `Color` type as our standard
user-facing API.

- [x] Migrate `bevy_gizmos`.
- [x] Take `impl Into<Color>` in all `bevy_gizmos` APIs
- [x] Migrate sprites
- [x] Migrate UI
- [x] Migrate `ColorMaterial`
- [x] Migrate `MaterialMesh2D`
- [x] Migrate fog
- [x] Migrate lights
- [x] Migrate StandardMaterial
- [x] Migrate wireframes
- [x] Migrate clear color
- [x] Migrate text
- [x] Migrate gltf loader
- [x] Register color types for reflection
- [x] Remove `LegacyColor`
- [x] Make sure CI passes

Incidental improvements to ease migration:

- added `Color::srgba_u8`, `Color::srgba_from_array` and friends
- added `set_alpha`, `is_fully_transparent` and `is_fully_opaque` to the
`Alpha` trait
- add and immediately deprecate (lol) `Color::rgb` and friends in favor
of more explicit and consistent `Color::srgb`
- standardized on white and black for most example text colors
- added vector field traits to `LinearRgba`: ~~`Add`, `Sub`,
`AddAssign`, `SubAssign`,~~ `Mul<f32>` and `Div<f32>`. Multiplications
and divisions do not scale alpha. `Add` and `Sub` have been cut from
this PR.
- added `LinearRgba` and `Srgba` `RED/GREEN/BLUE`
- added `LinearRgba_to_f32_array` and `LinearRgba::to_u32`

## Migration Guide

Bevy's color types have changed! Wherever you used a
`bevy::render::Color`, a `bevy::color::Color` is used instead.

These are quite similar! Both are enums storing a color in a specific
color space (or to be more precise, using a specific color model).
However, each of the different color models now has its own type.

TODO...

- `Color::rgba`, `Color::rgb`, `Color::rbga_u8`, `Color::rgb_u8`,
`Color::rgb_from_array` are now `Color::srgba`, `Color::srgb`,
`Color::srgba_u8`, `Color::srgb_u8` and `Color::srgb_from_array`.
- `Color::set_a` and `Color::a` is now `Color::set_alpha` and
`Color::alpha`. These are part of the `Alpha` trait in `bevy_color`.
- `Color::is_fully_transparent` is now part of the `Alpha` trait in
`bevy_color`
- `Color::r`, `Color::set_r`, `Color::with_r` and the equivalents for
`g`, `b` `h`, `s` and `l` have been removed due to causing silent
relatively expensive conversions. Convert your `Color` into the desired
color space, perform your operations there, and then convert it back
into a polymorphic `Color` enum.
- `Color::hex` is now `Srgba::hex`. Call `.into` or construct a
`Color::Srgba` variant manually to convert it.
- `WireframeMaterial`, `ExtractedUiNode`, `ExtractedDirectionalLight`,
`ExtractedPointLight`, `ExtractedSpotLight` and `ExtractedSprite` now
store a `LinearRgba`, rather than a polymorphic `Color`
- `Color::rgb_linear` and `Color::rgba_linear` are now
`Color::linear_rgb` and `Color::linear_rgba`
- The various CSS color constants are no longer stored directly on
`Color`. Instead, they're defined in the `Srgba` color space, and
accessed via `bevy::color::palettes::css`. Call `.into()` on them to
convert them into a `Color` for quick debugging use, and consider using
the much prettier `tailwind` palette for prototyping.
- The `LIME_GREEN` color has been renamed to `LIMEGREEN` to comply with
the standard naming.
- Vector field arithmetic operations on `Color` (add, subtract, multiply
and divide by a f32) have been removed. Instead, convert your colors
into `LinearRgba` space, and perform your operations explicitly there.
This is particularly relevant when working with emissive or HDR colors,
whose color channel values are routinely outside of the ordinary 0 to 1
range.
- `Color::as_linear_rgba_f32` has been removed. Call
`LinearRgba::to_f32_array` instead, converting if needed.
- `Color::as_linear_rgba_u32` has been removed. Call
`LinearRgba::to_u32` instead, converting if needed.
- Several other color conversion methods to transform LCH or HSL colors
into float arrays or `Vec` types have been removed. Please reimplement
these externally or open a PR to re-add them if you found them
particularly useful.
- Various methods on `Color` such as `rgb` or `hsl` to convert the color
into a specific color space have been removed. Convert into
`LinearRgba`, then to the color space of your choice.
- Various implicitly-converting color value methods on `Color` such as
`r`, `g`, `b` or `h` have been removed. Please convert it into the color
space of your choice, then check these properties.
- `Color` no longer implements `AsBindGroup`. Store a `LinearRgba`
internally instead to avoid conversion costs.

---------

Co-authored-by: Alice Cecile <alice.i.cecil@gmail.com>
Co-authored-by: Afonso Lage <lage.afonso@gmail.com>
Co-authored-by: Rob Parrett <robparrett@gmail.com>
Co-authored-by: Zachary Harrold <zac@harrold.com.au>
2024-02-29 19:35:12 +00:00
Alice Cecile
de004da8d5
Rename bevy_render::Color to LegacyColor (#12069)
# Objective

The migration process for `bevy_color` (#12013) will be fairly involved:
there will be hundreds of affected files, and a large number of APIs.

## Solution

To allow us to proceed granularly, we're going to keep both
`bevy_color::Color` (new) and `bevy_render::Color` (old) around until
the migration is complete.

However, simply doing this directly is confusing! They're both called
`Color`, making it very hard to tell when a portion of the code has been
ported.

As discussed in #12056, by renaming the old `Color` type, we can make it
easier to gradually migrate over, one API at a time.

## Migration Guide

THIS MIGRATION GUIDE INTENTIONALLY LEFT BLANK.

This change should not be shipped to end users: delete this section in
the final migration guide!

---------

Co-authored-by: Alice Cecile <alice.i.cecil@gmail.com>
2024-02-24 21:35:32 +00:00
Carter Anderson
dd619a1087
New Exposure and Lighting Defaults (and calibrate examples) (#11868)
# Objective

After adding configurable exposure, we set the default ev100 value to
`7` (indoor). This brought us out of sync with Blender's configuration
and defaults. This PR changes the default to `9.7` (bright indoor or
very overcast outdoors), as I calibrated in #11577. This feels like a
very reasonable default.

The other changes generally center around tweaking Bevy's lighting
defaults and examples to play nicely with this number, alongside a few
other tweaks and improvements.

Note that for artistic reasons I have reverted some examples, which
changed to directional lights in #11581, back to point lights.
 
Fixes #11577 

---

## Changelog

- Changed `Exposure::ev100` from `7` to `9.7` to better match Blender
- Renamed `ExposureSettings` to `Exposure`
- `Camera3dBundle` now includes `Exposure` for discoverability
- Bumped `FULL_DAYLIGHT ` and `DIRECT_SUNLIGHT` to represent the
middle-to-top of those ranges instead of near the bottom
- Added new `AMBIENT_DAYLIGHT` constant and set that as the new
`DirectionalLight` default illuminance.
- `PointLight` and `SpotLight` now have a default `intensity` of
1,000,000 lumens. This makes them actually useful in the context of the
new "semi-outdoor" exposure and puts them in the "cinema lighting"
category instead of the "common household light" category. They are also
reasonably close to the Blender default.
- `AmbientLight` default has been bumped from `20` to `80`.

## Migration Guide

- The increased `Exposure::ev100` means that all existing 3D lighting
will need to be adjusted to match (DirectionalLights, PointLights,
SpotLights, EnvironmentMapLights, etc). Or alternatively, you can adjust
the `Exposure::ev100` on your cameras to work nicely with your current
lighting values. If you are currently relying on default intensity
values, you might need to change the intensity to achieve the same
effect. Note that in Bevy 0.12, point/spot lights had a different hard
coded ev100 value than directional lights. In Bevy 0.13, they use the
same ev100, so if you have both in your scene, the _scale_ between these
light types has changed and you will likely need to adjust one or both
of them.
2024-02-15 20:42:48 +00:00
Doonv
dc9b486650
Change light defaults & fix light examples (#11581)
# Objective

Fix https://github.com/bevyengine/bevy/issues/11577.

## Solution

Fix the examples, add a few constants to make setting light values
easier, and change the default lighting settings to be more realistic.
(Now designed for an overcast day instead of an indoor environment)

---

I did not include any example-related changes in here.

## Changelogs (not including breaking changes)

### bevy_pbr

- Added `light_consts` module (included in prelude), which contains
common lux and lumen values for lights.
- Added `AmbientLight::NONE` constant, which is an ambient light with a
brightness of 0.
- Added non-EV100 variants for `ExposureSettings`'s EV100 constants,
which allow easier construction of an `ExposureSettings` from a EV100
constant.

## Breaking changes

### bevy_pbr

The several default lighting values were changed:

- `PointLight`'s default `intensity` is now `2000.0`
- `SpotLight`'s default `intensity` is now `2000.0`
- `DirectionalLight`'s default `illuminance` is now
`light_consts::lux::OVERCAST_DAY` (`1000.`)
- `AmbientLight`'s default `brightness` is now `20.0`
2024-02-14 20:43:10 +00:00
Joona Aalto
0166db33f7
Deprecate shapes in bevy_render::mesh::shape (#11773)
# Objective

#11431 and #11688 implemented meshing support for Bevy's new geometric
primitives. The next step is to deprecate the shapes in
`bevy_render::mesh::shape` and to later remove them completely for 0.14.

## Solution

Deprecate the shapes and reduce code duplication by utilizing the
primitive meshing API for the old shapes where possible.

Note that some shapes have behavior that can't be exactly reproduced
with the new primitives yet:

- `Box` is more of an AABB with min/max extents
- `Plane` supports a subdivision count
- `Quad` has a `flipped` property

These types have not been changed to utilize the new primitives yet.

---

## Changelog

- Deprecated all shapes in `bevy_render::mesh::shape`
- Changed all examples to use new primitives for meshing

## Migration Guide

Bevy has previously used rendering-specific types like `UVSphere` and
`Quad` for primitive mesh shapes. These have now been deprecated to use
the geometric primitives newly introduced in version 0.13.

Some examples:

```rust
let before = meshes.add(shape::Box::new(5.0, 0.15, 5.0));
let after = meshes.add(Cuboid::new(5.0, 0.15, 5.0));

let before = meshes.add(shape::Quad::default());
let after = meshes.add(Rectangle::default());

let before = meshes.add(shape::Plane::from_size(5.0));
// The surface normal can now also be specified when using `new`
let after = meshes.add(Plane3d::default().mesh().size(5.0, 5.0));

let before = meshes.add(
    Mesh::try_from(shape::Icosphere {
        radius: 0.5,
        subdivisions: 5,
    })
    .unwrap(),
);
let after = meshes.add(Sphere::new(0.5).mesh().ico(5).unwrap());
```
2024-02-08 18:01:34 +00:00
Tristan Guichaoua
694c06f3d0
Inverse missing_docs logic (#11676)
# Objective

Currently the `missing_docs` lint is allowed-by-default and enabled at
crate level when their documentations is complete (see #3492).
This PR proposes to inverse this logic by making `missing_docs`
warn-by-default and mark crates with imcomplete docs allowed.

## Solution

Makes `missing_docs` warn at workspace level and allowed at crate level
when the docs is imcomplete.
2024-02-03 21:40:55 +00:00
JMS55
fcd7c0fc3d
Exposure settings (adopted) (#11347)
Rebased and finished version of
https://github.com/bevyengine/bevy/pull/8407. Huge thanks to @GitGhillie
for adjusting all the examples, and the many other people who helped
write this PR (@superdump , @coreh , among others) :)

Fixes https://github.com/bevyengine/bevy/issues/8369

---

## Changelog
- Added a `brightness` control to `Skybox`.
- Added an `intensity` control to `EnvironmentMapLight`.
- Added `ExposureSettings` and `PhysicalCameraParameters` for
controlling exposure of 3D cameras.
- Removed the baked-in `DirectionalLight` exposure Bevy previously
hardcoded internally.

## Migration Guide
- If using a `Skybox` or `EnvironmentMapLight`, use the new `brightness`
and `intensity` controls to adjust their strength.
- All 3D scene will now have different apparent brightnesses due to Bevy
implementing proper exposure controls. You will have to adjust the
intensity of your lights and/or your camera exposure via the new
`ExposureSettings` component to compensate.

---------

Co-authored-by: Robert Swain <robert.swain@gmail.com>
Co-authored-by: GitGhillie <jillisnoordhoek@gmail.com>
Co-authored-by: Marco Buono <thecoreh@gmail.com>
Co-authored-by: vero <email@atlasdostal.com>
Co-authored-by: atlas dostal <rodol@rivalrebels.com>
2024-01-16 14:53:21 +00:00
Joona Aalto
a795de30b4
Use impl Into<A> for Assets::add (#10878)
# Motivation

When spawning entities into a scene, it is very common to create assets
like meshes and materials and to add them via asset handles. A common
setup might look like this:

```rust
fn setup(
    mut commands: Commands,
    mut meshes: ResMut<Assets<Mesh>>,
    mut materials: ResMut<Assets<StandardMaterial>>,
) {
    commands.spawn(PbrBundle {
        mesh: meshes.add(Mesh::from(shape::Cube { size: 1.0 })),
        material: materials.add(StandardMaterial::from(Color::RED)),
        ..default()
    });
}
```

Let's take a closer look at the part that adds the assets using `add`.

```rust
mesh: meshes.add(Mesh::from(shape::Cube { size: 1.0 })),
material: materials.add(StandardMaterial::from(Color::RED)),
```

Here, "mesh" and "material" are both repeated three times. It's very
explicit, but I find it to be a bit verbose. In addition to being more
code to read and write, the extra characters can sometimes also lead to
the code being formatted to span multiple lines even though the core
task, adding e.g. a primitive mesh, is extremely simple.

A way to address this is by using `.into()`:

```rust
mesh: meshes.add(shape::Cube { size: 1.0 }.into()),
material: materials.add(Color::RED.into()),
```

This is fine, but from the names and the type of `meshes`, we already
know what the type should be. It's very clear that `Cube` should be
turned into a `Mesh` because of the context it's used in. `.into()` is
just seven characters, but it's so common that it quickly adds up and
gets annoying.

It would be nice if you could skip all of the conversion and let Bevy
handle it for you:

```rust
mesh: meshes.add(shape::Cube { size: 1.0 }),
material: materials.add(Color::RED),
```

# Objective

Make adding assets more ergonomic by making `Assets::add` take an `impl
Into<A>` instead of `A`.

## Solution

`Assets::add` now takes an `impl Into<A>` instead of `A`, so e.g. this
works:

```rust
    commands.spawn(PbrBundle {
        mesh: meshes.add(shape::Cube { size: 1.0 }),
        material: materials.add(Color::RED),
        ..default()
    });
```

I also changed all examples to use this API, which increases consistency
as well because `Mesh::from` and `into` were being used arbitrarily even
in the same file. This also gets rid of some lines of code because
formatting is nicer.

---

## Changelog

- `Assets::add` now takes an `impl Into<A>` instead of `A`
- Examples don't use `T::from(K)` or `K.into()` when adding assets

## Migration Guide

Some `into` calls that worked previously might now be broken because of
the new trait bounds. You need to either remove `into` or perform the
conversion explicitly with `from`:

```rust
// Doesn't compile
let mesh_handle = meshes.add(shape::Cube { size: 1.0 }.into()),

// These compile
let mesh_handle = meshes.add(shape::Cube { size: 1.0 }),
let mesh_handle = meshes.add(Mesh::from(shape::Cube { size: 1.0 })),
```

## Concerns

I believe the primary concerns might be:

1. Is this too implicit?
2. Does this increase codegen bloat?

Previously, the two APIs were using `into` or `from`, and now it's
"nothing" or `from`. You could argue that `into` is slightly more
explicit than "nothing" in cases like the earlier examples where a
`Color` gets converted to e.g. a `StandardMaterial`, but I personally
don't think `into` adds much value even in this case, and you could
still see the actual type from the asset type.

As for codegen bloat, I doubt it adds that much, but I'm not very
familiar with the details of codegen. I personally value the user-facing
code reduction and ergonomics improvements that these changes would
provide, but it might be worth checking the other effects in more
detail.

Another slight concern is migration pain; apps might have a ton of
`into` calls that would need to be removed, and it did take me a while
to do so for Bevy itself (maybe around 20-40 minutes). However, I think
the fact that there *are* so many `into` calls just highlights that the
API could be made nicer, and I'd gladly migrate my own projects for it.
2024-01-08 22:14:43 +00:00
jnhyatt
087a345579
Rename Bezier to CubicBezier for clarity (#9554)
# Objective

A Bezier curve is a curve defined by two or more control points. In the
simplest form, it's just a line. The (arguably) most common type of
Bezier curve is a cubic Bezier, defined by four control points. These
are often used in animation, etc. Bevy has a Bezier curve struct called
`Bezier`. However, this is technically a misnomer as it only represents
cubic Bezier curves.

## Solution

This PR changes the struct name to `CubicBezier` to more accurately
reflect the struct's usage. Since it's exposed in Bevy's prelude, it can
potentially collide with other `Bezier` implementations. While that
might instead be an argument for removing it from the prelude, there's
also something to be said for adding a more general `Bezier` into Bevy,
in which case we'd likely want to use the name `Bezier`. As a final
motivator, not only is the struct located in `cubic_spines.rs`, there
are also several other spline-related structs which follow the
`CubicXxx` naming convention where applicable. For example,
`CubicSegment` represents a cubic Bezier curve (with coefficients
pre-baked).

---

## Migration Guide

- Change all `Bezier` references to `CubicBezier`
2023-08-28 17:37:42 +00:00
Kjolnyr
cfa750a741
Adding a bezier curve example (#8194)
# Objective

Examples on how to use the freshly merged `Bezier` struct ( #7653 ) are
missing.

## Solution

- Added a `bezier_curve.rs` example in the `animation/` folder.

---------

Co-authored-by: ira <JustTheCoolDude@gmail.com>
Co-authored-by: Aevyrie <aevyrie@gmail.com>
2023-04-17 16:16:56 +00:00