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

Add draw.aaellipse #3016

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open

Conversation

mzivic7
Copy link
Contributor

@mzivic7 mzivic7 commented Jul 22, 2024

This PR adds draw.aaellipse function, with width and thickness options.
It is completely interchangeable with draw.ellipse in terms of arguments.
Algorithm used is adapted from Xiaolin Wu's general fast antialiasing algorithm, and is very similar to draw.aacircle.
Only notable difference: in aacircle, 8 pixels are drawn at once, but here, only 4, because ellipse has 2 symmetries. Because of that aaelipse has additional vertical drawing (aacircle has only horizontal but because of extra symmetries it is vertical at the same time).
It is added to docs too.

Sample code
import pygame

pygame.init()
screen = pygame.display.set_mode((512, 512))
clock = pygame.time.Clock()

run = True
while run:
    for event in pygame.event.get():
        if event.type == pygame.QUIT:
            run = False
    screen.fill("black")
    pygame.draw.aaellipse(screen, "red", [100, 100, 100, 50], 1)
    pygame.draw.aaellipse(screen, "red", [100, 160, 100, 50])
    pygame.draw.aaellipse(screen, "red", [100, 220, 100, 50], 4)
    pygame.display.flip()
    clock.tick(60)
pygame.quit()

@mzivic7 mzivic7 requested a review from a team as a code owner July 22, 2024 18:44
@mzivic7 mzivic7 changed the title Draw.aaellipse() draw.aaellipse Jul 22, 2024
@mzivic7 mzivic7 mentioned this pull request Jul 22, 2024
19 tasks
@damusss damusss changed the title draw.aaellipse Add draw.aaellipse Jul 22, 2024
@damusss damusss added New API This pull request may need extra debate as it adds a new class or function to pygame draw pygame.draw labels Jul 22, 2024
@damusss damusss requested a review from MightyJosip July 22, 2024 20:53
src_c/draw.c Outdated Show resolved Hide resolved
@mzivic7 mzivic7 requested a review from MightyJosip July 31, 2024 13:03
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.

Well this looks good to me. The new algorithm performs better than the one in gfxdraw (which has gaps in it) and adds the filling functionality. Tested a variety of parameters locally and all looked nice and behaved how I would expect.

Can't find any problems with the code, not going to pretend I understand the intricacies of the algorithm for drawing, but it looks sensible from a look over and produces an output I respect. 👍

@mzivic7
Copy link
Contributor Author

mzivic7 commented Oct 5, 2024

Note: before merging, correct versionadded in docs/reST/ref/draw.rst, if needed.

src_c/draw.c Outdated Show resolved Hide resolved
src_c/draw.c Outdated Show resolved Hide resolved
update version in docs
comments
symetric -> symmetric
fix thickness > width/2 or height/2
fix bug with width/height < 4 with thickness
fix small vertical ellipse not same horizontal
fix small width=height drawing wrong circle
@mzivic7
Copy link
Contributor Author

mzivic7 commented Nov 9, 2024

I moved ellipse and aaellipse functions bellow circle and aacircle, because aacircle is called in aaellipse and aa* functions should always be bellow regular one.

aacircle is returning different return rect from aaellipse when it is 
off-surface
@Starbuck5
Copy link
Member

I see holes around the edges of the ellipse in this test case.

import pygame
screen = pygame.display.set_mode((600,600))
running = True
while running:
    for event in pygame.event.get():
        if event.type == pygame.QUIT:
            running = False
        if event.type == pygame.KEYDOWN and event.key == pygame.K_w:
            pygame.image.save(screen, "output.png")
    screen.fill("purple")
    pygame.draw.aaellipse(screen, "red", [100,200,300,400])
    pygame.display.flip()
pygame.quit()

output

@Starbuck5
Copy link
Member

Other things I see.

  • pygame.draw.aaellipse(screen, "red", [100,200,30,200000], width=0) crashes the app
  • putting in a rect with negative dimensions leads to weird stuff being drawn
  • if the width and the height are equal the surface being drawn on cannot be blitted in the future (you forgot to unlock it in this case)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
draw pygame.draw New API This pull request may need extra debate as it adds a new class or function to pygame
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants