Use warn_once where relevant instead of manually implementing a single warn check (#11693)

# Objective - Some places manually use a `bool` /`AtomicBool` to warn once. ## Solution - Use the `warn_once` macro which internally creates an `AtomicBool`. Downside: in some case the warning state would have been reset after recreating the struct carrying the warn state, whereas now it will always warn only once per program run (For example, if all `MeshPipeline`s are dropped or the `World` is recreated for `Local<bool>`/ a `bool` resource, which shouldn't happen over the course of a standard `App` run). --- ## Changelog ### Removed - `FontAtlasWarning` has been removed, but the corresponding warning is still emitted.
2024-09-20 06:22:01 +00:00 · 2024-02-05 22:05:43 +01:00 · 2024-02-05 22:05:43 +01:00 · 312df3cec7
commit 312df3cec7
parent 8faaef17e5
7 changed files with 15 additions and 55 deletions
--- a/crates/bevy_pbr/src/render/mesh.rs
+++ b/crates/bevy_pbr/src/render/mesh.rs
@ -38,13 +38,7 @@ use std::cell::Cell;
 use thread_local::ThreadLocal;

 #[cfg(debug_assertions)]
-use bevy_utils::tracing::warn;
-
-#[cfg(debug_assertions)]
-use std::sync::{
-    atomic::{AtomicBool, Ordering},
-    Arc,
-};
+use bevy_utils::warn_once;

 use crate::render::{
    morph::{
@ -372,9 +366,6 @@ pub struct MeshPipeline {
    ///
    /// This affects whether reflection probes can be used.
    pub binding_arrays_are_usable: bool,
-
-    #[cfg(debug_assertions)]
-    pub did_warn_about_too_many_textures: Arc<AtomicBool>,
 }

 impl FromWorld for MeshPipeline {
@ -432,8 +423,6 @@ impl FromWorld for MeshPipeline {
            mesh_layouts: MeshLayouts::new(&render_device),
            per_object_buffer_batch_size: GpuArrayBuffer::<MeshUniform>::batch_size(&render_device),
            binding_arrays_are_usable: binding_arrays_are_usable(&render_device),
-            #[cfg(debug_assertions)]
-            did_warn_about_too_many_textures: Arc::new(AtomicBool::new(false)),
        }
    }
 }
@ -460,14 +449,9 @@ impl MeshPipeline {
        let layout = &self.view_layouts[index];

        #[cfg(debug_assertions)]
-        if layout.texture_count > MESH_PIPELINE_VIEW_LAYOUT_SAFE_MAX_TEXTURES
-            && !self.did_warn_about_too_many_textures.load(Ordering::SeqCst)
-        {
-            self.did_warn_about_too_many_textures
-                .store(true, Ordering::SeqCst);
-
+        if layout.texture_count > MESH_PIPELINE_VIEW_LAYOUT_SAFE_MAX_TEXTURES {
            // Issue our own warning here because Naga's error message is a bit cryptic in this situation
-            warn!("Too many textures in mesh pipeline view layout, this might cause us to hit `wgpu::Limits::max_sampled_textures_per_shader_stage` in some environments.");
+            warn_once!("Too many textures in mesh pipeline view layout, this might cause us to hit `wgpu::Limits::max_sampled_textures_per_shader_stage` in some environments.");
        }

        &layout.bind_group_layout
--- a/crates/bevy_render/src/extract_resource.rs
+++ b/crates/bevy_render/src/extract_resource.rs
@ -42,7 +42,6 @@ pub fn extract_resource<R: ExtractResource>(
    mut commands: Commands,
    main_resource: Extract<Option<Res<R::Source>>>,
    target_resource: Option<ResMut<R>>,
-    #[cfg(debug_assertions)] mut has_warned_on_remove: Local<bool>,
 ) {
    if let Some(main_resource) = main_resource.as_ref() {
        if let Some(mut target_resource) = target_resource {
@ -51,9 +50,8 @@ pub fn extract_resource<R: ExtractResource>(
            }
        } else {
            #[cfg(debug_assertions)]
-            if !main_resource.is_added() && !*has_warned_on_remove {
-                *has_warned_on_remove = true;
-                bevy_log::warn!(
+            if !main_resource.is_added() {
+                bevy_log::warn_once!(
                    "Removing resource {} from render world not expected, adding using `Commands`.
                This may decrease performance",
                    std::any::type_name::<R>()
--- a/crates/bevy_text/src/glyph_brush.rs
+++ b/crates/bevy_text/src/glyph_brush.rs
@ -4,15 +4,15 @@ use bevy_math::{Rect, Vec2};
 use bevy_reflect::Reflect;
 use bevy_render::texture::Image;
 use bevy_sprite::TextureAtlasLayout;
-use bevy_utils::tracing::warn;
+use bevy_utils::warn_once;
 use glyph_brush_layout::{
    BuiltInLineBreaker, FontId, GlyphPositioner, Layout, SectionGeometry, SectionGlyph,
    SectionText, ToSectionText,
 };

 use crate::{
-    error::TextError, BreakLineOn, Font, FontAtlasSet, FontAtlasSets, FontAtlasWarning,
-    GlyphAtlasInfo, JustifyText, TextSettings, YAxisOrientation,
+    error::TextError, BreakLineOn, Font, FontAtlasSet, FontAtlasSets, GlyphAtlasInfo, JustifyText,
+    TextSettings, YAxisOrientation,
 };

 pub struct GlyphBrush {
@ -63,7 +63,6 @@ impl GlyphBrush {
        texture_atlases: &mut Assets<TextureAtlasLayout>,
        textures: &mut Assets<Image>,
        text_settings: &TextSettings,
-        font_atlas_warning: &mut FontAtlasWarning,
        y_axis_orientation: YAxisOrientation,
    ) -> Result<Vec<PositionedGlyph>, TextError> {
        if glyphs.is_empty() {
@ -114,11 +113,9 @@ impl GlyphBrush {
                    })?;

                if !text_settings.allow_dynamic_font_size
-                    && !font_atlas_warning.warned
                    && font_atlas_set.len() > text_settings.soft_max_font_atlases.get()
                {
-                    warn!("warning[B0005]: Number of font atlases has exceeded the maximum of {}. Performance and memory usage may suffer.", text_settings.soft_max_font_atlases.get());
-                    font_atlas_warning.warned = true;
+                    warn_once!("warning[B0005]: Number of font atlases has exceeded the maximum of {}. Performance and memory usage may suffer.", text_settings.soft_max_font_atlases.get());
                }

                let texture_atlas = texture_atlases.get(&atlas_info.texture_atlas).unwrap();
--- a/crates/bevy_text/src/lib.rs
+++ b/crates/bevy_text/src/lib.rs
@ -46,8 +46,7 @@ pub struct TextPlugin;
 #[derive(Resource)]
 pub struct TextSettings {
    /// Soft maximum number of font atlases supported in a [`FontAtlasSet`]. When this is exceeded,
-    /// a warning will be emitted a single time. The [`FontAtlasWarning`] resource ensures that
-    /// this only happens once.
+    /// a warning will be emitted a single time.
    pub soft_max_font_atlases: NonZeroUsize,
    /// Allows font size to be set dynamically exceeding the amount set in `soft_max_font_atlases`.
    /// Note each font size has to be generated which can have a strong performance impact.
@ -63,13 +62,6 @@ impl Default for TextSettings {
    }
 }

-/// This resource tracks whether or not a warning has been emitted due to the number
-/// of font atlases exceeding the [`TextSettings::soft_max_font_atlases`] setting.
-#[derive(Resource, Default)]
-pub struct FontAtlasWarning {
-    warned: bool,
-}
-
 /// Text is rendered for two different view projections, a [`Text2dBundle`] is rendered with a
 /// `BottomToTop` y axis, while UI is rendered with a `TopToBottom` y axis. This matters for text because
 /// the glyph positioning is different in either layout.
@ -90,7 +82,6 @@ impl Plugin for TextPlugin {
            .register_type::<BreakLineOn>()
            .init_asset_loader::<FontLoader>()
            .init_resource::<TextSettings>()
-            .init_resource::<FontAtlasWarning>()
            .init_resource::<FontAtlasSets>()
            .insert_resource(TextPipeline::default())
            .add_systems(
--- a/crates/bevy_text/src/pipeline.rs
+++ b/crates/bevy_text/src/pipeline.rs
@ -1,7 +1,6 @@
 use crate::{
    compute_text_bounds, error::TextError, glyph_brush::GlyphBrush, scale_value, BreakLineOn, Font,
-    FontAtlasSets, FontAtlasWarning, JustifyText, PositionedGlyph, Text, TextSection, TextSettings,
-    YAxisOrientation,
+    FontAtlasSets, JustifyText, PositionedGlyph, Text, TextSection, TextSettings, YAxisOrientation,
 };
 use ab_glyph::PxScale;
 use bevy_asset::{AssetId, Assets, Handle};
@ -54,7 +53,6 @@ impl TextPipeline {
        texture_atlases: &mut Assets<TextureAtlasLayout>,
        textures: &mut Assets<Image>,
        text_settings: &TextSettings,
-        font_atlas_warning: &mut FontAtlasWarning,
        y_axis_orientation: YAxisOrientation,
    ) -> Result<TextLayoutInfo, TextError> {
        let mut scaled_fonts = Vec::with_capacity(sections.len());
@ -97,7 +95,6 @@ impl TextPipeline {
            texture_atlases,
            textures,
            text_settings,
-            font_atlas_warning,
            y_axis_orientation,
        )?;

--- a/crates/bevy_text/src/text2d.rs
+++ b/crates/bevy_text/src/text2d.rs
@ -1,6 +1,6 @@
 use crate::{
-    BreakLineOn, Font, FontAtlasSets, FontAtlasWarning, PositionedGlyph, Text, TextError,
-    TextLayoutInfo, TextPipeline, TextSettings, YAxisOrientation,
+    BreakLineOn, Font, FontAtlasSets, PositionedGlyph, Text, TextError, TextLayoutInfo,
+    TextPipeline, TextSettings, YAxisOrientation,
 };
 use bevy_asset::Assets;
 use bevy_ecs::{
@ -163,7 +163,6 @@ pub fn update_text2d_layout(
    mut textures: ResMut<Assets<Image>>,
    fonts: Res<Assets<Font>>,
    text_settings: Res<TextSettings>,
-    mut font_atlas_warning: ResMut<FontAtlasWarning>,
    windows: Query<&Window, With<PrimaryWindow>>,
    mut scale_factor_changed: EventReader<WindowScaleFactorChanged>,
    mut texture_atlases: ResMut<Assets<TextureAtlasLayout>>,
@ -203,7 +202,6 @@ pub fn update_text2d_layout(
                &mut texture_atlases,
                &mut textures,
                text_settings.as_ref(),
-                &mut font_atlas_warning,
                YAxisOrientation::BottomToTop,
            ) {
                Err(TextError::NoSuchFont) => {
--- a/crates/bevy_ui/src/widget/text.rs
+++ b/crates/bevy_ui/src/widget/text.rs
@ -12,8 +12,8 @@ use bevy_reflect::{std_traits::ReflectDefault, Reflect};
 use bevy_render::texture::Image;
 use bevy_sprite::TextureAtlasLayout;
 use bevy_text::{
-    scale_value, BreakLineOn, Font, FontAtlasSets, FontAtlasWarning, Text, TextError,
-    TextLayoutInfo, TextMeasureInfo, TextPipeline, TextSettings, YAxisOrientation,
+    scale_value, BreakLineOn, Font, FontAtlasSets, Text, TextError, TextLayoutInfo,
+    TextMeasureInfo, TextPipeline, TextSettings, YAxisOrientation,
 };
 use bevy_window::{PrimaryWindow, Window};
 use taffy::style::AvailableSpace;
@ -153,7 +153,6 @@ pub fn measure_text_system(
 fn queue_text(
    fonts: &Assets<Font>,
    text_pipeline: &mut TextPipeline,
-    font_atlas_warning: &mut FontAtlasWarning,
    font_atlas_sets: &mut FontAtlasSets,
    texture_atlases: &mut Assets<TextureAtlasLayout>,
    textures: &mut Assets<Image>,
@ -189,7 +188,6 @@ fn queue_text(
            texture_atlases,
            textures,
            text_settings,
-            font_atlas_warning,
            YAxisOrientation::TopToBottom,
        ) {
            Err(TextError::NoSuchFont) => {
@ -224,7 +222,6 @@ pub fn text_system(
    fonts: Res<Assets<Font>>,
    windows: Query<&Window, With<PrimaryWindow>>,
    text_settings: Res<TextSettings>,
-    mut font_atlas_warning: ResMut<FontAtlasWarning>,
    ui_scale: Res<UiScale>,
    mut texture_atlases: ResMut<Assets<TextureAtlasLayout>>,
    mut font_atlas_sets: ResMut<FontAtlasSets>,
@ -246,7 +243,6 @@ pub fn text_system(
                queue_text(
                    &fonts,
                    &mut text_pipeline,
-                    &mut font_atlas_warning,
                    &mut font_atlas_sets,
                    &mut texture_atlases,
                    &mut textures,
@ -268,7 +264,6 @@ pub fn text_system(
            queue_text(
                &fonts,
                &mut text_pipeline,
-                &mut font_atlas_warning,
                &mut font_atlas_sets,
                &mut texture_atlases,
                &mut textures,