Skip to content

Commit

Permalink
Improve Layer Separations (#798)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
CryZe authored May 2, 2024
1 parent a7c9df2 commit 47bf322
Show file tree
Hide file tree
Showing 15 changed files with 101 additions and 34 deletions.
5 changes: 3 additions & 2 deletions src/analysis/current_pace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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),
Expand Down
5 changes: 4 additions & 1 deletion src/analysis/pb_chance/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
)
}
2 changes: 1 addition & 1 deletion src/analysis/possible_time_save.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Expand Down
6 changes: 6 additions & 0 deletions src/component/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/component/previous_segment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion src/component/splits/column.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
Expand Down
2 changes: 1 addition & 1 deletion src/component/timer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
6 changes: 4 additions & 2 deletions src/component/total_playtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion src/layout/parser/splits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
})
Expand Down
24 changes: 15 additions & 9 deletions src/rendering/component/graph.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::{
component::graph::State,
layout::LayoutState,
rendering::{PathBuilder, RenderContext, ResourceAllocator},
rendering::{Layer, PathBuilder, RenderContext, ResourceAllocator},
settings::Gradient,
};

Expand All @@ -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,
);
}

Expand All @@ -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 {
Expand All @@ -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();
Expand All @@ -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) {
Expand All @@ -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);
}
}

Expand Down
34 changes: 26 additions & 8 deletions src/rendering/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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(),
Expand Down Expand Up @@ -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);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/rendering/web/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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({}%)",
Expand Down
12 changes: 10 additions & 2 deletions src/timing/timer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down
22 changes: 18 additions & 4 deletions src/timing/timer_phase.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand All @@ -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,
}
}
}
4 changes: 4 additions & 0 deletions src/util/populate_string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 47bf322

Please sign in to comment.