naga and wgpu should polyfill WGSL instance_index functionality where it
is not available in GLSL. Until that is done, we can work around it in
bevy using a push constant which is converted to a uniform by naga and
wgpu.
# Objective
- Fixes#9375
## Solution
- Use a push constant to pass in the base instance to the shader on
WebGL2 so that base instance + gl_InstanceID is used to correctly
represent the instance index.
## TODO
- [ ] Benchmark vs per-object dynamic offset MeshUniform as this will
now push a uniform value per-draw as well as update the dynamic offset
per-batch.
- [x] Test on DX12 AMD/NVIDIA to check that this PR does not regress any
problems that were observed there. (@Elabajaba @robtfm were testing that
last time - help appreciated. <3 )
---
## Changelog
- Added: `bevy_render::instance_index` shader import which includes a
workaround for the lack of a WGSL `instance_index` polyfill for WebGL2
in naga and wgpu for the time being. It uses a push_constant which gets
converted to a plain uniform by naga and wgpu.
## Migration Guide
Shader code before:
```
struct Vertex {
@builtin(instance_index) instance_index: u32,
...
}
@vertex
fn vertex(vertex_no_morph: Vertex) -> VertexOutput {
...
var model = mesh[vertex_no_morph.instance_index].model;
```
After:
```
#import bevy_render::instance_index
struct Vertex {
@builtin(instance_index) instance_index: u32,
...
}
@vertex
fn vertex(vertex_no_morph: Vertex) -> VertexOutput {
...
var model = mesh[bevy_render::instance_index::get_instance_index(vertex_no_morph.instance_index)].model;
```
# Objective
Provide a slightly better message when a contributor needs to update the
generated example readme file for [any number of
reasons](https://github.com/bevyengine/bevy/pull/9372#discussion_r1285876202)
but hasn't added any examples.
This recently happened here:
https://github.com/bevyengine/bevy/pull/9370#issuecomment-1666776092
The contributor modified the readme template and is being told that they
added an example.
## Solution
The advice given is still correct. Just change the message so that it's
not accusing the contributor of adding an example.
It may be possible to instead add more specific messages instead if
someone is motivated to do that.
edit: reworked this whole PR text
---------
Co-authored-by: Nicola Papale <nicopap@users.noreply.github.com>
# Objective
The default for `ContentSize` should have the `measure_func` field set
to `None`, instead of a fixed size of zero. This means that until a
measure func is set the size of the UI node will be determined by its
`Style` constraints. This is preferable as it allows users to specify
the space the Node should take up in the layout while waiting for
content to load.
## Solution
Derive `Default` for `ContentSize`.
The PR also adds a `fixed_size` helper function to make it a bit easier
to access the old behaviour.
## Changelog
* Derived `Default` for `ContentSize`
* Added a `fixed_size` helper function to `ContentSize` that creates a
new `ContentSize` with a `MeasureFunc` that always returns the same
value, regardless of layout constraints.
## Migration Guide
The default for `ContentSize` now sets its `measure_func` to `None`,
instead of a fixed size measure that returns `Vec2::ZERO`.
The helper function `fixed_size` can be called with
`ContentSize::fixed_size(Vec2::ZERO)` to get the previous behaviour.
# Objective
It seems the behavior of field attributes was accidentally broken at
some point. Take the following code:
```rust
#[derive(Reflect)]
struct Foo {
#[reflect(ignore, default)]
value: usize
}
```
The above code should simply mark `value` as ignored and specify a
default behavior. However, what this actually does is discard both.
That's especially a problem when we don't want the field to be be given
a `Reflect` or `FromReflect` bound (which is why we ignore it in the
first place).
This only happens when the attributes are combined into one. The
following code works properly:
```rust
#[derive(Reflect)]
struct Foo {
#[reflect(ignore)]
#[reflect(default)]
value: usize
}
```
## Solution
Cleaned up the field attribute parsing logic to support combined field
attributes.
---
## Changelog
- Fixed a bug where `Reflect` derive attributes on fields are not able
to be combined into a single attribute
# Objective
- I forgot to update the example after the `ViewNodeRunner` was merged.
It was even partially mentioned in one of the comments.
## Solution
- Use the `ViewNodeRunner` in the post_processing example
- I also broke up a few lines that were a bit long
---------
Co-authored-by: JMS55 <47158642+JMS55@users.noreply.github.com>
# Objective
- Follow up to #8887
- The parsing code in `bevy_reflect/src/path/mod.rs` could also do with
some cleanup
## Solution
- Create the `parse.rs` module, move all parsing code to this module
- The parsing errors also now keep track of the whole parsed string, and
are much more fine-grained
### Detailed changes
- Move `PathParser` to `parse.rs` submodule
- Rename `token_to_access` to `access_following` (yep, goes from 132
lines to 16)
- Move parsing tests into the `parse.rs` file
# Objective
The doc comment for `text_system` is not quite correct. It implies that
a new `TextLayoutInfo` is generated on changes to `Text` and `Style`.
While changes to those components might indirectly trigger a
regeneration of the text layout, `text_system` itself only queries for
changes to `Node`
Also added details to `measure_text_system`'s doc comments explaining
how it reacts to changes.
---------
Co-authored-by: François <mockersf@gmail.com>
# Objective
- `bevy_tasks` emits warnings under certain conditions
When I run `cargo clippy -p bevy_tasks` the warning doesn't show up,
while if I run it with `cargo clippy -p bevy_asset` the warning shows
up.
## Solution
- Fix the warnings.
## Longer term solution
We should probably fix CI so that those warnings do not slip through.
But that's not the goal of this PR.
# Objective
The `post_processing` example is currently broken when run with webgl2.
```
cargo run --example post_processing --target=wasm32-unknown-unknown
```
```
wasm.js:387 panicked at 'wgpu error: Validation Error
Caused by:
In Device::create_render_pipeline
note: label = `post_process_pipeline`
In the provided shader, the type given for group 0 binding 2 has a size of 4. As the device does not support `DownlevelFlags::BUFFER_BINDINGS_NOT_16_BYTE_ALIGNED`, the type must have a size that is a multiple of 16 bytes.
```
I bisected the breakage to c7eaedd6a1.
## Solution
Add padding when using webgl2
# Objective
This doc comment for the `set` method of `ContentSize`:
```
Set a `Measure` for this function
```
doesn't seem to make sense, `ContentSize` is not a function.
# Solution
Replace it.
# Objective
When an `AudioSink` is removed from an entity, the audio player will
automatically start any `AudioSource` still attached, which normally is
the one used to start playback in the first place.
## Solution
Long story short, the default behavior is restarting the audio, and this
commit documents that.
---
## Changelog
Fixed documentation on `AudioSink` to clarify removal behavior.
# Objective
- Some examples crash in CI because of needing too many resources for
the windows runner
- Some examples have random results making it hard to compare
screenshots
## Solution
- `bloom_3d`: reduce the number of spheres
- `pbr`: use simpler spheres and reuse the mesh
- `tonemapping`: use simpler spheres and reuse the mesh
- `shadow_biases`: reduce the number of spheres
- `spotlight`: use a seeded rng, move more cubes in view while reducing
the total number of cubes, and reuse meshes and materials
- `external_source_external_thread`, `iter_combinations`,
`parallel_query`: use a seeded rng
Examples of errors encountered:
```
Caused by:
In Device::create_bind_group
note: label = `bloom_upsampling_bind_group`
Not enough memory left
```
```
Caused by:
In Queue::write_buffer
Parent device is lost
```
```
ERROR wgpu_core::device::life: Mapping failed Device(Lost)
```
# Objective
shader defs associated with a shader via `load_internal_asset!` or
`Shader::from_xxx_with_defs` were being accidentally ignored for
top-level shaders.
## Solution
include the defs for top level shaders.
# Objective
- Fixes#9114
## Solution
Inside `ScheduleGraph::build_schedule()` the variable `node_count =
self.systems.len() + self.system_sets.len()` is used to calculate the
indices for the `reachable` bitset derived from `self.hierarchy.graph`.
However, the number of nodes inside `self.hierarchy.graph` does not
always correspond to `self.systems.len() + self.system_sets.len()` when
`ambiguous_with` is used, because an ambiguous set is added to
`system_sets` (because we need an `NodeId` for the ambiguity graph)
without adding a node to `self.hierarchy`.
In this PR, we rename `node_count` to the more descriptive name
`hg_node_count` and set it to `self.hierarchy.graph.node_count()`.
---------
Co-authored-by: James Liu <contact@jamessliu.com>
# Objective
Fixes#9113
## Solution
disable `multi-threaded` default feature
## Migration Guide
The `multi-threaded` feature in `bevy_ecs` and `bevy_tasks` is no longer
enabled by default. However, this remains a default feature for the
umbrella `bevy` crate. If you depend on `bevy_ecs` or `bevy_tasks`
directly, you should consider enabling this to allow systems to run in
parallel.
Fixes#5856. Fixes#8080. Fixes#9040.
# Objective
We need to limit the update rate of games whose windows are not visible
(minimized or completely occluded). Compositors typically ignore the
VSync settings of windows that aren't visible. That, combined with the
lack of rendering work, results in a scenario where an app becomes
completely CPU-bound and starts updating without limit.
There are currently three update modes.
- `Continuous` updates an app as often as possible.
- `Reactive` updates when new window or device events have appeared, a
timer expires, or a redraw is requested.
- `ReactiveLowPower` is the same as `Reactive` except it ignores device
events (e.g. general mouse movement).
The problem is that the default "game" settings are set to `Contiuous`
even when no windows are visible.
### More Context
- https://github.com/libsdl-org/SDL/issues/1871
- https://github.com/glfw/glfw/issues/680
- https://github.com/godotengine/godot/issues/19741
- https://github.com/godotengine/godot/issues/64708
## Solution
Change the default "unfocused" `UpdateMode` for games to
`ReactiveLowPower` just like desktop apps. This way, even when the
window is occluded, the app still updates at a sensible rate or if
something about the window changes. I chose 20Hz arbitrarily.
# Objective
The `lifetimeless` module has been a source of confusion for bevy users
for a while now.
## Solution
Add a couple paragraph explaining that, yes, you can use one of the type
alias safely, without ever leaking any memory.
Redo of #7590 since I messed up my branch.
# Objective
- Revise docs.
- Refactor event loop code a little bit to make it easier to follow.
## Solution
- Do the above.
---
### Migration Guide
- `UpdateMode::Reactive { max_wait: .. }` -> `UpdateMode::Reactive {
wait: .. }`
- `UpdateMode::ReactiveLowPower { max_wait: .. }` ->
`UpdateMode::ReactiveLowPower { wait: .. }`
---------
Co-authored-by: Sélène Amanita <134181069+Selene-Amanita@users.noreply.github.com>
# Objective
Remove the `With<Parent>` query filter from the `parent_node_query`
parameter of the `bevy_ui::render::extract_uinode_borders` function.
This is a bug, the query is only used to retrieve the size of the
current node's parent. We don't care if that parent node has a `Parent`
or not.
---------
Co-authored-by: François <mockersf@gmail.com>
# Objective
- Fixes#9138
## Solution
- Calling `Camera2dBundle::default()` will now result in a
`Camera2dBundle` with `Vec3::ZERO` transform, `far` value of `1000.` and
`near` value of `-1000.`.
- This will enable the rendering of 2d entities in negative z space by
default.
- I did not modify `new_with_far` as moving the camera to `Vec3::ZERO`
in that function will cause entities in the positive z space to become
hidden without further changes. And the further changes cannot be
applied without it being a breaking change.
# Objective
Glam 0.24 added new glam types (```I64Vec``` and ```U64Vec```). However
these are not reflectable unlike the other glam types
## Solution
Implement reflect for these new types
---
## Changelog
Implements reflect with the impl_reflect_struct macro on ```I64Vec2```,
```I64Vec3```, ```I64Vec4```, ```U64Vec2```, ```U64Vec3```, and
```U64Vec4``` types
# Objective
- Repeat in `Gizmos` that they are drawned in immediate mode, which is
said at the module level but not here, and detail what it means.
- Clarify for every method of `Gizmos` that they should be called for
every frame.
- Clarify which methods belong to 3D or 2D space (kinda obvious for 2D
but still)
The first time I used gizmos I didn't understand how they work and was
confused as to why nothing showed up.
---------
Co-authored-by: François <mockersf@gmail.com>
Co-authored-by: SpecificProtagonist <vincentjunge@posteo.net>
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
- In bevy_polyline, we discovered an issue that happens when line width
is smaller than 1.0 and using perspective. It would sometimes end up
negative or NaN. I'm not entirely sure _why_ it happens.
## Solution
- Make sure the width doesn't go below 0 before multiplying it with the
alpha
# Notes
Here's a link to the bevy_polyline issue
https://github.com/ForesightMiningSoftwareCorporation/bevy_polyline/issues/46
I'm not sure if the solution is correct but it solved the issue in my
testing.
Co-authored-by: François <mockersf@gmail.com>
# Objective
This PR continues https://github.com/bevyengine/bevy/pull/8885
It aims to improve the `Mesh` documentation in the following ways:
- Put everything at the "top level" instead of the "impl".
- Explain better what is a Mesh, how it can be created, and that it can
be edited.
- Explain it can be used with a `Material`, and mention
`StandardMaterial`, `PbrBundle`, `ColorMaterial`, and
`ColorMesh2dBundle` since those cover most cases
- Mention the glTF/Bevy vocabulary discrepancy for "Mesh"
- Add an image for the example
- Various nitpicky modifications
## Note
- The image I added is 90.3ko which I think is small enough?
- Since rustdoc doesn't allow cross-reference not in dependencies of a
subcrate [yet](https://github.com/rust-lang/rust/issues/74481), I have a
lot of backtick references that are not links :(
- Since rustdoc doesn't allow linking to code in the crate (?) I put
link to github directly.
- Since rustdoc doesn't allow embed images in doc
[yet](https://github.com/rust-lang/rust/issues/32104), maybe
[soon](https://github.com/rust-lang/rfcs/pull/3397), I had to put only a
link to the image. I don't think it's worth adding
[embed_doc_image](https://docs.rs/embed-doc-image/latest/embed_doc_image/)
as a dependency for this.
# Objective
- Fix shader_material_glsl example
## Solution
- Expose the `PER_OBJECT_BUFFER_BATCH_SIZE` shader def through the
default `MeshPipeline` specialization.
- Make use of it in the `custom_material.vert` shader to access the mesh
binding.
---
## Changelog
- Added: Exposed the `PER_OBJECT_BUFFER_BATCH_SIZE` shader def through
the default `MeshPipeline` specialization to use in custom shaders not
using bevy_pbr::mesh_bindings that still want to use the mesh binding in
some way.
# Objective
- The `path` module was getting fairly large.
- The code in `AccessRef::read_element` and mut equivalent was very
complex and difficult to understand.
- The `ReflectPathError` had a lot of variants, and was difficult to
read.
## Solution
- Split the file in two, `access` now has its own module
- Rewrite the `read_element` methods, they were ~200 lines long, they
are now ~70 lines long — I didn't change any of the logic. It's really
just the same code, but error handling is separated.
- Split the `ReflectPathError` error
- Merge `AccessRef` and `Access`
- A few other changes that aim to reduce code complexity
### Fully detailed change list
- `Display` impl of `ParsedPath` now includes prefix dots — this allows
simplifying its implementation, and IMO `.path.to.field` is a better way
to express a "path" than `path.to.field` which could suggest we are
reading the `to` field of a variable named `path`
- Add a test to check that dot prefixes and other are correctly parsed —
Until now, no test contained a prefixing dot
- Merge `Access` and `AccessRef`, using a `Cow<'a, str>`. Generated code
seems to agree with this decision (`ParsedPath::parse` sheds 5% of
instructions)
- Remove `Access::as_ref` since there is no such thing as an `AccessRef`
anymore.
- Rename `AccessRef::to_owned` into `AccessRef::into_owned()` since it
takes ownership of `self` now.
- Add a `parse_static` that doesn't allocate new strings for named
fields!
- Add a section about path reflection in the `bevy_reflect` crate root
doc — I saw a few people that weren't aware of path reflection, so I
thought it was pertinent to add it to the root doc
- a lot of nits
- rename `index` to `offset` when it refers to offset in the path string
— There is no more confusion with the other kind of indices in this
context, also it's a common naming convention for parsing.
- Make a dedicated enum for parsing errors
- rename the `read_element` methods to `element` — shorter, but also
`read_element_mut` was a fairly poor name
- The error values now not only contain the expected type but also the
actual type.
- Remove lifetimes that could be inferred from the `GetPath` trait
methods.
---
## Change log
- Added the `ParsedPath::parse_static` method, avoids allocating when
parsing `&'static str`.
## Migration Guide
If you were matching on the `Err(ReflectPathError)` value returned by
`GetPath` and `ParsedPath` methods, now only the parse-related errors
and the offset are publicly accessible. You can always use the
`fmt::Display` to get a clear error message, but if you need
programmatic access to the error types, please open an issue.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com>
- attempt to clarify with better docstrings the default behaviour of
WindowPlugin and the component type it accepts.
# Objective
- I'm new to Rust and Bevy, I got a bit confused about how to customise
some window parameters (title, height, width etc) and while the docs do
show the struct code for that field `Option<Window>` I was a bit of a
doofus and skipped that to read the docstring entry for `primary_window`
and then the `Window` component directly which aren't linked together.
This is a minor improvement which I think will help in-case others, like
me, have a doofus moment.
---------
Co-authored-by: Sélène Amanita <134181069+Selene-Amanita@users.noreply.github.com>
# Objective
The documentation for the `print_when_completed` system stated that this
system would tick the `Timer` component on every entity in the scene.
This was incorrect as this system only ticks the `Timer` on entities
with the `PrintOnCompletionTimer` component.
## Solution
We suggest a modification to the documentation of this system to make it
more clear.
# Objective
This PR updates the name of the enum variant used in the docs for
`OrthographicProjection`.
## Solution
- Change the outdated 'WindowScale` to `WindowSize`.
# Objective
- Reduce the number of rebindings to enable batching of draw commands
## Solution
- Use the new `GpuArrayBuffer` for `MeshUniform` data to store all
`MeshUniform` data in arrays within fewer bindings
- Sort opaque/alpha mask prepass, opaque/alpha mask main, and shadow
phases also by the batch per-object data binding dynamic offset to
improve performance on WebGL2.
---
## Changelog
- Changed: Per-object `MeshUniform` data is now managed by
`GpuArrayBuffer` as arrays in buffers that need to be indexed into.
## Migration Guide
Accessing the `model` member of an individual mesh object's shader
`Mesh` struct the old way where each `MeshUniform` was stored at its own
dynamic offset:
```rust
struct Vertex {
@location(0) position: vec3<f32>,
};
fn vertex(vertex: Vertex) -> VertexOutput {
var out: VertexOutput;
out.clip_position = mesh_position_local_to_clip(
mesh.model,
vec4<f32>(vertex.position, 1.0)
);
return out;
}
```
The new way where one needs to index into the array of `Mesh`es for the
batch:
```rust
struct Vertex {
@builtin(instance_index) instance_index: u32,
@location(0) position: vec3<f32>,
};
fn vertex(vertex: Vertex) -> VertexOutput {
var out: VertexOutput;
out.clip_position = mesh_position_local_to_clip(
mesh[vertex.instance_index].model,
vec4<f32>(vertex.position, 1.0)
);
return out;
}
```
Note that using the instance_index is the default way to pass the
per-object index into the shader, but if you wish to do custom rendering
approaches you can pass it in however you like.
---------
Co-authored-by: robtfm <50659922+robtfm@users.noreply.github.com>
Co-authored-by: Elabajaba <Elabajaba@users.noreply.github.com>
# Objective
My attempt at implementing #7515
## Solution
Added struct `Pitch` and implemented on it `Source` trait.
## Changelog
### Added
- File pitch.rs to bevy_audio crate
- Struct `Pitch` and type aliases for `AudioSourceBundle<Pitch>` and
`SpatialAudioSourceBundle<Pitch>`
- New example showing how to use `PitchBundle`
### Changed
- `AudioPlugin` now adds system for `Pitch` audio
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
The previous formatting didn't render as you'd expect, with 'For CSS
Grid containers' getting adopted by the prior bullet point. Rather than
fixing that directly I opted for a slight reformatting for consistency
with other fields, notably left/right/top/bottom.
# Objective
Fixes#9298 - Default window title leaks "bevy" context
## Solution
I just replaced the literal string "Bevy App" with "App" in Window's
Default trait implementation.
# Objective
Cloning a `WorldQuery` type's "fetch" struct was made unsafe in #5593,
by adding the `unsafe fn clone_fetch` to `WorldQuery`. However, as that
method's documentation explains, it is not the right place to put the
safety invariant:
> While calling this method on its own cannot cause UB it is marked
`unsafe` as the caller must ensure that the returned value is not used
in any way that would cause two `QueryItem<Self>` for the same
`archetype_index` or `table_row` to be alive at the same time.
You can clone a fetch struct all you want and it will never cause
undefined behavior -- in order for something to go wrong, you need to
improperly call `WorldQuery::fetch` with it (which is marked unsafe).
Additionally, making it unsafe to clone a fetch struct does not even
prevent undefined behavior, since there are other ways to incorrectly
use a fetch struct. For example, you could just call fetch more than
once for the same entity, which is not currently forbidden by any
documented invariants.
## Solution
Document a safety invariant on `WorldQuery::fetch` that requires the
caller to not create aliased `WorldQueryItem`s for mutable types. Remove
the `clone_fetch` function, and add the bound `Fetch: Clone` instead.
---
## Changelog
- Removed the associated function `WorldQuery::clone_fetch`, and added a
`Clone` bound to `WorldQuery::Fetch`.
## Migration Guide
### `fetch` invariants
The function `WorldQuery::fetch` has had the following safety invariant
added:
> If this type does not implement `ReadOnlyWorldQuery`, then the caller
must ensure that it is impossible for more than one `Self::Item` to
exist for the same entity at any given time.
This invariant was always required for soundness, but was previously
undocumented. If you called this function manually anywhere, you should
check to make sure that this invariant is not violated.
### Removed `clone_fetch`
The function `WorldQuery::clone_fetch` has been removed. The associated
type `WorldQuery::Fetch` now has the bound `Clone`.
Before:
```rust
struct MyFetch<'w> { ... }
unsafe impl WorldQuery for MyQuery {
...
type Fetch<'w> = MyFetch<'w>
unsafe fn clone_fetch<'w>(fetch: &Self::Fetch<'w>) -> Self::Fetch<'w> {
MyFetch {
field1: fetch.field1,
field2: fetch.field2.clone(),
...
}
}
}
```
After:
```rust
#[derive(Clone)]
struct MyFetch<'w> { ... }
unsafe impl WorldQuery for MyQuery {
...
type Fetch<'w> = MyFetch<'w>;
}
```
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
Fixes https://github.com/bevyengine/bevy/issues/9234
re-breaks: The issues that were linked in #9169
## Solution
Revert the PR that broke tonemapping/postprocessing/etc.
Any passes that are post msaa resolve need to use the main textures, not
the msaa texture.
## Changelog
Idk what to put here since it's a revert.
# Objective
- CI job `check-bans` fails often for unrelated reasons to a PR. Reduce
those failures
## Solution
- Currently, the job only runs if a `Cargo.toml` file changed. This PR
would run the job only if the output of `cargo tree --depth 3` is
different in a PR from main. the job would still always run on main
# Objective
Fixes#9097
## Solution
Reorder the `ExtractSchedule` so that the `extract_text_uinodes` and
`extract_uinode_borders` systems are run after `extract_atlas_uinodes`.
## Changelog
`bevy_ui::render`:
* Added the `ExtractAtlasNode` variant to `RenderUiSystem`.
* Changed `ExtractSchedule` so that `extract_uinode_borders` and
`extract_text_uinodes` run after `extract_atlas_uinodes`.
This is not used directly within the rendering code.
# Objective
- Remove extraneous dependency on `wgpu-hal` as it is not used.
## Solution
- The dependency has been removed and should have no externally visible
impact.
