Feature-gate all references to bevy_text in bevy_ui (#11391)

# Objective

- `bevy_ui` fails to compile without `bevy_text` being enabled.
- Fixes #11363.

## Solution

- Add `#[cfg(feature = "bevy_text")]` to all items that require it.

I think this change is honestly a bit ugly, but I can't see any other
way around it. I considered making `bevy_text` required, but we agreed
[on
Discord](https://discord.com/channels/691052431525675048/743663673393938453/1196868117486379148)
that there were some use cases for `bevy_ui` without `bevy_text`. If you
have any ideas that decreases the amount of `#[cfg(...)]`s and
`#[allow(...)]`s, that would be greatly appreciated.

This was tested by running the following commands:

```shell
$ cargo clippy -p bevy_ui
$ cargo clippy -p bevy_ui -F bevy_text
$ cargo run -p ci
```

---

## Changelog

- Fixed `bevy_ui` not compiling without `bevy_text`.
This commit is contained in:
BD103 2024-01-28 11:24:54 -05:00 committed by GitHub
parent 6e959db134
commit 069a8776f5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 71 additions and 63 deletions

View file

@ -12,8 +12,6 @@ pub mod widget;
use bevy_derive::{Deref, DerefMut};
use bevy_reflect::Reflect;
#[cfg(feature = "bevy_text")]
use bevy_text::TextLayoutInfo;
#[cfg(feature = "bevy_text")]
mod accessibility;
mod focus;
mod geometry;
@ -40,8 +38,6 @@ pub mod prelude {
};
}
#[cfg(feature = "bevy_text")]
use crate::widget::TextFlags;
use bevy_app::prelude::*;
use bevy_ecs::prelude::*;
use bevy_input::InputSystem;
@ -81,6 +77,14 @@ impl Default for UiScale {
}
}
// Marks systems that can be ambiguous with [`widget::text_system`] if the `bevy_text` feature is enabled.
// See https://github.com/bevyengine/bevy/pull/11391 for more details.
#[derive(SystemSet, Debug, Hash, PartialEq, Eq, Clone)]
struct AmbiguousWithTextSystem;
#[derive(SystemSet, Debug, Hash, PartialEq, Eq, Clone)]
struct AmbiguousWithUpdateText2DLayout;
impl Plugin for UiPlugin {
fn build(&self, app: &mut App) {
app.init_resource::<UiSurface>()
@ -129,51 +133,6 @@ impl Plugin for UiPlugin {
ui_focus_system.in_set(UiSystem::Focus).after(InputSystem),
);
#[cfg(feature = "bevy_text")]
app.register_type::<TextLayoutInfo>()
.register_type::<TextFlags>();
// add these systems to front because these must run before transform update systems
#[cfg(feature = "bevy_text")]
app.add_systems(
PostUpdate,
(
widget::measure_text_system
.before(UiSystem::Layout)
// Potential conflict: `Assets<Image>`
// In practice, they run independently since `bevy_render::camera_update_system`
// will only ever observe its own render target, and `widget::measure_text_system`
// will never modify a pre-existing `Image` asset.
.ambiguous_with(bevy_render::camera::CameraUpdateSystem)
// Potential conflict: `Assets<Image>`
// Since both systems will only ever insert new [`Image`] assets,
// they will never observe each other's effects.
.ambiguous_with(bevy_text::update_text2d_layout)
// We assume Text is on disjoint UI entities to UiImage and UiTextureAtlasImage
// FIXME: Add an archetype invariant for this https://github.com/bevyengine/bevy/issues/1481.
.ambiguous_with(widget::update_image_content_size_system),
widget::text_system
.after(UiSystem::Layout)
.after(bevy_text::remove_dropped_font_atlas_sets)
// Text2d and bevy_ui text are entirely on separate entities
.ambiguous_with(bevy_text::update_text2d_layout),
),
);
#[cfg(feature = "bevy_text")]
app.add_plugins(accessibility::AccessibilityPlugin);
app.add_systems(PostUpdate, {
let system = widget::update_image_content_size_system.before(UiSystem::Layout);
// Potential conflicts: `Assets<Image>`
// They run independently since `widget::image_node_system` will only ever observe
// its own UiImage, and `widget::text_system` & `bevy_text::update_text2d_layout`
// will never modify a pre-existing `Image` asset.
#[cfg(feature = "bevy_text")]
let system = system
.ambiguous_with(bevy_text::update_text2d_layout)
.ambiguous_with(widget::text_system);
system
});
app.add_systems(
PostUpdate,
(
@ -189,18 +148,29 @@ impl Plugin for UiPlugin {
.after(UiSystem::Layout)
// clipping doesn't care about outlines
.ambiguous_with(update_clipping_system)
.ambiguous_with(widget::text_system),
.in_set(AmbiguousWithTextSystem),
ui_stack_system
.in_set(UiSystem::Stack)
// the systems don't care about stack index
.ambiguous_with(update_clipping_system)
.ambiguous_with(resolve_outlines_system)
.ambiguous_with(ui_layout_system)
.ambiguous_with(widget::text_system),
.in_set(AmbiguousWithTextSystem),
update_clipping_system.after(TransformSystem::TransformPropagate),
// Potential conflicts: `Assets<Image>`
// They run independently since `widget::image_node_system` will only ever observe
// its own UiImage, and `widget::text_system` & `bevy_text::update_text2d_layout`
// will never modify a pre-existing `Image` asset.
widget::update_image_content_size_system
.before(UiSystem::Layout)
.in_set(AmbiguousWithTextSystem)
.in_set(AmbiguousWithUpdateText2DLayout),
),
);
#[cfg(feature = "bevy_text")]
build_text_interop(app);
build_ui_render(app);
}
@ -212,3 +182,50 @@ impl Plugin for UiPlugin {
render_app.init_resource::<UiPipeline>();
}
}
/// A function that should be called from [`UiPlugin::build`] when [`bevy_text`] is enabled.
#[cfg(feature = "bevy_text")]
fn build_text_interop(app: &mut App) {
use crate::widget::TextFlags;
use bevy_text::TextLayoutInfo;
app.register_type::<TextLayoutInfo>()
.register_type::<TextFlags>();
app.add_systems(
PostUpdate,
(
widget::measure_text_system
.before(UiSystem::Layout)
// Potential conflict: `Assets<Image>`
// In practice, they run independently since `bevy_render::camera_update_system`
// will only ever observe its own render target, and `widget::measure_text_system`
// will never modify a pre-existing `Image` asset.
.ambiguous_with(bevy_render::camera::CameraUpdateSystem)
// Potential conflict: `Assets<Image>`
// Since both systems will only ever insert new [`Image`] assets,
// they will never observe each other's effects.
.ambiguous_with(bevy_text::update_text2d_layout)
// We assume Text is on disjoint UI entities to UiImage and UiTextureAtlasImage
// FIXME: Add an archetype invariant for this https://github.com/bevyengine/bevy/issues/1481.
.ambiguous_with(widget::update_image_content_size_system),
widget::text_system
.after(UiSystem::Layout)
.after(bevy_text::remove_dropped_font_atlas_sets)
// Text2d and bevy_ui text are entirely on separate entities
.ambiguous_with(bevy_text::update_text2d_layout),
),
);
app.add_plugins(accessibility::AccessibilityPlugin);
app.configure_sets(
PostUpdate,
AmbiguousWithTextSystem.ambiguous_with(widget::text_system),
);
app.configure_sets(
PostUpdate,
AmbiguousWithUpdateText2DLayout.ambiguous_with(bevy_text::update_text2d_layout),
);
}

View file

@ -14,9 +14,9 @@ pub use render_pass::*;
pub use ui_material_pipeline::*;
use crate::{
BackgroundColor, BorderColor, CalculatedClip, ContentSize, Node, Style, UiImage, UiScale, Val,
BackgroundColor, BorderColor, CalculatedClip, ContentSize, DefaultUiCamera, Node, Outline,
Style, TargetCamera, UiImage, UiScale, Val,
};
use crate::{DefaultUiCamera, Outline, TargetCamera};
use bevy_app::prelude::*;
use bevy_asset::{load_internal_asset, AssetEvent, AssetId, Assets, Handle};
@ -34,7 +34,6 @@ use bevy_render::{
view::{ExtractedView, ViewUniforms},
Extract, RenderApp, RenderSet,
};
#[cfg(feature = "bevy_text")]
use bevy_sprite::TextureAtlasLayout;
#[cfg(feature = "bevy_text")]
use bevy_text::{PositionedGlyph, Text, TextLayoutInfo};

View file

@ -1,14 +1,6 @@
use crate::{measurement::AvailableSpace, ContentSize, Measure, Node, UiImage, UiScale};
use bevy_asset::Assets;
use bevy_ecs::change_detection::DetectChanges;
use bevy_ecs::query::Without;
use bevy_ecs::{
prelude::Component,
query::With,
reflect::ReflectComponent,
system::{Local, Query, Res},
};
use bevy_ecs::prelude::*;
use bevy_math::Vec2;
use bevy_reflect::{std_traits::ReflectDefault, Reflect};
use bevy_render::texture::Image;