Set_Speed = "Instant" produces incorrect output

[bharathk005]

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");

[sunjay]

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)?

[bharathk005]

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

[bharathk005]

Cargo.lock.txt

[bharathk005]

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

[sunjay]

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.)

[bharathk005]

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

[sunjay]

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.

[bharathk005]

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.

[sunjay]

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

[sathwikmatsa]

The issue arises from the way MoveAnimation deals with Instant speed. More precisely, on line 126 below:

turtle/src/renderer_server/animation.rs

Lines 118 to 127 in c9918c4

	if cfg!(any(feature = "test", test)) || speed.is_instant() {
	// Set to the final position and draw a line with no animation
	turtle.state.position = target_pos;
	let prim = display_list.push_line(position, target_pos, pen);
	turtle.drawings.extend(prim);
	// Append to the current fill polygon, if any
	let fill_poly_index = turtle.current_fill_polygon.map(|poly_handle| {
	display_list.polygon_push(poly_handle, position)
	});

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

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