Commit graph

10 commits

Author SHA1 Message Date
Zachary Harrold
4742f74fc4
Broaden "Check for bevy_internal imports" CI Task (#15333)
# Objective

- Fixes #15319
- Fixes #15317

## Solution

- Updated CI task to check for _any_ `bevy_*` crates, rather than just
`bevy_internal`

---------

Co-authored-by: François Mockers <francois.mockers@vleue.com>
2024-09-20 17:08:37 +00:00
Hamir Mahal
ec728c31c1
style: simplify string formatting for readability (#15033)
# Objective

The goal of this change is to improve code readability and
maintainability.
2024-09-03 23:35:49 +00:00
Alice Cecile
4ac2a63556
Remove all existing system order ambiguities in DefaultPlugins (#15031)
# 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!
2024-09-03 20:24:34 +00:00
ickshonpe
01a3b0e830
UI texture atlas slice shader (#14990)
# 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.
2024-09-02 23:03:58 +00:00
ickshonpe
be100b8760
Resolve UI outlines using the correct target's viewport size (#14947)
# 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>
2024-09-02 16:56:58 +00:00
Alice Cecile
5243fe6956
Remove manual apply_deferred in bevy_ui (#14768)
# 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.
2024-08-15 20:51:25 +00:00
Alice Cecile
0c126ee676
Improve ambiguity detection example / test (#14760)
# 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>
2024-08-15 18:30:57 +00:00
Ben Frankel
9b254aab1e
Explicitly order CameraUpdateSystem before UiSystem::Prepare (#14609)
# 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.
2024-08-04 13:34:51 +00:00
Rich Churcher
e579622a65
time_system is ambiguous_with event_update_system (#14544)
# 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.
2024-07-31 12:13:17 +00:00
Thierry Berger
26fc4c7198
Test for ambiguous system ordering in CI (#13950)
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>
2024-07-17 21:05:48 +00:00