# Objective
- Transform propogation could stack overflow when there was a cycle.
- I think https://github.com/bevyengine/bevy/pull/4203 would use all available memory.
## Solution
- Make sure that the child entity's `Parent`s are their parents.
This is also required for when parallelising, although as noted in the comment, the naïve solution would be UB.
(The best way to fix this would probably be an `&mut UnsafeCell<T>` `WorldQuery`, or wrapper type with the same effect)
### Problem
It currently isn't possible to construct the default value of a reflected type. Because of that, it isn't possible to use `add_component` of `ReflectComponent` to add a new component to an entity because you can't know what the initial value should be.
### Solution
1. add `ReflectDefault` type
```rust
#[derive(Clone)]
pub struct ReflectDefault {
default: fn() -> Box<dyn Reflect>,
}
impl ReflectDefault {
pub fn default(&self) -> Box<dyn Reflect> {
(self.default)()
}
}
impl<T: Reflect + Default> FromType<T> for ReflectDefault {
fn from_type() -> Self {
ReflectDefault {
default: || Box::new(T::default()),
}
}
}
```
2. add `#[reflect(Default)]` to all component types that implement `Default` and are user facing (so not `ComputedSize`, `CubemapVisibleEntities` etc.)
This makes it possible to add the default value of a component to an entity without any compile-time information:
```rust
fn main() {
let mut app = App::new();
app.register_type::<Camera>();
let type_registry = app.world.get_resource::<TypeRegistry>().unwrap();
let type_registry = type_registry.read();
let camera_registration = type_registry.get(std::any::TypeId::of::<Camera>()).unwrap();
let reflect_default = camera_registration.data::<ReflectDefault>().unwrap();
let reflect_component = camera_registration
.data::<ReflectComponent>()
.unwrap()
.clone();
let default = reflect_default.default();
drop(type_registry);
let entity = app.world.spawn().id();
reflect_component.add_component(&mut app.world, entity, &*default);
let camera = app.world.entity(entity).get::<Camera>().unwrap();
dbg!(&camera);
}
```
### Open questions
- should we have `ReflectDefault` or `ReflectFromWorld` or both?
Supercedes https://github.com/bevyengine/bevy/pull/3340, and absorbs the test from there.
# Objective
- Fixes#3329
## Solution
- If the `Children` component has changed, we currently do not have a way to know how it has changed.
- Therefore, we must update the hierarchy downwards from that point to be correct.
Co-authored-by: Daniel McNab <36049421+DJMcNab@users.noreply.github.com>
# Objective
- in #3851, a feature for tracing was added to bevy_transform
- usage of that feature was moved to bevy_hierarchy, but the feature was not updated
## Solution
- add the feature to bevy_hierarchy, remove it from bevy_transform
# Objective
- Provide more information when despawning an entity
## Solution
- Add a debug log when despawning an entity
- Add spans to the recursive ways of despawning an entity
```sh
RUST_LOG=debug cargo run --example panic --features trace
# RUST_LOG=debug needed to show debug logs from bevy_ecs
# --features trace needed to have the extra spans
...
DEBUG bevy_app:frame:stage{name=Update}:system_commands{name="panic::despawn_parent"}:command{name="DespawnRecursive" entity=0v0}: bevy_ecs::world: Despawning entity 1v0
DEBUG bevy_app:frame:stage{name=Update}:system_commands{name="panic::despawn_parent"}:command{name="DespawnRecursive" entity=0v0}: bevy_ecs::world: Despawning entity 0v0
```
# Objective
- While animating 501 https://github.com/KhronosGroup/glTF-Sample-Models/tree/master/2.0/BrainStem, I noticed things were getting a little slow
- Looking in tracy, the system `extract_skinned_meshes` is taking a lot of time, with a mean duration of 15.17ms
## Solution
- ~~Use `Vec` instead of a `SmallVec`~~
- ~~Don't use an temporary variable~~
- Compute the affine matrix as an `Affine3A` instead
- Remove the `temp` vec
| |mean|
|---|---|
|base|15.17ms|
|~~vec~~|~~9.31ms~~|
|~~no temp variable~~|~~11.31ms~~|
|removing the temp vector|8.43ms|
|affine|13.21ms|
|all together|7.23ms|
# Objective
- Improve transform propagation performance
## Solution
- Use `Changed<Transform>` as part of the `root_query` and `transform_query` to avoid the indirection of having to look up the `Entity` in the `changed_transform_query`
- Get rid of the `changed_transform_query` entirely
- `transform_propagate_system` execution time for `many_cubes -- sphere` dropped from 1.07ms to 0.159ms, an 85% reduction for this system. Frame rate increased from ~42fps to ~44fps
# Objective
- Hierarchy tools are not just used for `Transform`: they are also used for scenes.
- In the future there's interest in using them for other features, such as visiibility inheritance.
- The fact that these tools are found in `bevy_transform` causes a great deal of user and developer confusion
- Fixes#2758.
## Solution
- Split `bevy_transform` into two!
- Make everything work again.
Note that this is a very tightly scoped PR: I *know* there are code quality and docs issues that existed in bevy_transform that I've just moved around. We should fix those in a seperate PR and try to merge this ASAP to reduce the bitrot involved in splitting an entire crate.
## Frustrations
The API around `GlobalTransform` is a mess: we have massive code and docs duplication, no link between the two types and no clear way to extend this to other forms of inheritance.
In the medium-term, I feel pretty strongly that `GlobalTransform` should be replaced by something like `Inherited<Transform>`, which lives in `bevy_hierarchy`:
- avoids code duplication
- makes the inheritance pattern extensible
- links the types at the type-level
- allows us to remove all references to inheritance from `bevy_transform`, making it more useful as a standalone crate and cleaning up its docs
## Additional context
- double-blessed by @cart in https://github.com/bevyengine/bevy/issues/4141#issuecomment-1063592414 and https://github.com/bevyengine/bevy/issues/2758#issuecomment-913810963
- preparation for more advanced / cleaner hierarchy tools: go read https://github.com/bevyengine/rfcs/pull/53 !
- originally attempted by @finegeometer in #2789. It was a great idea, just needed more discussion!
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
# Objective
Lets say we need to rotate stretched object for this purpose we can created stretched `Child` and add as child to `Parent`, later we can rotate `Parent`, `Child` in this situation should rotate keeping it form, it is not the case with `SpriteBundle` currently. If you try to do it with `SpriteBundle` it will deform.
## Solution
My pull request fixes order of transformations to scale -> rotate -> translate, with this fix `SpriteBundle` behaves as expected in described rotation, without deformation. Here is quote from "Essential Mathematics for Games":
> Generally, the desired order we wish to use for these transforms is to scale first, then rotate, then translate. Scaling first gives us the scaling along the axes we expect. We can then rotate around the origin of the frame, and then translate it into place.
I'm must say when I was using `MaterialMesh2dBundle` it behaves correctly in both cases with `bevy main` and with my fix, don't know why, was not able to figure it out why there is difference.
here is code I was using for testing:
```rust
use bevy::{
prelude::*,
render::render_resource::{Extent3d, TextureDimension, TextureFormat},
sprite::{MaterialMesh2dBundle, Mesh2dHandle},
};
fn main() {
let mut app = App::new();
app.insert_resource(ClearColor(Color::rgb(0.1, 0.2, 0.3)))
.add_plugins(DefaultPlugins)
.add_startup_system(setup);
app.run();
}
fn setup(
mut commands: Commands,
mut images: ResMut<Assets<Image>>,
mut meshes: ResMut<Assets<Mesh>>,
mut materials: ResMut<Assets<ColorMaterial>>,
) {
let mut c = OrthographicCameraBundle::new_2d();
c.orthographic_projection.scale = 1.0 / 10.0;
commands.spawn_bundle(c);
// note: mesh somehow works for both variants
// let quad: Mesh2dHandle = meshes.add(Mesh::from(shape::Quad::default())).into();
// let child = commands
// .spawn_bundle(MaterialMesh2dBundle {
// mesh: quad.clone(),
// transform: Transform::from_translation(Vec3::new(0.0, 0.0, -1.0))
// .with_scale(Vec3::new(10.0, 1.0, 1.0)),
// material: materials.add(ColorMaterial::from(Color::BLACK)),
// ..Default::default()
// })
// .id();
// commands
// .spawn_bundle(MaterialMesh2dBundle {
// mesh: quad,
// transform: Transform::from_rotation(Quat::from_rotation_z(0.78))
// .with_translation(Vec3::new(0.0, 0.0, 10.0)),
// material: materials.add(ColorMaterial::from(Color::WHITE)),
// ..Default::default()
// })
// .push_children(&[child]);
let white = images.add(get_image(Color::rgb(1.0, 1.0, 1.0)));
let black = images.add(get_image(Color::rgb(0.0, 0.0, 0.0)));
let child = commands
.spawn_bundle(SpriteBundle {
texture: black,
transform: Transform::from_translation(Vec3::new(0.0, 0.0, -1.0))
.with_scale(Vec3::new(10.0, 1.0, 1.0)),
..Default::default()
})
.id();
commands
.spawn_bundle(SpriteBundle {
texture: white,
transform: Transform::from_rotation(Quat::from_rotation_z(0.78))
.with_translation(Vec3::new(0.0, 0.0, 10.0)),
..Default::default()
})
.push_children(&[child]);
}
fn get_image(color: Color) -> Image {
let mut bytes = Vec::with_capacity((1 * 1 * 4 * 4) as usize);
let color = color.as_rgba_f32();
bytes.extend(color[0].to_le_bytes());
bytes.extend(color[1].to_le_bytes());
bytes.extend(color[2].to_le_bytes());
bytes.extend(1.0_f32.to_le_bytes());
Image::new(
Extent3d {
width: 1,
height: 1,
depth_or_array_layers: 1,
},
TextureDimension::D2,
bytes,
TextureFormat::Rgba32Float,
)
}
```
here is screenshot with `bevy main` and my fix:
What is says on the tin.
This has got more to do with making `clippy` slightly more *quiet* than it does with changing anything that might greatly impact readability or performance.
that said, deriving `Default` for a couple of structs is a nice easy win
# Objective
- Bevy currently has no simple way to make an "empty" Entity work correctly in a Hierachy.
- The current Solution is to insert a Tuple instead:
```rs
.insert_bundle((Transform::default(), GlobalTransform::default()))
```
## Solution
* Add a `TransformBundle` that combines the Components:
```rs
.insert_bundle(TransformBundle::default())
```
* The code is based on #2331, except for missing the more controversial usage of `TransformBundle` as a Sub-bundle in preexisting Bundles.
Co-authored-by: MinerSebas <66798382+MinerSebas@users.noreply.github.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
# Objective
- Missing obvious way to rotate a transform around a point. This is popularly used for rotation of an object in world space ("orbiting" a point), or for local rotation of an object around a pivot point on that object.
- Present in other (not to be named) game engines
- Was question from user on Discord today (thread "object rotation")
## Solution
- Added Transform::rotate_around method where point is specified in reference frame of the parent (if any) or in world space.
# Objective
bevy_transform needed documentation and warn(missing_docs) as requested by #3492
## Solution
warn(missing_docs) was activated and documentation was added to cover the crate
Co-authored-by: Troels Jessen <kairyuka@gmail.com>
# Objective
CI should check for missing backticks in doc comments.
Fixes#3435
## Solution
`clippy` has a lint for this: `doc_markdown`. This enables that lint in the CI script.
Of course, enabling this lint in CI causes a bunch of lint errors, so I've gone through and fixed all of them. This was a huge edit that touched a ton of files, so I split the PR up by crate.
When all of the following are merged, the CI should pass and this can be merged.
+ [x] #3467
+ [x] #3468
+ [x] #3470
+ [x] #3469
+ [x] #3471
+ [x] #3472
+ [x] #3473
+ [x] #3474
+ [x] #3475
+ [x] #3476
+ [x] #3477
+ [x] #3478
+ [x] #3479
+ [x] #3480
+ [x] #3481
+ [x] #3482
+ [x] #3483
+ [x] #3484
+ [x] #3485
+ [x] #3486
#3457 adds the `doc_markdown` clippy lint, which checks doc comments to make sure code identifiers are escaped with backticks. This causes a lot of lint errors, so this is one of a number of PR's that will fix those lint errors one crate at a time.
This PR fixes lints in the `bevy_transform` crate.
# Objective
Fix#2406
Scene parenting was not done completely, leaving the hierarchy maintenance to the standard system. As scene spawning happens in stage `PreUpdate` and hierarchy maintenance in stage `PostUpdate`, this left the scene in an invalid state parent wise for part of a frame
## Solution
Also add/update the `Children` component when spawning the scene.
I kept the `Children` component as a `SmallVec`, it could be moved to an `HashSet` to guarantee uniqueness
Co-authored-by: François <8672791+mockersf@users.noreply.github.com>
Fixes#2566Fixes#3005
There are only READMEs in the 4 crates here (with the exception of bevy itself).
Those 4 crates are ecs, reflect, tasks, and transform.
These should each now include their respective README files.
Co-authored-by: Hoidigan <57080125+Hoidigan@users.noreply.github.com>
Co-authored-by: Daniel Nelsen <57080125+Hoidigan@users.noreply.github.com>
# Objective
- `bevy_ecs` exposes as an optional feature `bevy_reflect`. Disabling it doesn't compile.
- `bevy_asset` exposes as an optional feature `filesystem_watcher`. Disabling it doesn't compile. It is also not possible to disable this feature from Bevy
## Solution
- Fix compilation errors when disabling the default features. Make it possible to disable the feature `filesystem_watcher` from Bevy
Objective
During work on #3009 I've found that not all jobs use actions-rs, and therefore, an previous version of Rust is used for them. So while compilation and other stuff can pass, checking markup and Android build may fail with compilation errors.
Solution
This PR adds `action-rs` for any job running cargo, and updates the edition to 2021.
This implements the most minimal variant of #1843 - a derive for marker trait. This is a prerequisite to more complicated features like statically defined storage type or opt-out component reflection.
In order to make component struct's purpose explicit and avoid misuse, it must be annotated with `#[derive(Component)]` (manual impl is discouraged for compatibility). Right now this is just a marker trait, but in the future it might be expanded. Making this change early allows us to make further changes later without breaking backward compatibility for derive macro users.
This already prevents a lot of issues, like using bundles in `insert` calls. Primitive types are no longer valid components as well. This can be easily worked around by adding newtype wrappers and deriving `Component` for them.
One funny example of prevented bad code (from our own tests) is when an newtype struct or enum variant is used. Previously, it was possible to write `insert(Newtype)` instead of `insert(Newtype(value))`. That code compiled, because function pointers (in this case newtype struct constructor) implement `Send + Sync + 'static`, so we allowed them to be used as components. This is no longer the case and such invalid code will trigger a compile error.
Co-authored-by: = <=>
Co-authored-by: TheRawMeatball <therawmeatball@gmail.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
# Objective
Noticed a comment saying changed detection should be enabled for hierarchy maintenance once stable
Fixes#891
## Solution
Added `Changed<Parent>` filter on the query
# Objective
Make it easier to construct transforms. E.g.
```rs
Transform::from_xyz(0.0, 0.0, 10.0).with_scale(Vec3::splat(2.0))
```
I found myself writing an extension method to do this so I don't have to write:
```rs
Transform {
translation: Vec3::new(0.0, 0.0, 10.0),
scale: Vec3::splat(2.0),
..Default::default()
}
```
## Solution
Add *builder style* methods to `Transform`.
Methods:
- `with_translation`
- `with_rotation`
- `with_scale`
I also added these methods to `GlobalTransform`. But they are probably less useful there.
# Objective
Enable using exact World lifetimes during read-only access . This is motivated by the new renderer's need to allow read-only world-only queries to outlive the query itself (but still be constrained by the world lifetime).
For example:
115b170d1f/pipelined/bevy_pbr2/src/render/mod.rs (L774)
## Solution
Split out SystemParam state and world lifetimes and pipe those lifetimes up to read-only Query ops (and add into_inner for Res). According to every safety test I've run so far (except one), this is safe (see the temporary safety test commit). Note that changing the mutable variants to the new lifetimes would allow aliased mutable pointers (try doing that to see how it affects the temporary safety tests).
The new state lifetime on SystemParam does make `#[derive(SystemParam)]` more cumbersome (the current impl requires PhantomData if you don't use both lifetimes). We can make this better by detecting whether or not a lifetime is used in the derive and adjusting accordingly, but that should probably be done in its own pr.
## Why is this a draft?
The new lifetimes break QuerySet safety in one very specific case (see the query_set system in system_safety_test). We need to solve this before we can use the lifetimes given.
This is due to the fact that QuerySet is just a wrapper over Query, which now relies on world lifetimes instead of `&self` lifetimes to prevent aliasing (but in systems, each Query has its own implied lifetime, not a centralized world lifetime). I believe the fix is to rewrite QuerySet to have its own World lifetime (and own the internal reference). This will complicate the impl a bit, but I think it is doable. I'm curious if anyone else has better ideas.
Personally, I think these new lifetimes need to happen. We've gotta have a way to directly tie read-only World queries to the World lifetime. The new renderer is the first place this has come up, but I doubt it will be the last. Worst case scenario we can come up with a second `WorldLifetimeQuery<Q, F = ()>` parameter to enable these read-only scenarios, but I'd rather not add another type to the type zoo.
## Objective
- Clean up remaining references to the trait `FromResources`, which was replaced in favor of `FromWorld` during the ECS rework.
## Solution
- Remove the derive macro for `FromResources`
- Change doc references of `FromResources` to `FromWorld`
(this is the first item in #2576)
## Objective
This code would result in a crash:
```rust
use bevy::prelude::*;
fn main() {
let mut world = World::new();
let child = world.spawn().id();
world.spawn().push_children(&[child]);
}
```
## Solution
Update the `EntityMut`'s location after inserting a component on the children entities, as it may have changed.
# Objective
- Remove all the `.system()` possible.
- Check for remaining missing cases.
## Solution
- Remove all `.system()`, fix compile errors
- 32 calls to `.system()` remains, mostly internals, the few others should be removed after #2446
This is extracted out of eb8f973646476b4a4926ba644a77e2b3a5772159 and includes some additional changes to remove all references to AppBuilder and fix examples that still used App::build() instead of App::new(). In addition I didn't extract the sub app feature as it isn't ready yet.
You can use `git diff --diff-filter=M eb8f973646476b4a4926ba644a77e2b3a5772159` to find all differences in this PR. The `--diff-filtered=M` filters all files added in the original commit but not in this commit away.
Co-Authored-By: Carter Anderson <mcanders1@gmail.com>
* bevy_pbr2: Add support for most of the StandardMaterial textures
Normal maps are not included here as they require tangents in a vertex attribute.
* bevy_pbr2: Ensure RenderCommandQueue is ready for PbrShaders init
* texture_pipelined: Add a light to the scene so we can see stuff
* WIP bevy_pbr2: back to front sorting hack
* bevy_pbr2: Uniform control flow for texture sampling in pbr.frag
From 'fintelia' on the Bevy Render Rework Round 2 discussion:
"My understanding is that GPUs these days never use the "execute both branches
and select the result" strategy. Rather, what they do is evaluate the branch
condition on all threads of a warp, and jump over it if all of them evaluate to
false. If even a single thread needs to execute the if statement body, however,
then the remaining threads are paused until that is completed."
* bevy_pbr2: Simplify texture and sampler names
The StandardMaterial_ prefix is no longer needed
* bevy_pbr2: Match default 'AmbientColor' of current bevy_pbr for now
* bevy_pbr2: Convert from non-linear to linear sRGB for the color uniform
* bevy_pbr2: Add pbr_pipelined example
* Fix view vector in pbr frag to work in ortho
* bevy_pbr2: Use a 90 degree y fov and light range projection for lights
* bevy_pbr2: Add AmbientLight resource
* bevy_pbr2: Convert PointLight color to linear sRGB for use in fragment shader
* bevy_pbr2: pbr.frag: Rename PointLight.projection to view_projection
The uniform contains the view_projection matrix so this was incorrect.
* bevy_pbr2: PointLight is an OmniLight as it has a radius
* bevy_pbr2: Factoring out duplicated code
* bevy_pbr2: Implement RenderAsset for StandardMaterial
* Remove unnecessary texture and sampler clones
* fix comment formatting
* remove redundant Buffer:from
* Don't extract meshes when their material textures aren't ready
* make missing textures in the queue step an error
Co-authored-by: Aevyrie <aevyrie@gmail.com>
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
This relicenses Bevy under the dual MIT or Apache-2.0 license. For rationale, see #2373.
* Changes the LICENSE file to describe the dual license. Moved the MIT license to docs/LICENSE-MIT. Added the Apache-2.0 license to docs/LICENSE-APACHE. I opted for this approach over dumping both license files at the root (the more common approach) for a number of reasons:
* Github links to the "first" license file (LICENSE-APACHE) in its license links (you can see this in the wgpu and rust-analyzer repos). People clicking these links might erroneously think that the apache license is the only option. Rust and Amethyst both use COPYRIGHT or COPYING files to solve this problem, but this creates more file noise (if you do everything at the root) and the naming feels way less intuitive.
* People have a reflex to look for a LICENSE file. By providing a single license file at the root, we make it easy for them to understand our licensing approach.
* I like keeping the root clean and noise free
* There is precedent for putting the apache and mit license text in sub folders (amethyst)
* Removed the `Copyright (c) 2020 Carter Anderson` copyright notice from the MIT license. I don't care about this attribution, it might make license compliance more difficult in some cases, and it didn't properly attribute other contributors. We shoudn't replace it with something like "Copyright (c) 2021 Bevy Contributors" because "Bevy Contributors" is not a legal entity. Instead, we just won't include the copyright line (which has precedent ... Rust also uses this approach).
* Updates crates to use the new "MIT OR Apache-2.0" license value
* Removes the old legion-transform license file from bevy_transform. bevy_transform has been its own, fully custom implementation for a long time and that license no longer applies.
* Added a License section to the main readme
* Updated our Bevy Plugin licensing guidelines.
As a follow-up we should update the website to properly describe the new license.
Closes#2373
# Objective
- Currently `Commands` are quite slow due to the need to allocate for each command and wrap it in a `Box<dyn Command>`.
- For example:
```rust
fn my_system(mut cmds: Commands) {
cmds.spawn().insert(42).insert(3.14);
}
```
will have 3 separate `Box<dyn Command>` that need to be allocated and ran.
## Solution
- Utilize a specialized data structure keyed `CommandQueueInner`.
- The purpose of `CommandQueueInner` is to hold a collection of commands in contiguous memory.
- This allows us to store each `Command` type contiguously in memory and quickly iterate through them and apply the `Command::write` trait function to each element.
# Objective
- CI jobs are starting to fail due to `clippy::bool-assert-comparison` and `clippy::single_component_path_imports` being triggered.
## Solution
- Fix all uses where `asset_eq!(<condition>, <bool>)` could be replace by `assert!`
- Move the `#[allow()]` for `single_component_path_imports` to `#![allow()]` at the start of the files.
Continuing the work on reducing the safety footguns in the code, I've removed one extra `UnsafeCell` in favour of safe `Cell` usage inisde `ComponentTicks`. That change led to discovery of misbehaving component insert logic, where data wasn't properly dropped when overwritten. Apart from that being fixed, some method names were changed to better convey the "initialize new allocation" and "replace existing allocation" semantic.
Depends on #2221, I will rebase this PR after the dependency is merged. For now, review just the last commit.
Co-authored-by: Carter Anderson <mcanders1@gmail.com>
Fixes#2274
When calling `despawn_recursive`, the recursive loop doesn't need to remove the entity from the children list of its parent when the parent will also be deleted
Upside:
* Removes two entity lookup per entity being recursively despawned
Downside:
* The change detection on the `Children` component of a deleted entity in the despawned hierarchy will not be triggered
