Improve Layer Separations (#798)

This improves the code that splits the rendered entities into the bottom
and top layers. The top layer is intended to be used for entities that
update frequently. This was mostly done pretty well before, but with the
web renderer it has been quite easy to find cases that could be
improved:

1. The graph used to always be rendered onto the top layer. This does
   not need to be the case when the timer is paused or just not running
   at all.
2. Game Time can change all the time, even if the timer is paused. So
   this is taken into account everywhere now.
3. Columns didn't consider pausing at all either.
4. There was a bug in the web renderer where a brightness of 0 was
   considered as the default for brightness and thus no brightness
   change was applied. This should've been at 1.
5. The new Rust release brought some new clippy lints around string
   buffer reusage that were addressed.

src/analysis/current_pace.rs

@@ -10,8 +10,9 @@ use crate::{analysis, timing::Snapshot, TimeSpan, TimerPhase};
 pub fn calculate(timer: &Snapshot<'_>, comparison: &str) -> (Option<TimeSpan>, bool) {
     let timing_method = timer.current_timing_method();
     let last_segment = timer.run().segments().last().unwrap();
+    let phase = timer.current_phase();
 
-    match timer.current_phase() {
+    match phase {
         TimerPhase::Running | TimerPhase::Paused => {
             let mut delta = analysis::last_delta(
                 timer.run(),
@@ -39,7 +40,7 @@ pub fn calculate(timer: &Snapshot<'_>, comparison: &str) -> (Option<TimeSpan>, b
 
             (
                 value,
-                is_live && timer.current_phase().is_running() && value.is_some(),
+                is_live && phase.updates_frequently(timing_method) && value.is_some(),
             )
         }
         TimerPhase::Ended => (last_segment.split_time()[timing_method], false),

src/analysis/pb_chance/mod.rs

@@ -90,5 +90,8 @@ pub fn for_timer(timer: &Snapshot<'_>) -> (f64, bool) {
         calculate(segments, method, current_time)
     };
 
-    (chance, is_live && timer.current_phase().is_running())
+    (
+        chance,
+        is_live && timer.current_phase().updates_frequently(method),
+    )
 }

src/analysis/possible_time_save.rs

@@ -52,7 +52,7 @@ pub fn calculate(
                 let segment_delta = TimeSpan::zero() - segment_delta;
                 if segment_delta < time {
                     time = segment_delta;
-                    updates_frequently = timer.current_phase().is_running();
+                    updates_frequently = timer.current_phase().updates_frequently(method);
                 }
             }
         }

src/component/graph.rs

@@ -118,6 +118,9 @@ pub struct State {
     pub best_segment_color: Color,
     /// The height of the chart.
     pub height: u32,
+    /// This value indicates whether the graph is currently frequently being
+    /// updated. This can be used for rendering optimizations.
+    pub updates_frequently: bool,
 }
 
 /// Describes a point on the graph to visualize.
@@ -257,6 +260,9 @@ impl Component {
         state.middle = x_axis;
         state.is_live_delta_active = draw_info.is_live_delta_active;
         state.points = draw_info.points;
+        state.updates_frequently = timer
+            .current_phase()
+            .updates_frequently(timer.current_timing_method());
     }
 
     /// Calculates the component's state based on the timer and layout settings

src/component/previous_segment.rs

@@ -239,7 +239,7 @@ impl Component {
         }
 
         state.display_two_rows = self.settings.display_two_rows;
-        state.updates_frequently = live_segment.is_some() && phase.is_running();
+        state.updates_frequently = live_segment.is_some() && phase.updates_frequently(method);
     }
 
     /// Calculates the component's state based on the timer and the layout

src/component/splits/column.rs

@@ -280,9 +280,14 @@ fn update_time_column(
             false,
         )
     });
+
     let is_empty = column_settings.start_with == ColumnStartWith::Empty && !updated;
-    state.updates_frequently = is_live && column_value.is_some();
+
+    state.updates_frequently =
+        is_live && column_value.is_some() && timer.current_phase().updates_frequently(method);
+
     state.value.clear();
+
     if !is_empty {
         let _ = match formatter {
             ColumnFormatter::Time => write!(

src/component/timer.rs

@@ -296,7 +296,7 @@ impl Component {
             formatter::Fraction::with_accuracy(self.settings.accuracy).format(time),
         );
 
-        state.updates_frequently = phase.is_running() && time.is_some();
+        state.updates_frequently = phase.updates_frequently(method) && time.is_some();
         state.semantic_color = semantic_color;
         state.height = self.settings.height;
     }

src/component/total_playtime.rs

@@ -8,7 +8,7 @@ use crate::{
     platform::prelude::*,
     settings::{Color, Field, Gradient, SettingsDescription, Value},
     timing::formatter::{Days, Regular, TimeFormatter},
-    Timer,
+    Timer, TimingMethod,
 };
 use core::fmt::Write;
 use serde_derive::{Deserialize, Serialize};
@@ -101,7 +101,9 @@ impl Component {
         state.key_abbreviations.push("Playtime".into());
 
         state.display_two_rows = self.settings.display_two_rows;
-        state.updates_frequently = timer.current_phase().is_running();
+        state.updates_frequently = timer
+            .current_phase()
+            .updates_frequently(TimingMethod::RealTime);
     }
 
     /// Calculates the component's state based on the timer provided.

src/layout/parser/splits.rs

@@ -115,7 +115,7 @@ pub fn settings(reader: &mut Reader<'_>, component: &mut Component) -> Result<()
                         comparison_override(reader, |v| {
                             for column in &mut settings.columns {
                                 if let ColumnKind::Time(column) = &mut column.kind {
-                                    column.comparison_override = v.clone();
+                                    column.comparison_override.clone_from(&v);
                                 }
                             }
                         })

src/rendering/component/graph.rs

@@ -1,7 +1,7 @@
 use crate::{
     component::graph::State,
     layout::LayoutState,
-    rendering::{PathBuilder, RenderContext, ResourceAllocator},
+    rendering::{Layer, PathBuilder, RenderContext, ResourceAllocator},
     settings::Gradient,
 };
 
@@ -19,30 +19,36 @@ pub(in crate::rendering) fn render(
     const LINE_WIDTH: f32 = 0.025;
     const CIRCLE_RADIUS: f32 = 0.035;
 
-    context.render_top_rectangle(
+    let layer = Layer::from_updates_frequently(component.updates_frequently);
+
+    context.render_layer_rectangle(
         [0.0, 0.0],
         [width, component.middle],
         &Gradient::Plain(component.top_background_color),
+        layer,
     );
-    context.render_top_rectangle(
+    context.render_layer_rectangle(
         [0.0, component.middle],
         [width, 1.0],
         &Gradient::Plain(component.bottom_background_color),
+        layer,
     );
 
     for &y in &component.horizontal_grid_lines {
-        context.render_top_rectangle(
+        context.render_layer_rectangle(
             [0.0, y - GRID_LINE_WIDTH],
             [width, y + GRID_LINE_WIDTH],
             &Gradient::Plain(component.grid_lines_color),
+            layer,
         );
     }
 
     for &x in &component.vertical_grid_lines {
-        context.render_top_rectangle(
+        context.render_layer_rectangle(
             [width * x - GRID_LINE_WIDTH, 0.0],
             [width * x + GRID_LINE_WIDTH, 1.0],
             &Gradient::Plain(component.grid_lines_color),
+            layer,
         );
     }
 
@@ -57,7 +63,7 @@ pub(in crate::rendering) fn render(
         builder.line_to(width * p2.x, component.middle);
         builder.close();
         let partial_fill_path = builder.finish();
-        context.top_layer_path(partial_fill_path, component.partial_fill_color);
+        context.fill_path(partial_fill_path, component.partial_fill_color, layer);
 
         component.points.len() - 1
     } else {
@@ -72,7 +78,7 @@ pub(in crate::rendering) fn render(
     builder.line_to(width * component.points[len - 1].x, component.middle);
     builder.close();
     let fill_path = builder.finish();
-    context.top_layer_path(fill_path, component.complete_fill_color);
+    context.fill_path(fill_path, component.complete_fill_color, layer);
 
     for points in component.points.windows(2) {
         let mut builder = context.handles.path_builder();
@@ -86,7 +92,7 @@ pub(in crate::rendering) fn render(
         };
 
         let line_path = builder.finish();
-        context.top_layer_stroke_path(line_path, color, LINE_WIDTH);
+        context.stroke_path(line_path, color, LINE_WIDTH, layer);
     }
 
     for (i, point) in component.points.iter().enumerate().skip(1) {
@@ -100,7 +106,7 @@ pub(in crate::rendering) fn render(
             let circle_path = context
                 .handles
                 .build_circle(width * point.x, point.y, CIRCLE_RADIUS);
-            context.top_layer_path(circle_path, color);
+            context.fill_path(circle_path, color, layer);
         }
     }
 

src/rendering/mod.rs

@@ -435,7 +435,13 @@ impl<A: ResourceAllocator> RenderContext<'_, A> {
             .push(Entity::FillPath(rectangle, shader, transform));
     }
 
-    fn backend_render_top_rectangle(&mut self, [x1, y1]: Pos, [x2, y2]: Pos, shader: FillShader) {
+    fn backend_render_layer_rectangle(
+        &mut self,
+        [x1, y1]: Pos,
+        [x2, y2]: Pos,
+        shader: FillShader,
+        layer: Layer,
+    ) {
         let transform = self
             .transform
             .pre_translate(x1, y1)
@@ -444,18 +450,24 @@ impl<A: ResourceAllocator> RenderContext<'_, A> {
         let rectangle = self.rectangle();
 
         self.scene
-            .top_layer_mut()
+            .layer_mut(layer)
             .push(Entity::FillPath(rectangle, shader, transform));
     }
 
-    fn top_layer_path(&mut self, path: Handle<A::Path>, color: Color) {
+    fn fill_path(&mut self, path: Handle<A::Path>, color: Color, layer: Layer) {
         self.scene
-            .top_layer_mut()
+            .layer_mut(layer)
             .push(Entity::FillPath(path, solid(&color), self.transform));
     }
 
-    fn top_layer_stroke_path(&mut self, path: Handle<A::Path>, color: Color, stroke_width: f32) {
-        self.scene.top_layer_mut().push(Entity::StrokePath(
+    fn stroke_path(
+        &mut self,
+        path: Handle<A::Path>,
+        color: Color,
+        stroke_width: f32,
+        layer: Layer,
+    ) {
+        self.scene.layer_mut(layer).push(Entity::StrokePath(
             path,
             stroke_width,
             color.to_array(),
@@ -498,9 +510,15 @@ impl<A: ResourceAllocator> RenderContext<'_, A> {
         }
     }
 
-    fn render_top_rectangle(&mut self, top_left: Pos, bottom_right: Pos, gradient: &Gradient) {
+    fn render_layer_rectangle(
+        &mut self,
+        top_left: Pos,
+        bottom_right: Pos,
+        gradient: &Gradient,
+        layer: Layer,
+    ) {
         if let Some(colors) = decode_gradient(gradient) {
-            self.backend_render_top_rectangle(top_left, bottom_right, colors);
+            self.backend_render_layer_rectangle(top_left, bottom_right, colors, layer);
         }
     }
 

src/rendering/web/mod.rs

@@ -604,7 +604,7 @@ impl Renderer {
                         if let Some((image, _)) = &*image {
                             str_buf.clear();
                             use std::fmt::Write;
-                            if background_image.brightness != 0.0 {
+                            if background_image.brightness != 1.0 {
                                 let _ = write!(
                                     str_buf,
                                     "brightness({}%)",

src/timing/timer/mod.rs

@@ -569,7 +569,11 @@ impl Timer {
             .position(|c| c == self.current_comparison)
             .unwrap();
         let index = (index + 1) % len;
-        self.current_comparison = self.run.comparisons().nth(index).unwrap().to_owned();
+        self.run
+            .comparisons()
+            .nth(index)
+            .unwrap()
+            .populate(&mut self.current_comparison);
 
         // FIXME: OnNextComparison
     }
@@ -582,7 +586,11 @@ impl Timer {
             .position(|c| c == self.current_comparison)
             .unwrap();
         let index = (index + len - 1) % len;
-        self.current_comparison = self.run.comparisons().nth(index).unwrap().to_owned();
+        self.run
+            .comparisons()
+            .nth(index)
+            .unwrap()
+            .populate(&mut self.current_comparison);
 
         // FIXME: OnPreviousComparison
     }

src/timing/timer_phase.rs

@@ -1,3 +1,5 @@
+use crate::TimingMethod;
+
 /// Describes which phase the timer is currently in. This tells you if there's
 /// an active speedrun attempt and whether it is paused or it ended.
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
@@ -14,23 +16,35 @@ pub enum TimerPhase {
 }
 
 impl TimerPhase {
-    /// Returns `true` if the value is [`TimerPhase::NotRunning`].
+    /// Returns [`true`] if the value is [`TimerPhase::NotRunning`].
     pub const fn is_not_running(&self) -> bool {
         matches!(self, Self::NotRunning)
     }
 
-    /// Returns `true` if the value is [`TimerPhase::Running`].
+    /// Returns [`true`] if the value is [`TimerPhase::Running`].
     pub const fn is_running(&self) -> bool {
         matches!(self, Self::Running)
     }
 
-    /// Returns `true` if the value is [`TimerPhase::Ended`].
+    /// Returns [`true`] if the value is [`TimerPhase::Ended`].
     pub const fn is_ended(&self) -> bool {
         matches!(self, Self::Ended)
     }
 
-    /// Returns `true` if the value is [`TimerPhase::Paused`].
+    /// Returns [`true`] if the value is [`TimerPhase::Paused`].
     pub const fn is_paused(&self) -> bool {
         matches!(self, Self::Paused)
     }
+
+    /// Returns [`true`] if the timer is currently in a phase where it updates
+    /// frequently. This means that the timer is [`TimerPhase::Running`] or
+    /// [`TimerPhase::Paused`] and the timing method is not
+    /// [`TimingMethod::RealTime`].
+    pub(crate) const fn updates_frequently(&self, method: TimingMethod) -> bool {
+        match self {
+            Self::Running => true,
+            Self::Paused => matches!(method, TimingMethod::GameTime),
+            _ => false,
+        }
+    }
 }

src/util/populate_string.rs

@@ -27,6 +27,10 @@ impl PopulateString for String {
 }
 
 impl PopulateString for &str {
+    // If the string doesn't fit into the capacity of the buffer, we just
+    // allocate a new buffer instead of forcing it to reallocate, which would
+    // mean copying all the bytes of the previous buffer, which we don't care about.
+    #[allow(clippy::assigning_clones)]
     fn populate(self, buf: &mut String) {
         if self.len() <= buf.capacity() {
             buf.clear();