turtle icon indicating copy to clipboard operation
turtle copied to clipboard

Set_Speed = "Instant" produces incorrect output

Open bharathk005 opened this issue 4 years ago • 10 comments

While setting the turtle speed to "instant" I am seeing issues with the fill color. Tried with turtle.set_speed(Speed::instant()); turtle.set_speed("instant");

image

bharathk005 avatar Mar 14 '21 15:03 bharathk005

Could you tell me which Rust version (rustc --version --verbose) and operating system you are on? Please also provide your Cargo.lock file and tell me the exact command you ran to reproduce this.

This is definitely odd because setting the speed has nothing to do with how fills are calculated. It just affects how quickly the points are recorded. Does this still happen if the speed is very high (e.g. at 25)?

sunjay avatar Mar 14 '21 19:03 sunjay

@sunjay rustc 1.50.0 (cb75ad5db 2021-02-10) windows cargo run --example rust (from the root directory) Attaching Cargo.lock

bharathk005 avatar Mar 15 '21 13:03 bharathk005

Cargo.lock.txt

bharathk005 avatar Mar 15 '21 13:03 bharathk005

@sunjay I missed mentioning that setting the speed to 25 does not reproduce the issue. Occurs only when speed is set to "instant".

bharathk005 avatar Mar 17 '21 03:03 bharathk005

Another thing I'd be curious to test is if it still happens if you set the IPC channel crate version to 0.14. We recently updated in #235 and this could happen if that crate is sending messages in the wrong order. (Sending messages in the wrong order shouldn't technically be possible because we always wait for a drawing to finish before sending the next one, but it's worth testing anyway just in case.)

sunjay avatar Mar 17 '21 03:03 sunjay

IPC channel is already in 0.14. I think #235 was merged 2 days ago. I never pulled from upstream after that.

bharathk005 avatar Mar 17 '21 03:03 bharathk005

Ah okay. We can rule that out then. For anyone looking to work on this: I suggest printing out the messages being sent to the renderer process and making sure they are the same regardless of the speed. I also suggest printing out the display list and comparing at different speeds. Note that you don't need to run the entire example to debug this, just up to a portion of the drawing that appears different at different speeds.

sunjay avatar Mar 17 '21 03:03 sunjay

I put some print messages in rust example also in the send method of ipc_protocol Could see that setting "instant" and 25 produces the same output.

image

image

bharathk005 avatar Mar 17 '21 15:03 bharathk005

Right. That's good. Then the next place I'd look is whether the display lists are the same.

sunjay avatar Mar 17 '21 17:03 sunjay

The issue arises from the way MoveAnimation deals with Instant speed. More precisely, on line 126 below: https://github.com/sunjay/turtle/blob/c9918c4e963a8257fb91eda0e355bac6c0f625fa/src/renderer_server/animation.rs#L118-L127 we update fill polygon with old position (or current position before animation) instead of new or target position.

sample code to reproduce:

use turtle::Turtle;

fn main() {
    let mut turtle = Turtle::new();
    turtle.set_speed(x);

    turtle.begin_fill();
    for _ in 0..2 {
        turtle.forward(200.0);
        turtle.right(90.0);
    }
    turtle.end_fill();
}

x is not instant x is instant
Screenshot from 2021-07-17 11-26-22 Screenshot from 2021-07-17 11-23-09

display_list.polygon_push(poly_handle, target_pos) will fix the issue.

sathwikmatsa avatar Jul 17 '21 06:07 sathwikmatsa