Skip to content
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.

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

feat(widget): add circle widget #159

Merged
merged 1 commit into from
May 9, 2023
Merged

Conversation

fujiapple852
Copy link
Contributor

Adds a simple Circle widget.

This was something I needed to show an accuracy radius on the world map and thought it may be useful to upstream. There may be more efficient circle drawing algorithms, especially for low-res terminal UIs, this is just the naive textbook implementation.

@joshka
Copy link
Member

joshka commented Apr 29, 2023

Good idea.

Naive is probably fine, though if you care about it you can avoid 7/8s of the trig calls by reflecting at 45 degrees and plotting that point 8 times. (e.g. at (x,y),(y,x),(-y,x),(-x,y),(-x,-y),(-y,-x),(x,-y),(y,-x)). And Bresenham's algorithm is there if you want ever need to avoid the calls altogether and also decrease (or increase for large circles) the number of points actually plotted.

One thing missing on this is tests. Would you mind adding some? Unit tests can be as simple as:

let mut buffer = Buffer::empty(Rect::new(0, 0, 10, 10));
let canvas = ...

canvas.render(buffer.area, buffer);

let expected = Buffer::with_lines(vec![
  "    ....     "
  "  .      .   "
]);
assert_eq(buffer, expected);

Also, perhaps add a circle to the demo / canvas examples?

@fujiapple852
Copy link
Contributor Author

fujiapple852 commented May 1, 2023

@joshka I've added a unit test (trickier than you'd think to create a unit-test sized example!) and also added a Circle to the demo app on the map.

@fujiapple852
Copy link
Contributor Author

I asked phind.com (basically ChatGPT 4) to apply your PR comment feedback to the algorithm and it come up with the following:

impl Shape for Circle {
    fn draw(&self, painter: &mut Painter<'_, '_>) {
        let mut x = 0;
        let mut y = self.radius as i64;
        let mut d = 3 - 2 * (self.radius as i64);

        while x <= y {
            let points = [
                (self.x + x as f64, self.y + y as f64),
                (self.x + y as f64, self.y + x as f64),
                (self.x - x as f64, self.y + y as f64),
                (self.x - y as f64, self.y + x as f64),
                (self.x + x as f64, self.y - y as f64),
                (self.x + y as f64, self.y - x as f64),
                (self.x - x as f64, self.y - y as f64),
                (self.x - y as f64, self.y - x as f64),
            ];

            for &(x, y) in points.iter() {
                if let Some((x, y)) = painter.get_point(x, y) {
                    painter.paint(x, y, self.color);
                }
            }

            if d < 0 {
                d += 4 * x as i64 + 6;
            } else {
                d += 4 * (x as i64 - y as i64) + 10;
                y -= 1;
            }
            x += 1;
        }
    }
}

I tried it locally and it works, though I thought the circles weren't as round for low res terminal UIs. So I'd say better to stick to the naive algorithm.

Copy link
Member

@joshka joshka left a comment

Choose a reason for hiding this comment

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

LGTM - the make it fast part of this is not blocking

examples/demo/ui.rs Show resolved Hide resolved
src/widgets/canvas/circle.rs Show resolved Hide resolved
@joshka joshka added the Type: Enhancement New feature or request label May 5, 2023
Copy link
Member

@orhun orhun left a comment

Choose a reason for hiding this comment

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

Tested, works well. Approved!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants