Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions crates/egui/src/data/output.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! All the data egui returns to the backend at the end of each frame.

use std::sync::Arc;

use crate::{OrderedViewportIdMap, RepaintCause, ViewportOutput, WidgetType};

/// What egui emits each frame from [`crate::Context::run`].
Expand Down Expand Up @@ -495,10 +497,10 @@ pub struct WidgetInfo {
pub label: Option<String>,

/// The contents of some editable text (for [`TextEdit`](crate::TextEdit) fields).
pub current_text_value: Option<String>,
pub current_text_value: Option<Arc<str>>,

/// The previous text value.
pub prev_text_value: Option<String>,
pub prev_text_value: Option<Arc<str>>,

/// The current value of checkboxes and radio buttons.
pub selected: Option<bool>,
Expand Down Expand Up @@ -618,12 +620,12 @@ impl WidgetInfo {
#[expect(clippy::needless_pass_by_value)]
pub fn text_edit(
enabled: bool,
prev_text_value: impl ToString,
text_value: impl ToString,
prev_text_value: impl Into<Arc<str>>,
text_value: impl Into<Arc<str>>,
hint_text: impl ToString,
) -> Self {
let text_value = text_value.to_string();
let prev_text_value = prev_text_value.to_string();
let text_value = text_value.into();
let prev_text_value = prev_text_value.into();
let hint_text = hint_text.to_string();
let prev_text_value = if text_value == prev_text_value {
None
Expand All @@ -639,16 +641,15 @@ impl WidgetInfo {
}
}

#[expect(clippy::needless_pass_by_value)]
pub fn text_selection_changed(
enabled: bool,
text_selection: std::ops::RangeInclusive<usize>,
current_text_value: impl ToString,
current_text_value: impl Into<Arc<str>>,
) -> Self {
Self {
enabled,
text_selection: Some(text_selection),
current_text_value: Some(current_text_value.to_string()),
current_text_value: Some(current_text_value.into()),
..Self::new(WidgetType::TextEdit)
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/egui/src/painter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,7 @@ impl Painter {
/// Paint the results with [`Self::galley`].
#[inline]
#[must_use]
pub fn layout_job(&self, layout_job: LayoutJob) -> Arc<Galley> {
pub fn layout_job(&self, layout_job: impl Into<Arc<LayoutJob>>) -> Arc<Galley> {
self.fonts_mut(|f| f.layout_job(layout_job))
}

Expand Down
2 changes: 1 addition & 1 deletion crates/egui/src/response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -874,7 +874,7 @@ impl Response {
}
}
if let Some(value) = info.current_text_value {
builder.set_value(value);
builder.set_value((*value).to_owned());
}
if let Some(value) = info.value {
builder.set_numeric_value(value);
Expand Down
41 changes: 21 additions & 20 deletions crates/egui/src/widgets/text_edit/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,7 @@ impl TextEdit<'_> {
// .unwrap_or_else(|| ui.style().interact(&response).text_color()); // too bright
.unwrap_or_else(|| ui.visuals().widgets.inactive.text_color());

let prev_text = text.as_str().to_owned();
let prev_text = text.clone_to_arc();
let hint_text_str = hint_text.text().to_owned();

let font_id = font_selection.resolve(ui.style());
Expand All @@ -516,7 +516,8 @@ impl TextEdit<'_> {

let font_id_clone = font_id.clone();
let mut default_layouter = move |ui: &Ui, text: &dyn TextBuffer, wrap_width: f32| {
let text = mask_if_password(password, text.as_str());
// TODO(afishhh): Keep as `Arc<str>` instead of cloning to a `String`
let text = (*mask_if_password(password, text.clone_to_arc())).to_owned();
Comment on lines +519 to +520
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest we do this in this PR, or it feels a bit half-finished

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do agree but this goes straight into a LayoutJob and I'm not sure how exactly to handle Arcifying that.

Should I just split out a builder type and make LayoutJob itself immutable?

let layout_job = if multiline {
LayoutJob::simple(text, font_id_clone.clone(), text_color, wrap_width)
} else {
Expand Down Expand Up @@ -819,8 +820,8 @@ impl TextEdit<'_> {
response.widget_info(|| {
WidgetInfo::text_edit(
ui.is_enabled(),
mask_if_password(password, prev_text.as_str()),
mask_if_password(password, text.as_str()),
mask_if_password(password, prev_text.clone()),
mask_if_password(password, text.clone_to_arc()),
hint_text_str.as_str(),
)
});
Expand All @@ -830,15 +831,15 @@ impl TextEdit<'_> {
let info = WidgetInfo::text_selection_changed(
ui.is_enabled(),
char_range,
mask_if_password(password, text.as_str()),
mask_if_password(password, text.clone_to_arc()),
);
response.output_event(OutputEvent::TextSelectionChanged(info));
} else {
response.widget_info(|| {
WidgetInfo::text_edit(
ui.is_enabled(),
mask_if_password(password, prev_text.as_str()),
mask_if_password(password, text.as_str()),
mask_if_password(password, prev_text.clone()),
mask_if_password(password, text.clone_to_arc()),
hint_text_str.as_str(),
)
});
Expand Down Expand Up @@ -875,7 +876,7 @@ impl TextEdit<'_> {
}
}

fn mask_if_password(is_password: bool, text: &str) -> String {
fn mask_if_password(is_password: bool, text: Arc<str>) -> Arc<str> {
fn mask_password(text: &str) -> String {
std::iter::repeat_n(
epaint::text::PASSWORD_REPLACEMENT_CHAR,
Expand All @@ -885,9 +886,9 @@ fn mask_if_password(is_password: bool, text: &str) -> String {
}

if is_password {
mask_password(text)
mask_password(&text).into()
} else {
text.to_owned()
text
}
}

Expand Down Expand Up @@ -916,10 +917,10 @@ fn events(

// We feed state to the undoer both before and after handling input
// so that the undoer creates automatic saves even when there are no events for a while.
state.undoer.lock().feed_state(
ui.input(|i| i.time),
&(cursor_range, text.as_str().to_owned()),
);
state
.undoer
.lock()
.feed_state(ui.input(|i| i.time), &(cursor_range, text.clone_to_arc()));

let copy_if_not_password = |ui: &Ui, text: String| {
if !password {
Expand Down Expand Up @@ -1030,7 +1031,7 @@ fn events(
if let Some((redo_ccursor_range, redo_txt)) = state
.undoer
.lock()
.redo(&(cursor_range, text.as_str().to_owned()))
.redo(&(cursor_range, text.clone_to_arc()))
{
text.replace_with(redo_txt);
Some(*redo_ccursor_range)
Expand All @@ -1048,7 +1049,7 @@ fn events(
if let Some((undo_ccursor_range, undo_txt)) = state
.undoer
.lock()
.undo(&(cursor_range, text.as_str().to_owned()))
.undo(&(cursor_range, text.clone_to_arc()))
{
text.replace_with(undo_txt);
Some(*undo_ccursor_range)
Expand Down Expand Up @@ -1126,10 +1127,10 @@ fn events(

state.cursor.set_char_range(Some(cursor_range));

state.undoer.lock().feed_state(
ui.input(|i| i.time),
&(cursor_range, text.as_str().to_owned()),
);
state
.undoer
.lock()
.feed_state(ui.input(|i| i.time), &(cursor_range, text.clone_to_arc()));

(any_change, cursor_range)
}
Expand Down
2 changes: 1 addition & 1 deletion crates/egui/src/widgets/text_edit/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::{
text_selection::{CCursorRange, TextCursorState},
};

pub type TextEditUndoer = crate::util::undoer::Undoer<(CCursorRange, String)>;
pub type TextEditUndoer = crate::util::undoer::Undoer<(CCursorRange, Arc<str>)>;

/// The text edit state stored between frames.
///
Expand Down
38 changes: 37 additions & 1 deletion crates/egui/src/widgets/text_edit/text_buffer.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{borrow::Cow, ops::Range};
use std::{borrow::Cow, ops::Range, sync::Arc};

use epaint::{
Galley,
Expand All @@ -24,6 +24,17 @@ pub trait TextBuffer {
/// Returns this buffer as a `str`.
fn as_str(&self) -> &str;

/// Clone this buffer to an [`Arc<str>`].
///
/// By default this is implemented by creating a new [`Arc`] from the result
/// of [`as_str`]. However, if the buffer is already managed by an [`Arc`],
/// this can be implemented more efficiently and avoid some cloning.
///
/// [`as_str`]: Self::as_str
fn clone_to_arc(&self) -> Arc<str> {
Arc::from(self.as_str())
}

/// Inserts text `text` into this buffer at character index `char_index`.
///
/// # Notes
Expand Down Expand Up @@ -315,3 +326,28 @@ impl TextBuffer for &str {
std::any::TypeId::of::<&str>()
}
}

/// Immutable view of an [`Arc<str>`]
impl TextBuffer for Arc<str> {
fn is_mutable(&self) -> bool {
false
}

fn as_str(&self) -> &str {
self
}

fn clone_to_arc(&self) -> Arc<str> {
self.clone()
}

fn insert_text(&mut self, _text: &str, _ch_idx: usize) -> usize {
0
}

fn delete_char_range(&mut self, _ch_range: Range<usize>) {}

fn type_id(&self) -> std::any::TypeId {
std::any::TypeId::of::<Self>()
}
}
13 changes: 6 additions & 7 deletions crates/epaint/src/text/fonts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,7 @@ impl FontsView<'_> {
///
/// The implementation uses memoization so repeated calls are cheap.
#[inline]
pub fn layout_job(&mut self, job: LayoutJob) -> Arc<Galley> {
pub fn layout_job(&mut self, job: impl Into<Arc<LayoutJob>>) -> Arc<Galley> {
let allow_split_paragraphs = true; // Optimization for editing text with many paragraphs.
self.galley_cache.layout(
self.fonts,
Expand Down Expand Up @@ -852,7 +852,7 @@ impl GalleyCache {
fn layout_internal(
&mut self,
fonts: &mut FontsImpl,
mut job: LayoutJob,
mut job: Arc<LayoutJob>,
pixels_per_point: f32,
allow_split_paragraphs: bool,
) -> (u64, Arc<Galley>) {
Expand All @@ -878,7 +878,7 @@ impl GalleyCache {
// * https://github.com/emilk/egui/issues/5084
// * https://github.com/emilk/egui/issues/5163

job.wrap.max_width = job.wrap.max_width.round();
Arc::make_mut(&mut job).wrap.max_width = job.wrap.max_width.round();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is slightly unfortunate and could be avoided by threading a separate max_width throughout layout but that sounds like a lot of work while when working with a lot of text you probably don't necessarily need line wrapping anyway.

}

let hash = crate::util::hash((&job, OrderedFloat(pixels_per_point))); // TODO(emilk): even faster hasher?
Expand Down Expand Up @@ -906,7 +906,6 @@ impl GalleyCache {
galley
}
std::collections::hash_map::Entry::Vacant(entry) => {
let job = Arc::new(job);
if allow_split_paragraphs && should_cache_each_paragraph_individually(&job) {
let (child_galleys, child_hashes) =
self.layout_each_paragraph_individually(fonts, &job, pixels_per_point);
Expand Down Expand Up @@ -946,10 +945,10 @@ impl GalleyCache {
&mut self,
fonts: &mut FontsImpl,
pixels_per_point: f32,
job: LayoutJob,
job: impl Into<Arc<LayoutJob>>,
allow_split_paragraphs: bool,
) -> Arc<Galley> {
self.layout_internal(fonts, job, pixels_per_point, allow_split_paragraphs)
self.layout_internal(fonts, job.into(), pixels_per_point, allow_split_paragraphs)
.1
}

Expand Down Expand Up @@ -1039,7 +1038,7 @@ impl GalleyCache {

// TODO(emilk): we could lay out each paragraph in parallel to get a nice speedup on multicore machines.
let (hash, galley) =
self.layout_internal(fonts, paragraph_job, pixels_per_point, false);
self.layout_internal(fonts, Arc::new(paragraph_job), pixels_per_point, false);
child_hashes.push(hash);

// This will prevent us from invalidating cache entries unnecessarily:
Expand Down
Loading