Skip to content

Commit fd50d6d

Browse files
committed
Refactor CacheEntry to avoid excessive RefCell overhead
Since the `RefCell::borrow{,_mut}()` methods both impose runtime overhead each time they are called, we should avoid calling them whenever possible. There are several things we can do to improve the situation: 1. Since `Widgets::draw_widget()` already takes `&mut self`, we can safely skip the runtime checks with `RefCell::get_mut()`. This imposes zero overhead. 2. Since `Widgets::draw_widget()` already takes `&mut self`, we can take a mutable reference to `CacheEntry::texture` directly. Since we don't have any immutable public methods exposing the `texture` fields, there's no need to wrap this in a `RefCell` at all. The elimination of these two `RefCell` runtime checks performed during the hot path in `Widgets::draw()` should hopefully improve performance slightly.
1 parent bd9d763 commit fd50d6d

File tree

1 file changed

+19
-17
lines changed

1 file changed

+19
-17
lines changed

src/widget.rs

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -144,21 +144,23 @@ impl<'tc, W: Widget> Widgets<'tc, W> {
144144
}
145145

146146
fn draw_widget(&mut self, id: WidgetId, canvas: &mut Canvas<Window>) -> anyhow::Result<()> {
147-
{
148-
let widget = self.cache[&id].widget.borrow();
149-
let (x, y) = widget.properties().origin;
150-
let (width, height) = widget.properties().bounds;
151-
152-
// Retrieve base widget texture, resizing if bounds have changed.
153-
let textures = &mut self.textures;
154-
let mut widget_texture = self.cache[&id].texture.borrow_mut();
155-
let target = widget_texture.create_or_resize(textures.creator, width, height)?;
156-
157-
// Draw the widget to the target texture and copy it to the canvas.
158-
widget.draw(&mut Context { canvas, textures }, target)?;
159-
let dst = Rect::new(x, y, width, height);
160-
canvas.copy(target, None, dst).map_err(Error::msg)?;
161-
}
147+
let (widget, texture) = self
148+
.cache
149+
.get_mut(&id)
150+
.map(|e| (e.widget.get_mut(), &mut e.texture))
151+
.unwrap();
152+
153+
let (x, y) = widget.properties().origin;
154+
let (width, height) = widget.properties().bounds;
155+
156+
// Retrieve base widget texture, resizing if bounds have changed.
157+
let textures = &mut self.textures;
158+
let target = texture.create_or_resize(textures.creator, width, height)?;
159+
160+
// Draw the widget to the target texture and copy it to the canvas.
161+
widget.draw(&mut Context { canvas, textures }, target)?;
162+
let dst = Rect::new(x, y, width, height);
163+
canvas.copy(target, None, dst).map_err(Error::msg)?;
162164

163165
for child_id in self.get_children_of(id).to_vec() {
164166
if child_id != id {
@@ -180,7 +182,7 @@ pub struct WidgetId(u32);
180182
#[derive(Debug)]
181183
struct CacheEntry<'tc, W> {
182184
widget: RefCell<W>,
183-
texture: RefCell<WidgetTexture<'tc>>,
185+
texture: WidgetTexture<'tc>,
184186
parent: WidgetId,
185187
children: Vec<WidgetId>,
186188
}
@@ -189,7 +191,7 @@ impl<'tc, W> CacheEntry<'tc, W> {
189191
fn new(widget: W, parent: WidgetId) -> Self {
190192
CacheEntry {
191193
widget: RefCell::new(widget),
192-
texture: RefCell::new(WidgetTexture::default()),
194+
texture: WidgetTexture::default(),
193195
parent,
194196
children: Vec::new(),
195197
}

0 commit comments

Comments
 (0)