From 52f20a9f6af9bda96ebbf40e94f0abde7a4158db Mon Sep 17 00:00:00 2001 From: Kevin Reid Date: Sun, 15 Oct 2023 08:10:45 -0700 Subject: [PATCH] Use `arcstr::ArcStr` in block names, UI, and raytracing. Benefits: * Evaluating blocks or cloning no longer clones display names. * `ui` doesn't need a lazy-allocated `Arc`; closer to no_std. * Text raytracer no longer allocates for its name slices. It also implements grapheme cluster splitting, because that happened to be easy. --- CHANGELOG.md | 3 ++ Cargo.lock | 14 +++++- Cargo.toml | 1 + all-is-cubes-desktop/src/terminal.rs | 4 +- all-is-cubes-ui/Cargo.toml | 1 - all-is-cubes-ui/src/logo.rs | 5 +- all-is-cubes-ui/src/ui_content/options.rs | 4 +- all-is-cubes-ui/src/vui/page.rs | 6 +-- all-is-cubes-ui/src/vui/widgets/text.rs | 4 +- all-is-cubes-ui/src/vui/widgets/tooltip.rs | 25 +++++----- all-is-cubes/Cargo.toml | 8 +++- all-is-cubes/src/block/attributes.rs | 14 +++--- all-is-cubes/src/block/builder.rs | 7 +-- all-is-cubes/src/block/evaluated.rs | 6 ++- all-is-cubes/src/block/tests.rs | 6 +-- all-is-cubes/src/content.rs | 19 ++++++-- all-is-cubes/src/lib.rs | 2 + all-is-cubes/src/op.rs | 3 ++ all-is-cubes/src/raytracer/text.rs | 41 ++++++++-------- all-is-cubes/src/save/conversion.rs | 9 +--- all-is-cubes/src/save/schema.rs | 6 +-- all-is-cubes/src/space.rs | 4 +- all-is-cubes/src/universe.rs | 37 ++++++++++++++- all-is-cubes/tests/alloc.rs | 55 +++++++--------------- 24 files changed, 166 insertions(+), 118 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8604c927d..21690f0c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,9 @@ - All functions using `usize` to identify a coordinate axis now use `math::Axis` instead. `Face6::axis_number()` and `Face7::axis_number()` are now called `axis()`. + - In block names and universe member names, `Cow<'static, str>` and `Arc` have been replaced with `arcstr::ArcStr`. + This type allows both refcounting and static literal strings. + - The following functions have changed signature to use the new type `math::Gridgid`: - `math::GridAab::transform()` - `math::GridMatrix::decompose()` diff --git a/Cargo.lock b/Cargo.lock index 526551b3e..a38a752e3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -55,6 +55,7 @@ version = "0.6.0" dependencies = [ "allocation-counter", "arbitrary", + "arcstr", "base64 0.21.2", "bitflags 2.3.3", "bytemuck", @@ -88,6 +89,7 @@ dependencies = [ "serde_json", "serde_repr", "tokio", + "unicode-segmentation", "yield-progress", ] @@ -257,7 +259,6 @@ dependencies = [ "futures-task", "indoc", "log", - "once_cell", "pretty_assertions", "tokio", ] @@ -422,6 +423,15 @@ dependencies = [ "derive_arbitrary", ] +[[package]] +name = "arcstr" +version = "1.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f907281554a3d0312bb7aab855a8e0ef6cbf1614d06de54105039ca8b34460e" +dependencies = [ + "serde", +] + [[package]] name = "array-init" version = "2.1.0" @@ -1959,7 +1969,7 @@ checksum = "02c69ce7e7c0f17aa18fdd9d0de39727adb9c6281f2ad12f57cbe54ae6e76e7d" dependencies = [ "az", "bytemuck", - "half 1.8.2", + "half 2.3.1", "serde", "typenum", ] diff --git a/Cargo.toml b/Cargo.toml index baab65923..80c5a870e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -79,6 +79,7 @@ thiserror = "1.0.40" # The library crates do not require Tokio. tokio = { version = "1.28.0", default-features = false } trycmd = "0.14.1" # keep in sync with `snapbox` +unicode-segmentation = { version = "1.10.1", default-features = false } wasm-bindgen-futures = "0.4.34" # Note: "expose_ids" feature is not needed globally but is enabled to avoid compiling two versions wgpu = { version = "0.17.0", features = ["expose-ids"] } diff --git a/all-is-cubes-desktop/src/terminal.rs b/all-is-cubes-desktop/src/terminal.rs index 917758eb1..88a98430a 100644 --- a/all-is-cubes-desktop/src/terminal.rs +++ b/all-is-cubes-desktop/src/terminal.rs @@ -1,6 +1,5 @@ //! Rendering as terminal text. Why not? Turn cubes into rectangles. -use std::borrow::Cow; use std::sync::mpsc::{self, TrySendError}; use std::time::{Duration, Instant}; @@ -8,6 +7,7 @@ use anyhow::Context; use crossterm::event::{Event, KeyCode, KeyEvent, KeyModifiers, MouseEvent, MouseEventKind}; use tui::layout::Rect; +use all_is_cubes::arcstr::literal_substr; use all_is_cubes::camera::{self, Camera, StandardCameras, Viewport}; use all_is_cubes::euclid::{Point2D, Vector2D}; use all_is_cubes::listen::{ListenableCell, ListenableSource}; @@ -356,7 +356,7 @@ impl Accumulate for ColorCharacterBuf { fn hit_nothing(&mut self) { self.text - .add(Rgba::TRANSPARENT, &CharacterRtData(Cow::Borrowed(" "))); + .add(Rgba::TRANSPARENT, &CharacterRtData(literal_substr!(" "))); self.override_color = true; } diff --git a/all-is-cubes-ui/Cargo.toml b/all-is-cubes-ui/Cargo.toml index 69585a703..41d90a39e 100644 --- a/all-is-cubes-ui/Cargo.toml +++ b/all-is-cubes-ui/Cargo.toml @@ -23,7 +23,6 @@ futures-core = { workspace = true } futures-task = { workspace = true } indoc = { workspace = true } log = { workspace = true } -once_cell = { workspace = true } [dev-dependencies] futures-channel = { workspace = true } diff --git a/all-is-cubes-ui/src/logo.rs b/all-is-cubes-ui/src/logo.rs index 82420a658..73632d526 100644 --- a/all-is-cubes-ui/src/logo.rs +++ b/all-is-cubes-ui/src/logo.rs @@ -1,8 +1,9 @@ //! Drawing the All is Cubes logo. -use alloc::{borrow::Cow, sync::Arc}; +use alloc::sync::Arc; use all_is_cubes::{ + arcstr::literal, block::Block, content::palette, drawing::{ @@ -22,7 +23,7 @@ pub fn logo_text() -> Arc { let background_text_block: Block = palette::LOGO_STROKE.into(); Arc::new(vui::widgets::LargeText { - text: Cow::Borrowed("All is Cubes"), + text: literal!("All is Cubes"), font: || &FONT_9X15_BOLD, brush: { VoxelBrush::new([ diff --git a/all-is-cubes-ui/src/ui_content/options.rs b/all-is-cubes-ui/src/ui_content/options.rs index f75e6ad21..ae7b7eb43 100644 --- a/all-is-cubes-ui/src/ui_content/options.rs +++ b/all-is-cubes-ui/src/ui_content/options.rs @@ -105,7 +105,7 @@ fn graphics_toggle_button( setter: fn(&mut GraphicsOptions, bool), ) -> WidgetTree { let icon = hud_inputs.hud_blocks.blocks[icon_key].clone(); - let text_label = String::from(icon.evaluate().unwrap().attributes.display_name); + let text_label = icon.evaluate().unwrap().attributes.display_name.clone(); let button: Arc = widgets::ToggleButton::new( hud_inputs.graphics_options.clone(), getter, @@ -132,7 +132,7 @@ fn graphics_toggle_button( children: vec![LayoutTree::leaf(button), { // TODO: extract this for general use and reconcile with pages::parts::shrink() let text: WidgetTree = LayoutTree::leaf(Arc::new(widgets::LargeText { - text: text_label.into(), + text: text_label, font: || &font::FONT_6X10, brush: VoxelBrush::single(block::Block::from(palette::ALMOST_BLACK)), text_style: TextStyle::default(), diff --git a/all-is-cubes-ui/src/vui/page.rs b/all-is-cubes-ui/src/vui/page.rs index 7a57d8561..6736ad9a8 100644 --- a/all-is-cubes-ui/src/vui/page.rs +++ b/all-is-cubes-ui/src/vui/page.rs @@ -1,6 +1,6 @@ -use alloc::borrow::Cow; use alloc::sync::Arc; +use all_is_cubes::arcstr::ArcStr; use all_is_cubes::block::AIR; use all_is_cubes::block::{Block, BlockAttributes, Resolution}; use all_is_cubes::camera; @@ -182,7 +182,7 @@ pub(crate) mod parts { ))) } - pub fn heading(text: impl Into>) -> WidgetTree { + pub fn heading(text: impl Into) -> WidgetTree { LayoutTree::leaf(Arc::new(widgets::LargeText { text: text.into(), font: || &font::FONT_9X15_BOLD, @@ -191,7 +191,7 @@ pub(crate) mod parts { })) } - pub fn paragraph(text: impl Into>) -> WidgetTree { + pub fn paragraph(text: impl Into) -> WidgetTree { LayoutTree::leaf(Arc::new(widgets::LargeText { text: text.into(), font: || &font::FONT_6X10, diff --git a/all-is-cubes-ui/src/vui/widgets/text.rs b/all-is-cubes-ui/src/vui/widgets/text.rs index 10926a284..080babcd4 100644 --- a/all-is-cubes-ui/src/vui/widgets/text.rs +++ b/all-is-cubes-ui/src/vui/widgets/text.rs @@ -1,6 +1,6 @@ -use alloc::borrow::Cow; use alloc::sync::Arc; +use all_is_cubes::arcstr::ArcStr; use all_is_cubes::drawing::embedded_graphics::{ mono_font::{MonoFont, MonoTextStyle}, prelude::{Dimensions, Point}, @@ -22,7 +22,7 @@ use crate::vui::{widgets, LayoutGrant, LayoutRequest, Layoutable, Widget, Widget #[allow(clippy::exhaustive_structs)] // TODO: find a better strategy pub struct LargeText { /// Text to be displayed. - pub text: Cow<'static, str>, + pub text: ArcStr, /// Font with which to draw the text. /// Needs to be a function to be Send+Sync. pub font: fn() -> &'static MonoFont<'static>, diff --git a/all-is-cubes-ui/src/vui/widgets/tooltip.rs b/all-is-cubes-ui/src/vui/widgets/tooltip.rs index 8e5b6e11d..9328d3d04 100644 --- a/all-is-cubes-ui/src/vui/widgets/tooltip.rs +++ b/all-is-cubes-ui/src/vui/widgets/tooltip.rs @@ -2,8 +2,7 @@ use alloc::sync::Arc; use std::error::Error; use std::sync::Mutex; -use once_cell::sync::Lazy; - +use all_is_cubes::arcstr::{literal, ArcStr}; use all_is_cubes::block::{space_to_blocks, AnimationHint, BlockAttributes, Resolution, AIR}; use all_is_cubes::character::{Character, CharacterChange}; use all_is_cubes::drawing::embedded_graphics::{ @@ -21,8 +20,6 @@ use all_is_cubes::universe::{URef, Universe}; use crate::ui_content::hud::{HudBlocks, HudFont}; use crate::vui::{LayoutRequest, Layoutable, Widget, WidgetController, WidgetTransaction}; -static EMPTY_ARC_STR: Lazy> = Lazy::new(|| "".into()); - #[derive(Debug)] pub(crate) struct TooltipState { /// Character we're reading inventory state from @@ -65,7 +62,7 @@ impl TooltipState { } } - pub fn set_message(&mut self, text: Arc) { + pub fn set_message(&mut self, text: ArcStr) { self.dirty_inventory = false; self.set_contents(TooltipContents::Message(text)) } @@ -77,7 +74,7 @@ impl TooltipState { } /// Advances time and returns the string that should be newly written to the screen, if different than the previous call. - fn step(&mut self, hud_blocks: &HudBlocks, tick: Tick) -> Option> { + fn step(&mut self, hud_blocks: &HudBlocks, tick: Tick) -> Option { if let Some(ref mut age) = self.age { *age += tick.delta_t(); if *age > Duration::from_secs(1) { @@ -101,8 +98,8 @@ impl TooltipState { .icon(&hud_blocks.icons) .evaluate() .ok() - .map(|ev_block| ev_block.attributes.display_name.into_owned().into()) - .unwrap_or_else(|| EMPTY_ARC_STR.clone()); + .map(|ev_block| ev_block.attributes.display_name.clone()) + .unwrap_or_else(|| literal!("")); let new_contents = TooltipContents::InventoryItem { source_slot: selected_slot, text: new_text, @@ -160,17 +157,19 @@ enum TooltipContents { /// right away. JustStartedExisting, Blanked, - Message(Arc), + Message(ArcStr), InventoryItem { source_slot: usize, - text: Arc, + text: ArcStr, }, } impl TooltipContents { - fn text(&self) -> &Arc { + fn text(&self) -> &ArcStr { + static EMPTY: ArcStr = literal!(""); + match self { - TooltipContents::JustStartedExisting | TooltipContents::Blanked => &EMPTY_ARC_STR, + TooltipContents::JustStartedExisting | TooltipContents::Blanked => &EMPTY, TooltipContents::Message(m) => m, TooltipContents::InventoryItem { text, .. } => text, } @@ -275,7 +274,7 @@ impl WidgetController for TooltipController { fn step(&mut self, tick: Tick) -> Result> { // None if no update is needed - let text_update: Option> = self + let text_update: Option = self .definition .state .try_lock() diff --git a/all-is-cubes/Cargo.toml b/all-is-cubes/Cargo.toml index d4cc0fe7f..2ba2c2a1c 100644 --- a/all-is-cubes/Cargo.toml +++ b/all-is-cubes/Cargo.toml @@ -50,7 +50,11 @@ harness = false [features] default = ["std"] # Adds std-dependent functionality such as thread safety. -std = ["dep:once_cell", "yield-progress/sync"] +std = [ + "dep:once_cell", + "arcstr/std", # not required but nicer behavior + "yield-progress/sync", +] # Adds `impl arbitrary::Arbitrary for ...` # Note: Not using euclid/arbitrary because it's broken arbitrary = ["dep:arbitrary", "ordered-float/arbitrary"] @@ -71,6 +75,7 @@ threads = ["std", "dep:rayon"] [dependencies] arbitrary = { workspace = true, optional = true } +arcstr = { version = "1.1.5", default-features = false, features = ["serde", "substr"] } base64 = { workspace = true, optional = true, features = ["std"] } # used in serialization bitflags = { workspace = true } bytemuck = { workspace = true, features = ["derive"] } @@ -105,6 +110,7 @@ re_sdk = { workspace = true, optional = true } # rc feature needed because we are [de]serializing `Arc`s serde = { workspace = true, optional = true, features = ["derive", "rc"] } serde_repr = { version = "0.1.12", optional = true, default-features = false } +unicode-segmentation = { workspace = true } yield-progress = { workspace = true } [build-dependencies] diff --git a/all-is-cubes/src/block/attributes.rs b/all-is-cubes/src/block/attributes.rs index 703e22971..881efb4af 100644 --- a/all-is-cubes/src/block/attributes.rs +++ b/all-is-cubes/src/block/attributes.rs @@ -1,9 +1,10 @@ //! [`BlockAttributes`] and closely related types. -use alloc::borrow::Cow; use core::fmt; use core::num::NonZeroU16; +use arcstr::ArcStr; + use crate::math::{Face6, GridRotation}; use crate::op::Operation; use crate::universe::VisitRefs; @@ -26,9 +27,10 @@ pub struct BlockAttributes { /// /// The default value is the empty string. The empty string should be considered a /// reasonable choice for solid-color blocks with no special features. - // --- - // TODO: Make this a refcounted type so we can make cloning O(1). - pub display_name: Cow<'static, str>, + //--- + // Design note: The use of `ArcStr` allows cloning a `BlockAttributes` to be O(1) + // allocate no additional memory. + pub display_name: ArcStr, /// Whether players' [cursors](crate::character::Cursor) target it or pass through it. /// @@ -92,7 +94,7 @@ impl fmt::Debug for BlockAttributes { impl BlockAttributes { const DEFAULT: Self = BlockAttributes { - display_name: Cow::Borrowed(""), + display_name: arcstr::literal!(""), selectable: true, rotation_rule: RotationPlacementRule::Never, tick_action: None, @@ -159,7 +161,7 @@ impl Default for BlockAttributes { impl<'a> arbitrary::Arbitrary<'a> for BlockAttributes { fn arbitrary(u: &mut arbitrary::Unstructured<'a>) -> arbitrary::Result { Ok(BlockAttributes { - display_name: Cow::Owned(u.arbitrary()?), + display_name: u.arbitrary::()?.into(), selectable: u.arbitrary()?, rotation_rule: u.arbitrary()?, tick_action: None, // TODO: need Arbitrary for Block diff --git a/all-is-cubes/src/block/builder.rs b/all-is-cubes/src/block/builder.rs index a595872e2..bb1361363 100644 --- a/all-is-cubes/src/block/builder.rs +++ b/all-is-cubes/src/block/builder.rs @@ -1,9 +1,10 @@ //! Lesser-used helpers for [`BlockBuilder`]. -use alloc::borrow::Cow; use alloc::sync::Arc; use alloc::vec::Vec; +use arcstr::ArcStr; + use crate::block::{ AnimationHint, Atom, Block, BlockAttributes, BlockCollision, BlockDef, BlockParts, BlockPtr, Modifier, Primitive, Resolution, RotationPlacementRule, AIR, @@ -28,7 +29,7 @@ use crate::universe::{Name, URef, Universe}; /// /// assert_eq!(block.evaluate().unwrap().color, Rgba::new(0.5, 0.5, 0., 1.)); /// assert_eq!( -/// block.evaluate().unwrap().attributes.display_name.as_ref(), +/// block.evaluate().unwrap().attributes.display_name.as_str(), /// "BROWN", /// ); /// ``` @@ -72,7 +73,7 @@ impl BlockBuilder { } /// Sets the value for [`BlockAttributes::display_name`]. - pub fn display_name(mut self, value: impl Into>) -> Self { + pub fn display_name(mut self, value: impl Into) -> Self { self.attributes.display_name = value.into(); self } diff --git a/all-is-cubes/src/block/evaluated.rs b/all-is-cubes/src/block/evaluated.rs index 89587ff80..e3483b995 100644 --- a/all-is-cubes/src/block/evaluated.rs +++ b/all-is-cubes/src/block/evaluated.rs @@ -595,6 +595,10 @@ pub const AIR_EVALUATED: EvaluatedBlock = EvaluatedBlock { voxel_opacity_mask: None, }; +/// This separate item is needed to convince the compiler that `AIR_ATTRIBUTES.display_name` +/// isn't being dropped if we return `&AIR_EVALUATED`. +pub(crate) const AIR_EVALUATED_REF: &EvaluatedBlock = &AIR_EVALUATED; + pub(super) const AIR_EVALUATED_MIN: MinEval = MinEval { attributes: AIR_ATTRIBUTES, voxels: Evoxels::One(Evoxel::AIR), @@ -602,7 +606,7 @@ pub(super) const AIR_EVALUATED_MIN: MinEval = MinEval { /// Used only by [`AIR_EVALUATED`]. const AIR_ATTRIBUTES: BlockAttributes = BlockAttributes { - display_name: alloc::borrow::Cow::Borrowed(""), + display_name: arcstr::literal!(""), selectable: false, rotation_rule: block::RotationPlacementRule::Never, tick_action: None, diff --git a/all-is-cubes/src/block/tests.rs b/all-is-cubes/src/block/tests.rs index b05ef3c02..174b696e5 100644 --- a/all-is-cubes/src/block/tests.rs +++ b/all-is-cubes/src/block/tests.rs @@ -7,8 +7,6 @@ #![allow(clippy::bool_assert_comparison)] -use alloc::borrow::Cow; - use pretty_assertions::assert_eq; use crate::block::{ @@ -127,7 +125,7 @@ mod eval { fn opaque_atom_and_attributes() { let color = Rgba::new(1.0, 2.0, 3.0, 1.0); let attributes = BlockAttributes { - display_name: Cow::Borrowed("hello world"), + display_name: arcstr::literal!("hello world"), selectable: false, ..BlockAttributes::default() }; @@ -190,7 +188,7 @@ mod eval { let mut universe = Universe::new(); let attributes = BlockAttributes { - display_name: Cow::Borrowed("hello world"), + display_name: arcstr::literal!("hello world"), ..BlockAttributes::default() }; let block = Block::builder() diff --git a/all-is-cubes/src/content.rs b/all-is-cubes/src/content.rs index a361c165c..434c03c19 100644 --- a/all-is-cubes/src/content.rs +++ b/all-is-cubes/src/content.rs @@ -3,7 +3,6 @@ //! This module is private; the public interface to this stuff is the separate //! `all-is-cubes-content` crate. -use alloc::borrow::Cow; use alloc::string::ToString; use alloc::vec::Vec; @@ -15,9 +14,10 @@ use embedded_graphics::text::Baseline; use embedded_graphics::text::Text; use embedded_graphics::text::TextStyleBuilder; +use crate::arcstr::{literal, ArcStr}; use crate::block::{Block, Resolution, Resolution::R16, RotationPlacementRule}; use crate::inv::{Slot, Tool}; -use crate::math::{Face6, FreeCoordinate, GridAab, GridCoordinate, Rgb, Rgba}; +use crate::math::{Face6, FaceMap, FreeCoordinate, GridAab, GridCoordinate, Rgb, Rgba}; use crate::raycast::Raycaster; use crate::space::{SetCubeError, Space}; use crate::universe::Universe; @@ -180,6 +180,15 @@ pub fn make_slab( /// assert_ne!(space[[0, 0, -10]], AIR); /// ``` pub fn axes(space: &mut Space) -> Result<(), SetCubeError> { + const DIR_NAMES: FaceMap = FaceMap { + nx: literal!("x"), + ny: literal!("y"), + nz: literal!("z"), + px: literal!("X"), + py: literal!("Y"), + pz: literal!("Z"), + }; + for face in Face6::ALL { let axis = face.axis(); let direction = face.normal_vector::()[axis]; @@ -187,13 +196,13 @@ pub fn axes(space: &mut Space) -> Result<(), SetCubeError> { .within(space.bounds()); for step in raycaster { let i = step.cube_ahead().lower_bounds()[axis] * direction; // always positive - let (color, display_name): (Rgb, Cow<'static, str>) = if i.rem_euclid(2) == 0 { + let (color, display_name): (Rgb, ArcStr) = if i.rem_euclid(2) == 0 { (axis.color(), i.rem_euclid(10).to_string().into()) } else { if direction > 0 { - (rgb_const!(1.0, 1.0, 1.0), ["X", "Y", "Z"][axis].into()) + (rgb_const!(1.0, 1.0, 1.0), DIR_NAMES[face].clone()) } else { - (rgb_const!(0.0, 0.0, 0.0), ["x", "y", "z"][axis].into()) + (rgb_const!(0.0, 0.0, 0.0), DIR_NAMES[face].clone()) } }; space.set( diff --git a/all-is-cubes/src/lib.rs b/all-is-cubes/src/lib.rs index 35c917d08..95e0a1095 100644 --- a/all-is-cubes/src/lib.rs +++ b/all-is-cubes/src/lib.rs @@ -214,5 +214,7 @@ pub mod transaction; pub mod universe; pub mod util; +/// Re-export the version of the `arcstr` string type library we're using. +pub use arcstr; /// Re-export the version of the `euclid` vector math library we're using. pub use euclid; diff --git a/all-is-cubes/src/op.rs b/all-is-cubes/src/op.rs index 055d66ea0..e2fbb3dd4 100644 --- a/all-is-cubes/src/op.rs +++ b/all-is-cubes/src/op.rs @@ -16,6 +16,9 @@ use crate::universe::VisitRefs; /// TODO: The current representation as an enum is a temporary measure to replace a /// bunch of existing mechanics. In the future it will be something somewhat more like a /// general functional programming language. +/// +/// TODO: Ensure that cloning this type is O(1) and doesn't duplicate memory, because as a +/// part of `BlockAttribtes` and `EvaluatedBlock` it is cloned a lot. #[doc = include_str!("save/serde-warning.md")] #[derive(Clone, Debug, Eq, Hash, PartialEq)] #[non_exhaustive] diff --git a/all-is-cubes/src/raytracer/text.rs b/all-is-cubes/src/raytracer/text.rs index 65549b857..7dd518afd 100644 --- a/all-is-cubes/src/raytracer/text.rs +++ b/all-is-cubes/src/raytracer/text.rs @@ -2,10 +2,11 @@ #![cfg_attr(not(feature = "std"), allow(unused_imports))] -use alloc::borrow::{Cow, ToOwned}; -use alloc::string::{String, ToString}; +use alloc::string::String; +use arcstr::{literal_substr, ArcStr, Substr}; use euclid::vec2; +use unicode_segmentation::UnicodeSegmentation; use crate::camera::{eye_for_look_at, Camera, GraphicsOptions, Viewport}; use crate::math::{FreeVector, Rgba}; @@ -15,31 +16,27 @@ use crate::space::{Space, SpaceBlockData}; /// TODO: better name, docs #[derive(Clone, Debug, Eq, PartialEq)] #[allow(clippy::exhaustive_structs)] -pub struct CharacterRtData(pub Cow<'static, str>); +pub struct CharacterRtData(pub Substr); impl RtBlockData for CharacterRtData { type Options = (); fn from_block(_: RtOptionsRef<'_, Self::Options>, s: &SpaceBlockData) -> Self { - // TODO: For more Unicode correctness, index by grapheme cluster // TODO: allow customizing the fallback character - Self( - s.evaluated() - .attributes - .display_name - .chars() - .next() - .map(|c| Cow::Owned(c.to_string())) - .unwrap_or(Cow::Borrowed("#")), - ) + let name: &ArcStr = &s.evaluated().attributes.display_name; + let char = match name.graphemes(true).next() { + Some(s) => name.substr_from(s), + None => literal_substr!("#"), + }; + Self(char) } fn error(_: RtOptionsRef<'_, Self::Options>) -> Self { - Self(Cow::Borrowed("X")) + Self(literal_substr!("X")) } fn sky(_: RtOptionsRef<'_, Self::Options>) -> Self { - Self(Cow::Borrowed(" ")) + Self(literal_substr!(" ")) } } @@ -49,7 +46,7 @@ impl RtBlockData for CharacterRtData { #[allow(clippy::derive_partial_eq_without_eq)] pub struct CharacterBuf { /// Text to draw, if determined yet. - hit_text: Option, + hit_text: Option, } impl Accumulate for CharacterBuf { @@ -63,12 +60,12 @@ impl Accumulate for CharacterBuf { #[inline] fn add(&mut self, _surface_color: Rgba, d: &Self::BlockData) { if self.hit_text.is_none() { - self.hit_text = Some(String::from(d.0.clone())); + self.hit_text = Some(d.0.clone()); } } fn hit_nothing(&mut self) { - self.hit_text = Some(".".to_owned()); + self.hit_text = Some(literal_substr!(".")); } fn mean(items: [Self; N]) -> Self { @@ -79,10 +76,16 @@ impl Accumulate for CharacterBuf { } } +impl From for Substr { + #[inline] + fn from(buf: CharacterBuf) -> Substr { + buf.hit_text.unwrap_or_else(|| literal_substr!(".")) + } +} impl From for String { #[inline] fn from(buf: CharacterBuf) -> String { - buf.hit_text.unwrap_or_else(|| ".".to_owned()) + Substr::from(buf).to_string() } } diff --git a/all-is-cubes/src/save/conversion.rs b/all-is-cubes/src/save/conversion.rs index 23973e8e4..ededb8c2a 100644 --- a/all-is-cubes/src/save/conversion.rs +++ b/all-is-cubes/src/save/conversion.rs @@ -188,7 +188,7 @@ mod block { animation_hint, } = value; schema::BlockAttributesV1Ser { - display_name: Cow::Borrowed(&**display_name), + display_name: display_name.clone(), selectable, rotation_rule: rotation_rule.into(), tick_action: tick_action.as_ref().map( @@ -216,12 +216,7 @@ mod block { animation_hint, } = value; Self { - // convert Cow<'a> to Cow<'static> - display_name: if display_name == "" { - Cow::Borrowed("") - } else { - Cow::Owned(display_name.into()) - }, + display_name, selectable, rotation_rule: rotation_rule.into(), tick_action: tick_action.map(|schema::TickActionSer { operation, period }| { diff --git a/all-is-cubes/src/save/schema.rs b/all-is-cubes/src/save/schema.rs index 944a35f35..87ca1d0b2 100644 --- a/all-is-cubes/src/save/schema.rs +++ b/all-is-cubes/src/save/schema.rs @@ -16,10 +16,10 @@ //! * [`Cow`] is sometimes used to avoid unnecessary clones during serialization. use alloc::borrow::Cow; -use alloc::sync::Arc; use alloc::vec::Vec; use core::num::NonZeroU16; +use arcstr::ArcStr; use ordered_float::NotNan; use serde::{Deserialize, Serialize}; @@ -99,7 +99,7 @@ pub(crate) enum PrimitiveSer<'a> { #[derive(Debug, Deserialize, Serialize)] pub(crate) struct BlockAttributesV1Ser<'a> { #[serde(default, skip_serializing_if = "str::is_empty")] - pub display_name: Cow<'a, str>, + pub display_name: ArcStr, #[serde(default = "return_true", skip_serializing_if = "is_true")] pub selectable: bool, #[serde(default, skip_serializing_if = "is_default")] @@ -387,6 +387,6 @@ pub(crate) enum URefSer { #[derive(Debug, Eq, Ord, PartialEq, PartialOrd, Deserialize, Serialize)] pub(crate) enum NameSer { - Specific(Arc), + Specific(ArcStr), Anonym(usize), } diff --git a/all-is-cubes/src/space.rs b/all-is-cubes/src/space.rs index 73da977ed..f5b220b01 100644 --- a/all-is-cubes/src/space.rs +++ b/all-is-cubes/src/space.rs @@ -14,7 +14,7 @@ use manyfmt::Fmt; use crate::behavior::{self, BehaviorSet}; use crate::block::TickAction; -use crate::block::{Block, EvaluatedBlock, Resolution, AIR, AIR_EVALUATED}; +use crate::block::{Block, EvaluatedBlock, Resolution, AIR, AIR_EVALUATED_REF}; #[cfg(doc)] use crate::character::Character; use crate::character::Spawn; @@ -274,7 +274,7 @@ impl Space { if let Some(index) = self.bounds.index(position.into()) { self.palette.entry(self.contents[index]).evaluated() } else { - &AIR_EVALUATED + AIR_EVALUATED_REF } } diff --git a/all-is-cubes/src/universe.rs b/all-is-cubes/src/universe.rs index 28b7965fb..1e914ecf9 100644 --- a/all-is-cubes/src/universe.rs +++ b/all-is-cubes/src/universe.rs @@ -20,6 +20,7 @@ use alloc::vec::Vec; use core::fmt; use core::sync::atomic::{self, Ordering}; +use arcstr::ArcStr; use manyfmt::Fmt; use crate::block::BlockDef; @@ -59,10 +60,9 @@ mod tests; #[doc = include_str!("save/serde-warning.md")] #[allow(clippy::exhaustive_enums)] #[derive(Clone, Debug, Hash, Eq, Ord, PartialEq, PartialOrd)] -#[cfg_attr(feature = "arbitrary", derive(arbitrary::Arbitrary))] pub enum Name { /// An explicitly set name. - Specific(Arc), + Specific(ArcStr), /// An automatically assigned name. Anonym(usize), @@ -97,6 +97,12 @@ impl From for Name { } } +impl From for Name { + fn from(value: ArcStr) -> Self { + Self::Specific(value) + } +} + impl fmt::Display for Name { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { @@ -107,6 +113,33 @@ impl fmt::Display for Name { } } +// Manual impl because `ArcStr` doesn't impl Arbitrary. +#[cfg(feature = "arbitrary")] +impl<'a> arbitrary::Arbitrary<'a> for Name { + fn arbitrary(u: &mut arbitrary::Unstructured<'a>) -> arbitrary::Result { + Ok(match u.int_in_range::(0..=2)? { + 0 => Name::Specific(u.arbitrary::()?.into()), + 1 => Name::Anonym(u.arbitrary()?), + 2 => Name::Pending, + _ => unreachable!(), + }) + } + + fn size_hint(depth: usize) -> (usize, Option) { + use arbitrary::size_hint; + size_hint::recursion_guard(depth, |depth| { + size_hint::and( + u8::size_hint(depth), + size_hint::or_all(&[ + String::size_hint(depth), + usize::size_hint(depth), + <()>::size_hint(depth), + ]), + ) + }) + } +} + /// Copiable unique (within this process) identifier for a [`Universe`]. /// /// Used to check whether [`URef`]s belong to particular [`Universe`]s. diff --git a/all-is-cubes/tests/alloc.rs b/all-is-cubes/tests/alloc.rs index f7ca9f983..f7f9fc251 100644 --- a/all-is-cubes/tests/alloc.rs +++ b/all-is-cubes/tests/alloc.rs @@ -2,44 +2,30 @@ //! //! In a separate test crate to avoid modifying the global allocator elsewhere. -use std::borrow::Cow; - -use all_is_cubes::universe::Universe; use allocation_counter::{measure, AllocationInfo}; use all_is_cubes::block::{self, BlockAttributes}; +use all_is_cubes::universe::Universe; -/// TODO: Make the name refcounted so this count goes to zero. #[test] fn clone_block_attributes() { - let display_name: Cow<'static, str> = String::from("hello").into(); let original = BlockAttributes { - // This field will allocate when cloned - display_name: display_name.clone(), + // These fields are refcounted or `Copy` and will not allocate when cloned + display_name: arcstr::literal!("hello"), + selectable: true, + animation_hint: block::AnimationHint::UNCHANGING, - // TODO: This field could allocate when cloned but we're not worrying about that now + // TODO: This field could allocate when cloned and we should fix that and test it tick_action: None, // These fields currently will never allocate when cloned - selectable: true, rotation_rule: block::RotationPlacementRule::Never, - animation_hint: block::AnimationHint::UNCHANGING, }; let mut clone = None; - assert_eq!( - measure(|| { - clone = Some(original.clone()); - }), - AllocationInfo { - count_total: 1, - count_current: 1, - count_max: 1, - bytes_total: display_name.len() as u64, - bytes_current: display_name.len() as i64, - bytes_max: display_name.len() as u64 - } - ) + assert_no_alloc(|| { + clone = Some(original.clone()); + }); } /// Test that cloning an `EvaluatedBlock`, with voxels, allocates nothing (except @@ -50,21 +36,14 @@ fn clone_evaluated_block() { let [block] = all_is_cubes::content::make_some_voxel_blocks(universe); let original = block.evaluate().unwrap(); - // TODO: should be zero - let expected_bytes = original.attributes.display_name.len() as u64; let mut clone = None; - assert_eq!( - measure(|| { - clone = Some(original.clone()); - }), - AllocationInfo { - count_total: 1, - count_current: 1, - count_max: 1, - bytes_total: expected_bytes, - bytes_current: expected_bytes as i64, - bytes_max: expected_bytes - } - ) + assert_no_alloc(|| { + clone = Some(original.clone()); + }); +} + +#[track_caller] +fn assert_no_alloc(f: impl FnOnce()) { + assert_eq!(measure(f), AllocationInfo::default()); }