# Objective
As discussed in https://github.com/bevyengine/bevy/issues/7386, system
order ambiguities within `DefaultPlugins` are a source of bugs in the
engine and badly pollute diagnostic output for users.
We should eliminate them!
This PR is an alternative to #15027: with all external ambiguities
silenced, this should be much less prone to merge conflicts and the test
output should be much easier for authors to understand.
Note that system order ambiguities are still permitted in the
`RenderApp`: these need a bit of thought in terms of how to test them,
and will be fairly involved to fix. While these aren't *good*, they'll
generally only cause graphical bugs, not logic ones.
## Solution
All remaining system order ambiguities have been resolved.
Review this PR commit-by-commit to see how each of these problems were
fixed.
## Testing
`cargo run --example ambiguity_detection` passes with no panics or
logging!
# Objective
Fixes https://github.com/bevyengine/bevy/issues/14183
## Solution
Reimplement the UI texture atlas slicer using a shader.
The problems with #14183 could be fixed more simply by hacking around
with the coordinates and scaling but that way is very fragile and might
get broken again the next time we make changes to the layout
calculations. A shader based solution is more robust, it's impossible
for gaps to appear between the image slices with these changes as we're
only drawing a single quad.
I've not tried any benchmarks yet but it should much more efficient as
well, in the worst cases even hundreds or thousands of times faster.
Maybe could have used the UiMaterialPipeline. I wrote the shader first
and used fat vertices and then realised it wouldn't work that way with a
UiMaterial. If it's rewritten it so it puts all the slice geometry in
uniform buffer, then it might work? Adding the uniform buffer would
probably make the shader more complicated though, so don't know if it's
even worth it. Instancing is another alternative.
## Testing
The examples are working and it seems to match the old API correctly but
I've not used the texture atlas slicing API for anything before, I
reviewed the PR but that was back in January.
Needs a review by someone who knows the rendering pipeline and wgsl
really well because I don't really have any idea what I'm doing.
# Objective
`resolve_outlines_system` wasn't updated when multi-window support was
added and it always uses the size of the primary window when resolving
viewport coords, regardless of the layout's camera target.
Fixes#14945
## Solution
It's awkward to get the viewport size of the target for an individual
node without walking the tree or adding extra fields to `Node`, so I
removed `resolve_outlines_system` and instead the outline values are
updated in `ui_layout_system`.
---------
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
# Objective
This `apply_deferred` doesn't seem to have any effect, pointlessly
restricts parallelism and is responsible for a large number of system
order ambiguities. Spotted as part of #7386.
## Solution
Remove it.
This is the *only* manual apply_deferred in the code base currently.
## Testing
I've checked various UI examples and `split_screen`, and couldn't
discern any difference.
This looks like a remnant of a `(a, apply_deferred, b).chain()` pattern
where `b` got removed, leaving us with a weird vestige.
# Objective
While tackling https://github.com/bevyengine/bevy/issues/7386, I noticed
a few nits / frustrations with the existing testing infrastructure.
Rather than mixing those changes in with much more challenging to review
changes to reduce ambiguities, I've split this work into its own PR.
## Solution
Substantially simplify the `ambiguity_detection` test code, and reduce
the verbosity of logging.
## Testing
When run locally, the test functions as expected, with somewhat cleaner
logging.
---------
Co-authored-by: Jan Hohenheim <jan@hohenheim.ch>
# Objective
Fixes https://github.com/bevyengine/bevy/issues/14277.
May also fix https://github.com/bevyengine/bevy/issues/14255, needs
verification.
## Solution
Explicitly order `CameraUpdateSystem` before `UiSystem::Prepare`, so
that when the window resizes, `camera_system` will update the `Camera`'s
viewport size before `ui_layout_system` also reacts to the window resize
and tries to read the new `Camera` viewport size to set UI node sizes
accordingly.
## Testing
I tested that explicitly ordering `CameraUpdateSystem` _after_ triggers
the buggy behavior, and explicitly ordering it _before_ does not trigger
the buggy behavior or crash the app (which also demonstrates that the
system sets are ambiguous).
---
## Migration Guide
`CameraUpdateSystem` is now explicitly ordered before
`UiSystem::Prepare` instead of being ambiguous with it.
# Objective
Resolve possible ambiguity detection panic between `time_system` and
`event_update_system`.
Fixes#14524
## Solution
Sets `.ambiguous_with(event_update_system)` on `time_system`. This is
slightly new territory for me, so please treat with scepticism.
## Testing
As described in the issue, added
```
.configure_schedules(ScheduleBuildSettings {
ambiguity_detection: LogLevel::Error,
..default()
})
```
to the `time` example and ran it.
Progress towards https://github.com/bevyengine/bevy/issues/7386.
Following discussion
https://discord.com/channels/691052431525675048/1253260494538539048/1253387942311886960
This Pull Request adds an example to detect system order ambiguities,
and also asserts none exist.
A lot of schedules are ignored in ordered to have the test passing, we
should thrive to make them pass, but in other pull requests.
<details><summary>example output <b>summary</b>, without ignored
schedules</summary>
<p>
```txt
$ cargo run --example ambiguity_detection 2>&1 | grep -C 1 "pairs of syst"
2024-06-21T13:17:55.776585Z WARN bevy_ecs::schedule::schedule: Schedule First has ambiguities.
1 pairs of systems with conflicting data access have indeterminate execution order. Consider adding `before`, `after`, or `ambiguous_with` relationships between these:
-- bevy_time::time_system (in set TimeSystem) and bevy_ecs::event::event_update_system (in set EventUpdates)
--
2024-06-21T13:17:55.782265Z WARN bevy_ecs::schedule::schedule: Schedule PreUpdate has ambiguities.
11 pairs of systems with conflicting data access have indeterminate execution order. Consider adding `before`, `after`, or `ambiguous_with` relationships between these:
-- bevy_pbr::prepass::update_mesh_previous_global_transforms and bevy_asset::server::handle_internal_asset_events
--
2024-06-21T13:17:55.809516Z WARN bevy_ecs::schedule::schedule: Schedule PostUpdate has ambiguities.
63 pairs of systems with conflicting data access have indeterminate execution order. Consider adding `before`, `after`, or `ambiguous_with` relationships between these:
-- bevy_ui::accessibility::image_changed and bevy_ecs::schedule::executor::apply_deferred
--
2024-06-21T13:17:55.816287Z WARN bevy_ecs::schedule::schedule: Schedule Last has ambiguities.
3 pairs of systems with conflicting data access have indeterminate execution order. Consider adding `before`, `after`, or `ambiguous_with` relationships between these:
-- bevy_gizmos::update_gizmo_meshes<bevy_gizmos::aabb::AabbGizmoConfigGroup> (in set UpdateGizmoMeshes) and bevy_gizmos::update_gizmo_meshes<bevy_gizmos::light::LightGizmoConfigGroup> (in set UpdateGizmoMeshes)
--
2024-06-21T13:17:55.831074Z WARN bevy_ecs::schedule::schedule: Schedule ExtractSchedule has ambiguities.
296 pairs of systems with conflicting data access have indeterminate execution order. Consider adding `before`, `after`, or `ambiguous_with` relationships between these:
-- bevy_render::extract_component::extract_components<bevy_sprite::SpriteSource> and bevy_render::render_asset::extract_render_asset<bevy_sprite::mesh2d::material::PreparedMaterial2d<bevy_sprite::mesh2d::color_material::ColorMaterial>>
```
</p>
</details>
To try locally:
```sh
CI_TESTING_CONFIG="./.github/example-run/ambiguity_detection.ron" cargo run --example ambiguity_detection --features "bevy_ci_testing,trace,trace_chrome"
```
---------
Co-authored-by: Jan Hohenheim <jan@hohenheim.ch>
