Adding arc() method to Turtle

[DeltaManiac]

Fixes #82

[sunjay]

Hello! Thanks for adding this method! Also, congrats on getting issue/PR number #100! That's a big milestone for the project! :tada:

My apologies for not getting back to your issue comment. I saw it, thought of my response, and then must have forgotten to actually send it. :sweat_smile:

Your implementation is a good start and I'd like to see how it behaves when it is used in a drawing. Could you add an example to the examples directory that uses this function to draw a couple of different arcs? In particular it would be good to try it with different pen thicknesses to see what happens. Try drawing parts of circles, entire circles, and then do bigger angles like 800 or something.

This will probably bring out some interesting edge cases and will help me verify the implementation.

I have some other review comments that I will add later when I have a chance to look at this PR in more detail. My response above should give you something to work on in the meantime.

Thanks again for adding this method! Let me know if you have any questions about any of this. :smile:

[DeltaManiac]

Hey @sunjay ! Congrats on this being PR#100! :)

Ive added a simple example that draws the beats logo.

I did notice some issues while working with different pen sizes.

For example the below code

    let mut turtle = Turtle::new();
    turtle.drawing_mut().set_background_color("black");
    let _radius= 50.0;
    turtle.set_pen_size(100.0);
    turtle.set_pen_color("light blue");
    turtle.arc(_radius, None);

seems to generate this weird pattern.

I've notice this becomes more evident as the pen size approaches radius.

Radius = 2.0

    let mut turtle = Turtle::new();
    turtle.drawing_mut().set_background_color("black");
    let _radius= 2.0;
    turtle.set_pen_size(100.0);
    turtle.set_pen_color("light blue");
    turtle.arc(_radius, None);

Radius = 15.0

    let mut turtle = Turtle::new();
    turtle.drawing_mut().set_background_color("black");
    let _radius= 15.0;
    turtle.set_pen_size(100.0);
    turtle.set_pen_color("light blue");
    turtle.arc(_radius, None);

Radius = 25.0

    let mut turtle = Turtle::new();
    turtle.drawing_mut().set_background_color("black");
    let _radius= 25.0;
    turtle.set_pen_size(100.0);
    turtle.set_pen_color("light blue");
    turtle.arc(_radius, None);

Any ideas on what could be causing this behavior?

Cheers Delta

[sunjay]

Hello! Thanks for continuing to work on this! :smile:

Any ideas on what could be causing this behavior?

Yes! This is actually exactly why I wanted you to test with different line thicknesses. I had a suspicion that we would run into this issue. Check out this code in the Rust logo example:

https://github.com/sunjay/turtle/blob/05a5fa118b154c5504bfc9fb770081c2846dd80d/examples/rust.rs#L136-L143

I suggest you try to replace those two movements in that code example with a single movement turtle.forward(0.5) to see what happens.

Explanation

The problem is that when turtle draws, it takes very small steps. If the pen thickness is very large, these steps become very wide but still only go forward a short distance. This ends up resulting in the turtle drawing several very thin lines perpendicular to itself. That's what you're seeing in the screenshots that you provided.

Potential Solution

The solution to this is to make our approach a little more sophisticated. Instead of drawing individual lines, we should probably use something like a CircleArc from the piston-graphics crate to draw a more complete shape.

Background

To give you an idea of what we would have to do, let's look at how turtle.forward is implemented.

This is the source code of that method:

https://github.com/sunjay/turtle/blob/05a5fa118b154c5504bfc9fb770081c2846dd80d/src/turtle.rs#L118-L120

This calls the method with the same name on TurtleWindow:

https://github.com/sunjay/turtle/blob/05a5fa118b154c5504bfc9fb770081c2846dd80d/src/turtle_window.rs#L127-L159

This method calculates the start, end, and duration of the movement animation, starts a timer, and then begins to play that animation synchronously.

https://github.com/sunjay/turtle/blob/05a5fa118b154c5504bfc9fb770081c2846dd80d/src/animation.rs#L67-L90

The animation is driven by the advance method on the MoveAnimation struct which calculates the state of the animation at the current point that it should be at based on the timer that we passed in initially. This works because we linearly interpolate between the start and end positions that we calculated in TurtleWindow::forward. We do that using the lerp function provided in the interpolation crate.

If the animation is still running, we pass in a Path that will be used to render the temporary path that just exists for the purpose of animation. Once the animation is complete, we return a Path that the renderer should store and continue to render from then onwards.

https://github.com/sunjay/turtle/blob/05a5fa118b154c5504bfc9fb770081c2846dd80d/src/turtle_window.rs#L184-L200

These paths are sent to the renderer process to be drawn by the renderer via a drawing command:

https://github.com/sunjay/turtle/blob/05a5fa118b154c5504bfc9fb770081c2846dd80d/src/query.rs#L25-L36

The drawing commands are processed in the following:

https://github.com/sunjay/turtle/blob/05a5fa118b154c5504bfc9fb770081c2846dd80d/src/renderer.rs#L153-L166

Then, the temporary path and each stored path are rendered in the renderer:

https://github.com/sunjay/turtle/blob/05a5fa118b154c5504bfc9fb770081c2846dd80d/src/renderer.rs#L200-L246

The result of all of this is the lines that show up on your screen!

Mentoring Instructions

To implement arc using CircleArc, we would probably have to do something similar to these steps. We would essentially be introducing a new drawing primitive like Path but specifically for drawing arcs. Along every step of the way, instead of drawing lines, we would need to draw part of an arc. We would probably need to introduce new drawing commands for arcs and even a new temporary_arc variable to mimic the temporary_path we have now. Instead of interpolating on the distance, we would interpolate on the angle in radians.

Looking at all of this, it is actually a pretty monumental task to get arc support setup! I'm so sorry I didn't see this sooner! It took your experimenting for me to realize that a more complicated solution is actually necessary.

If you would like to continue working on this, I can take you there step by step through a series of smaller PRs. This is too big to be done in a single contribution.

That being said, the size and scope of this task has suddenly gotten quite big so I totally understand if you don't have the time to work on it. :)

Let me know what you are willing to do (and have time for). Sorry again for not anticipating this sooner.

[DeltaManiac]

Hey @sunjay

Thanks for the detailed explanation about what causes the issue.

Regarding further course of action the changes necessary could be broken down into smaller issues which can be picked up as time permits.

[sunjay]

Thank you for being understanding! I will add more information to the original issue and close this PR for the time being. I really appreciate you investing your time and my apologies for not realizing the complexity of this issue sooner.