Skip to content

feat(canvas): add scale factor to canvas options #122

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Wybxc wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat(canvas): add scale factor to canvas options #122

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a14c828
feat(canvas): add scale factor to canvas options
Wybxc Aug 19, 2025
101a106
chore: improve error handling in scale factor settings
Wybxc Sep 8, 2025
dea1acc
chore: improve the document of scale factor
Wybxc Sep 8, 2025
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
55 changes: 52 additions & 3 deletions src/backend/canvas.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,41 @@
const CELL_HEIGHT: f64 = 19.0;

/// Options for the [`CanvasBackend`].
#[derive(Debug, Default)]
#[derive(Debug)]
pub struct CanvasBackendOptions {
/// The element ID.
grid_id: Option<String>,
/// Override the automatically detected size.
size: Option<(u32, u32)>,
/// Scale factor for the canvas. Setting this to a value greater than 1.0
/// will improve the rendering quality on high-DPI displays, at the cost
/// of performance.
///
/// The memory usage of the canvas will increase with the square of the scale factor.
/// For example, a scale factor of 2.0 will use 4 times the memory.
///
/// The default value is 1.0.
///
/// Generally, a scale factor of 2.0 is a good compromise between quality and performance.
scale: f64,
/// Always clip foreground drawing to the cell rectangle. Helpful when
/// dealing with out-of-bounds rendering from problematic fonts. Enabling
/// this option may cause some performance issues when dealing with large
/// numbers of simultaneous changes.
always_clip_cells: bool,
}

impl Default for CanvasBackendOptions {
fn default() -> Self {
Self {
grid_id: None,
size: None,
scale: 1.0,
always_clip_cells: false,
}
}
}

impl CanvasBackendOptions {
/// Constructs a new [`CanvasBackendOptions`].
pub fn new() -> Self {
Expand All @@ -65,6 +87,28 @@
self.size = Some(size);
self
}

/// Sets the scale factor for the canvas. Setting this to a value greater than 1.0
/// will improve the rendering quality on high-DPI displays, at the cost
/// of performance.
///
/// The memory usage of the canvas will increase with the square of the scale factor.
/// For example, a scale factor of 2.0 will use 4 times the memory.
///
/// The default value is 1.0.
///
/// Generally, a scale factor of 2.0 is a good compromise between quality and performance.
///
/// # Panics
///
/// Panics if `scale` is not positive.
pub fn scale(mut self, scale: f64) -> Self {
if scale <= 0.0 {
Copy link
Member

@junkdog junkdog Sep 8, 2025

Choose a reason for hiding this comment

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

should it be scale < 0.0? i guess 0 is not a valid scale.

i think it would be good to set a restriction the upper bounds too, maybe 4.0 - it's already at x16 memory consumption. i haven't tried it locally, but i guess setting really high values would cause the canvas to fail somehow.

otherwise i think it LGTM 👍

Copy link
Contributor Author

@Wybxc Wybxc Sep 9, 2025

Choose a reason for hiding this comment

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

should it be scale < 0.0? i guess 0 is not a valid scale.

When the scale is set to 0, the graphic will not be displayed, so it is reasonable to throw an error when scale=0.

i think it would be good to set a restriction the upper bounds too, maybe 4.0 - it's already at x16 memory consumption.

Excessively high scaling requires substantial hardware resources, but it is not impossible to run. In such cases, I believe a hard error should not be issued - after all, with sufficient computational resources, it remains feasible to execute. A warning, or even just a note in the documentation to caution users, would suffice.

Copy link
Member

@junkdog junkdog Sep 9, 2025

Choose a reason for hiding this comment

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

so it is reasonable to throw an error when scale=0.

i can't read if statements 😅 yeah, i agree - scale=0 should throw.

Excessively high scaling

do values above 3-4 have any visual impact?

what happens to the canvas/tab/browser/system if it allocates more memory than the user has available? (i can take a look after work too)

Copy link
Contributor Author

@Wybxc Wybxc Sep 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

do values above 3-4 have any visual impact?

In fact, 3x resolution has reached the limit of what is discernible. Perhaps I should make this point clearer in the documentation.

what happens to the canvas/tab/browser/system if it allocates more memory than the user has available? (i can take a look after work too)

First, the running speed will slow down. If increased further, the entire webpage (Firefox) or the <canvas> element (Safari/Chrome) will stop functioning. However, in any case, the browser's security measures are well-implemented and will not allow a webpage to consume all system resources.

Copy link
Member

@junkdog junkdog Sep 9, 2025

Choose a reason for hiding this comment

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

if there's no discernible difference except resource consumption past 3x, i think we can restrict the range to <= 3 (or 4). or is there ever a reason for higher scale values?

Copy link
Member

@orhun orhun Sep 11, 2025

Choose a reason for hiding this comment

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

would be also nice to document the side effects of slightly high values

panic!("Scale must be greater than 0");
}
self.scale = scale;
self
}
}

