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

Fix incorrectly drawn edges with fillpoly #2131

Merged
merged 5 commits into from
Oct 5, 2023

Conversation

Temmie3754
Copy link
Member

Resolves #515 by changing the intersection calculator to round up or down depending on if it is entering/exiting the shape.

@Temmie3754 Temmie3754 requested a review from a team as a code owner April 25, 2023 09:28
@Temmie3754
Copy link
Member Author

This also resolves #343 by making the behaviour consistent between the top and bottom.

@Temmie3754 Temmie3754 linked an issue Apr 26, 2023 that may be closed by this pull request
Copy link
Member

@ankith26 ankith26 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested this locally, and while I can confirm that this resolves #515 it somehow makes #343 worse by making both upward and downward triangles have non-smooth edges. Though atleast it's consistent that way now, IG

I quickly tested swapping floor and ceil in your code, and it actually fixes #343 completely

@Temmie3754
Copy link
Member Author

Temmie3754 commented Apr 27, 2023

Tested this locally, and while I can confirm that this resolves #515 it somehow makes #343 worse by making both upward and downward triangles have non-smooth edges. Though atleast it's consistent that way now, IG

I quickly tested swapping floor and ceil in your code, and it actually fixes #343 completely

Yes both being non smooth is intended, the aaline has to be adjusted to be in the correct position. Swapping floor and ceil can't be done since it will then make the shapes drawn inconsistent with the non-fill methods (and thus leave the issue of #515 unresolved). Can't have both of those behaviours at the same time.

Copy link
Member

@MyreMylar MyreMylar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can confirm that both issues are reasonably fixed.

For the AA outline of a filled shape issue, as the drawing algorithm is now consistent in upward and downwards direction you can adjust the outline in a predictable fashion for both directions, expanding the size of the AA version (so we can see it) and just flipping the y for up versus down. Adapting the original issue code we get:

import pygame
import pygame.gfxdraw

BLACK = (0, 0, 0)
RED = (255, 0, 0)
WHITE = (255, 255, 255)
pygame.init()

h = 600
w = 1000

screen = pygame.display.set_mode((w, h))
screen.fill(WHITE)

x0 = w // 4
y0 = h // 8
y1 = h // 2
tri0 = ((x0, y0), (2 * x0, y1), (3 * x0, y0))
tri1 = tuple((x, h + 10 - y) for x, y in tri0)

y_diff = 1
for tri in [tri0, tri1]:
    pygame.draw.polygon(screen, BLACK, tri)
    (x0, y0), (x1, y1), (x2, y2) = tri
    pygame.gfxdraw.aatrigon(screen, x0-2, y0-y_diff, x1, y1+y_diff, x2+2, y2-y_diff, BLACK)
    y_diff = -1


pygame.display.flip()

while True:
    for event in pygame.event.get():
        if event.type == pygame.QUIT:
            exit()

And smooth triangles whether facing upwards or downwards. I maintain it is still easier to get AA triangles in most cases by just drawing the triangle at 2x resolution and then smooth scaling the surface down to the desired size with transform.

I'm hesitant to spend too much time chasing after gfxdraw as I think we should be trying to eventually deprecate it - moving any useful functionality into draw - to reduce the amount of 'duplicate concept' submodules. It makes little sense to users for us to have a draw submodule, and then another draw submodule for no clear reason.

For the other issue the shapes now look like this:

image

Which looks much more even in both horizontal and vertical directions to me.

@yunline yunline added draw pygame.draw bugfix PR that fixes bug labels Jun 4, 2023
@MyreMylar MyreMylar added this to the 2.4.0 milestone Aug 14, 2023
Copy link
Contributor

@dr0id dr0id left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would welcome if there where some automated tests for this (as there is already code provided in the issue, could just turn that into tests).

@Temmie3754 Temmie3754 merged commit 8d0596b into pygame-community:main Oct 5, 2023
28 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix PR that fixes bug draw pygame.draw
Projects
None yet
5 participants