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

Possible bug in fill_triangle() method #33

Closed
peter-l5 opened this issue May 5, 2023 · 4 comments · Fixed by #34
Closed

Possible bug in fill_triangle() method #33

peter-l5 opened this issue May 5, 2023 · 4 comments · Fixed by #34

Comments

@peter-l5
Copy link
Contributor

peter-l5 commented May 5, 2023

The fill_triangle method seems to be drawing triangles incorrectly around the middle vertex (in the vertical direction). This appears to be related to the method drawing a horizontal line with the same y co-ordinate twice at the level of the middle vertex. The behaviour happens because the final value of the loop variable y in the loop used to fill the top part of the triangle is repeated for the first line used to fill the bottom part of the triangle. The code from the two loops is below for reference.

The images below illustrate the issue around the middle vertex.

I have developed and tested a fix for this employed in the repository framebuf2 and will post it here as a pull request separately in the next day or two. While I realise that this library is due to be deprecated soon, I figure it may still be worthwhile to fix it.

        for y in range(y0, last + 1):
            a = x0 + sa // dy01
            b = x0 + sb // dy02
            sa += dx01
            sb += dx02
            if a > b:
                a, b = b, a
            self.hline(a, y, b - a + 1, *args, **kwargs)
        sa = dx12 * (y - y1)
        sb = dx02 * (y - y0)
        while y <= y2:
            a = x1 + sa // dy12
            b = x0 + sb // dy02
            sa += dx12
            sb += dx02
            if a > b:
                a, b = b, a
            self.hline(a, y, b - a + 1, *args, **kwargs)
            y += 1

fill_triangle_issue1
fill_triangle_issue2

peter-l5 added a commit to peter-l5/Adafruit_CircuitPython_GFX-fix-for-33 that referenced this issue May 5, 2023
This fix for issue adafruit#33 replaces the for loop at lines 283 to 291 with a while loop so that the y variable is properly incremented between the loops to fill the top and bottom parts of the triangle. This avoids screen row at y0 being drawn twice, and corrects the shape of the triangle. The changes to lines 278 to 282 (as numbered after the fix) initiate the two loops in a way that addresses issue adafruit#22 where a triangle with a flat bottom edge was not drawn correctly.

This fix has been implemented in the derived repository  [framebuf2](https://github.com/peter-l5/framebuf2) which applies the algorithm here and has been  tested with the following code. (Note that `f=True` parameter draws a filled triangle in this implementation.)
```
    for z in range(3,123+1,20):
        for x in range(10,120+1,20):
            for y in range(0,128):
                display.fill(0)
                display.triangle(x,y,30,z,90,63,c=1,f=True)
                display.show()
```
@peter-l5
Copy link
Contributor Author

peter-l5 commented May 5, 2023

Hi, I submitted some code to fix this under #34, however this failed the automated workflow testing before getting to a stage where it could be reviewed. I'm afraid I don't know how to resolve this issue in order that the test would be passed.

@jposada202020
Copy link
Contributor

Hello @peter-l5 :), you could use pre-commit, it would reformat the files for the CI to pass. You could follow the instructions here
https://learn.adafruit.com/creating-and-sharing-a-circuitpython-library/check-your-code
or here
https://learn.adafruit.com/improve-your-code-with-pylint/overview

If you have any further questions, we could discuss in the Adafruit discord server :).

@peter-l5
Copy link
Contributor Author

peter-l5 commented May 5, 2023

Thanks! @jposada202020, I will take a look and try that out, might be a few days before I can pick this up again - just to be aware.

@jposada202020
Copy link
Contributor

No worries :) take your time

peter-l5 added a commit to peter-l5/Adafruit_CircuitPython_GFX-fix-for-33 that referenced this issue May 6, 2023
@tekktrik tekktrik linked a pull request May 8, 2023 that will close this issue
dhalbert added a commit that referenced this issue May 11, 2023
fix fill_triangle bug issue #33
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.

2 participants