/// Canvas renderer.
Expand All @@ -84,9 +128,13 @@
parent_element: web_sys::Element,
width: u32,
height: u32,
scale: f64,
background_color: Color,
) -> Result<Self, Error> {
let canvas = create_canvas_in_element(&parent_element, width, height)?;
let inner_width = (width as f64 * scale) as u32;
let inner_height = (height as f64 * scale) as u32;
let canvas = create_canvas_in_element(&parent_element, inner_width, inner_height)?;
canvas.set_attribute("style", &format!("width: {width}px; height: {height}px;"))?;

let context_options = Map::new();
context_options.set(&JsValue::from_str("alpha"), &Boolean::from(JsValue::TRUE));
Expand All @@ -101,6 +149,7 @@
.expect("Unable to cast canvas context");
context.set_font("16px monospace");
context.set_text_baseline("top");
context.scale(scale, scale)?;

Ok(Self {
inner: canvas,
Expand Down Expand Up @@ -162,7 +211,7 @@
.size
.unwrap_or_else(|| (parent.client_width() as u32, parent.client_height() as u32));

let canvas = Canvas::new(parent, width, height, Color::Black)?;
let canvas = Canvas::new(parent, width, height, options.scale, Color::Black)?;
let buffer = get_sized_buffer_from_canvas(&canvas.inner);
let changed_cells = bitvec![0; buffer.len() * buffer[0].len()];
Ok(Self {
Expand Down Expand Up @@ -273,7 +322,7 @@
/// 1. Only processes cells that have changed since the last render.
/// 2. Tracks the last foreground color used to avoid unnecessary style changes
/// 3. Only creates clipping paths for potentially problematic glyphs (non-ASCII)
/// or when `always_clip_cells` is enabled.

Check warning on line 325 in src/backend/canvas.rs

View workflow job for this annotation

GitHub Actions / clippy

doc list item without indentation 

warning: doc list item without indentation
   --> src/backend/canvas.rs:325:9
    |
325 |     /// or when `always_clip_cells` is enabled.
    |         ^
    |
    = help: if this is supposed to be its own paragraph, add a blank line
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#doc_lazy_continuation
    = note: `#[warn(clippy::doc_lazy_continuation)]` on by default
help: indent this line
    |
325 |     ///    or when `always_clip_cells` is enabled.
    |         +++
fn draw_symbols(&mut self) -> Result<(), Error> {
let changed_cells = &self.changed_cells;
let mut index = 0;
Expand Down Expand Up @@ -405,7 +454,7 @@
fn draw_debug(&mut self) -> Result<(), Error> {
self.canvas.context.save();

let color = self.debug_mode.as_ref().unwrap();

Check warning on line 457 in src/backend/canvas.rs

View workflow job for this annotation

GitHub Actions / clippy

used `unwrap()` on an `Option` value 

warning: used `unwrap()` on an `Option` value
   --> src/backend/canvas.rs:457:21
    |
457 |         let color = self.debug_mode.as_ref().unwrap();
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: if this value is `None`, it will panic
    = help: consider using `expect()` to provide a better panic message
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unwrap_used
note: the lint level is defined here
   --> src/lib.rs:1:23
    |
1   | #![warn(missing_docs, clippy::unwrap_used)]
    |                       ^^^^^^^^^^^^^^^^^^^
for (y, line) in self.buffer.iter().enumerate() {
for (x, _) in line.iter().enumerate() {
self.canvas.context.set_stroke_style_str(color);
Expand Down
Loading