drawille-rs
drawille-rs copied to clipboard
Fix for Canvas.rows() output being 1 row/col too large when Canvas::new() is passed particular values
Fixes ftxqxd/drawille-rs#14, where output string can be 1 row and/or col too large resulting in additional white space when given width is divisible by 2 and/or height is divisible by 4.
The only time this issue occurs is when Canvas::new()
is called and the given width/height are divisible by the 'braille-pixel' ratio, where width is divisible by 2 and height by 4. For example if a width of 12 is given, the Canvas struct's width
would be 6 and when rows()
is called, the maxrow
variable would be off by one and cause an additional empty column to appear in the output string. Another example, if a height of 4 is given, the Canvas' height
would be 1 and when rows()
is called, the maxcol
variable would be set to 1, allowing the loop to run an additional iteration and inserting an empty row in the output string.
Prior to this fix, the following code:
let mut canvas = Canvas::new(2, 4);
for y in 0..4 {
for x in 0..2 {
canvas.set(x, y);
}
}
dbg!(canvas.frame());
Would output the following:
[examples\test_canvas_properties.rs:14] canvas.frame() = "⣿ \n "
And with the fix the output is instead:
[examples\test_canvas_properties.rs:14] canvas.frame() = "⣿"
Let me know if there's anything else I could do or if there's an issue with this pull request. Thank you!
An alternative to this quick fix would be to instead dynamically update the width
and height
member fields of Canvas whenever it is modified, with for example a check_bounds(u16,u16)
:
fn check_bounds(&mut self, row: u16, col: u16) {
if self.width < row + 1 {
self.width = row + 1;
}
if self.height < col + 1 {
self.height = col + 1;
}
}
And to also properly calculate the Canvas width and height in the constructor:
pub fn new(width: u32, height: u32) -> Canvas {
let width_adjustment: u16 = if width != 0 && width % 2 == 0 { 0 } else { 1 };
let height_adjustment: u16 = if height != 0 && height % 4 == 0 { 0 } else { 1 };
Canvas {
chars: FnvHashMap::default(),
width: (width / 2) as u16 + width_adjustment,
height: (height / 4) as u16 + height_adjustment,
}
}
With these changes, the bounds checking for loop in Canvas.rows() can be removed and the loop ranges become exclusive rather than inclusive:
pub fn rows(&self) -> Vec<String> {
let maxrow = self.width;
let maxcol = self.height;
let mut result = Vec::with_capacity(maxcol as usize + 1);
for y in 0..maxcol {
let mut row = String::with_capacity(maxrow as usize + 1);
for x in 0..maxrow {
// -- snip --
}
I have implemented these changes in a dev branch on my fork, /TristanTurcotte/drawille-rs/tree/dev
Let me know and I can push those changes into this Pull Request. I could also include Canvas.get_width()
and Canvas.get_height()
additions if you wish.