Skip to content

turtle location is wrong after several moves #41

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

Closed
ricknun opened this issue Nov 5, 2024 · 12 comments · Fixed by #45
Closed

turtle location is wrong after several moves #41

ricknun opened this issue Nov 5, 2024 · 12 comments · Fixed by #45

Comments

@ricknun
Copy link

ricknun commented Nov 5, 2024

Turtle command "forward(self, distance: float)" (and likely other commands) truncate turtle location to an integer value after each move. This can accumulate up to a 1 pixel error per turtle move. I would expect that the turtle location be maintained to floating point resolution and then be plotted to the nearest pixel each move. The attached program for a Circuit Playground Bluefruit + TFT Gizmo shows the problem. For example, 105 moves forward of 1.904762 pixels moves 105 pixels, not 105*1.904762 ~= 200. I am using CircuitPython v9.1.4. An image of running the attached .py program is attached too. It's possible this issue goes deeper than just Turtle Graphics -- I don't know what its underlying graphics system is.
turtle_truncate.txt
20241105_141322

@ricknun
Copy link
Author

ricknun commented Nov 5, 2024

This problem was found in a more complicated turtle graphics program and reduced to the attached turtle_truncate.py.

@ricknun
Copy link
Author

ricknun commented Nov 6, 2024

I checked Python v3.13 turtle graphics (see https://docs.python.org/3/library/turtle.html) on my Windows 10 PC. As expected, the result there is that the attached code produces 3 lines each 200 pixels long. There, turtle location is maintained with floating point numbers. With many plot points Adafruit turtle graphics leaves the turtle significantly far from where it should be. The attached .py file also mentions another problem with Adafruit turtle graphics, described in issue #40.

turtle_truncate.txt

@ricknun ricknun changed the title turtle location is truncated to an integer after each move turtle location is wrong after several moves Nov 12, 2024
@ricknun
Copy link
Author

ricknun commented Nov 12, 2024

I changed the issue title to better indicate impact to the end user. As another example, here is Adafruit's own example code "turtle_overlayed_koch.py" running on CPB + TFT Gizmo. Notice the yellow many-move Koch snowflake overlays the others correctly in the upper-left but is way off to the lower-right. Any turtle image with many small moves ends up being significantly off from where it should be.

TurtleIntegerTruncateBug

@dhalbert
Copy link
Contributor

Can you see a straightforward fix for this, like always storing the actual location, and doing int rounding/truncation only when doing display? A PR would be welcome.

@ricknun
Copy link
Author

ricknun commented Nov 12, 2024

I tried to understand adafruit_turtle.py enough to suggest a fix but couldn't do so. It seems important that the overlaid Koch program (and example code in issue #40) used to work well -- It should be possible to back-rev adafruit_turtle.py and other files involved to find who broke the code when, but I don't know how to do that. If the person that broke it is still around I'd get them involved. If not, at least I'd know what they were trying to do and where they went wrong.

@dhalbert
Copy link
Contributor

Can you give a really simple example that shows the problem? Then we can try versions in systematic way and get to the exact change that caused the issue.

@ricknun
Copy link
Author

ricknun commented Nov 12, 2024

In the original post see turtle_truncate.txt (and rename it to .py). See comments in the file.

@dhalbert
Copy link
Contributor

dhalbert commented Nov 12, 2024

Did we have a discussion already about .mode("standard") vs .mode("logo")? "logo" appears to be the default. Did you try switching modes to "standard"? That changes the default heading orientation (you noted a difference in your example).

@ricknun
Copy link
Author

ricknun commented Nov 12, 2024

turtle.mode("standard") and "logo" has to do with issue #40 and is discussed there -- I think mode "standard" was default in the past and someone changed it to "logo", breaking the 4 example files sited in that issue. The issue here doesn't change between "standard" and "logo" modes. In the provided turtle_truncate.py program you can add turtle.mode("standard") and comment out the turtle.setheading(90) line, to get the same image as shown in the original post.

@FoamyGuy
Copy link
Contributor

FoamyGuy commented Dec 4, 2024

The mode standard vs. logo has been changed in #43.

With regards to the float rounding / location issue: I was able to successfully re-create the issue with the provided sample code that draws 3 lines that should be the same, but vary in size on circuitpython.

I also went back and tested with some of the very oldest releases of this library and found that they behave the same way. So this float rounding location issue is not something that changed over time, it always occurred and resulted in the position being different from expected if it could track the partial pixels and account for them like the CPython implementation does.

I looked into the code a bit and think that this section is likely where the rounding is occuring: https://github.com/adafruit/Adafruit_CircuitPython_turtle/blob/main/adafruit_turtle.py#L406-L451 When you give float values like 1.9 those get passed in to this function as xn and/or yn values. x0 and y0 are initialized to the current whole pixel x/y coordinates, manipulated some based on values in xn and yn and then ultimately cast to int and passed to _plot(). However I added some print statements before the casting and call to _plot() and found that x0 and y0 are actually already int values before they even get cast to int again. So something in the manipulation above is converting them to int and truncating it seems.

I did not come up with a fix for the issue though. Perhaps the information above can be helpful to anyone who wants to try to submit a fix for it.

@ricknun
Copy link
Author

ricknun commented Dec 4, 2024

Regarding "I also went back and tested with some of the very oldest releases of this library" -- Details?

I demonstrated the problem using Adafruit's example file turtle_overlayed_koch.py above. Its GitHub "Blame" display shows it is about 5 years old, so it probably (an assumption) worked correctly back then. The Blame display shows who worked on it and perhaps they are still available to get involved. They might look at the bad image above and say whether it existed in the past or not.

@FoamyGuy
Copy link
Contributor

FoamyGuy commented Dec 4, 2024

Details?

I tested release 1.1.1 https://github.com/adafruit/Adafruit_CircuitPython_turtle/releases/tag/1.1.1 which was only the 4th release of the library. I chose that one because I am testing on a TFT gizmo external display and I assumed from the release name that this is the first one that supported it.

I can try to test the first release on a device with a built-in display later.

jepler added a commit that referenced this issue Dec 4, 2024
Previously, the endpoint of the line was always moved
along in increments of 1 pixel, so that the endpoint would always be
rounded down. This could accumulate to give quite large differences
from what the program intended.

Ensure that "goto" always ends up storing the floating point endpoints
and that the line is drawn from the rounded-integer starting coordinate
and rounded-integer ending coordinate.

This makes the 3 test lines in the OP's "turtle_truncate.txt" example
be the same length.

Closes: #41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants