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

pygame.draw.ellipse does not draw ellipses with odd sizes #233

Closed
illume opened this issue Feb 15, 2015 · 7 comments

Comments

Projects
None yet
3 participants
@illume
Copy link
Member

commented Feb 15, 2015

Steps to fix this issue of "pygame.draw.ellipse not being able to draw an odd sized ellipse".

  • write a unit test that shows the problem of odd sizes not being possible inside tests/draw_test.py (good first issue DONE)
  • fix draw ellipse in src_c/draw.c (harder, not a good first issue)

https://www.pygame.org/docs/ref/draw.html#pygame.draw.ellipse


Originally reported by: donLorenzo (Bitbucket: donLorenzo, GitHub: donLorenzo)


Reported by Florian Krause on the pygame mailing list:

#!plain

I was actually wrong. It seems even worse. Pygama is not capable to draw ellipses with odd sizes at all. It will always be the same ellipse as x-1, y-1.
E.g. an elipse with size 3,3 will result in the same ellipse as the one with size 2,2.
Can anyone reproduce this?

Code to reproduce:

#!python

import pygame
pygame.init()
screen = pygame.display.set_mode((100, 100))
pygame.draw.ellipse(screen, (255,0,0), (0,0,5,5))
pygame.display.update()

The resulting ellipse will have a 4x4 size


@illume

This comment has been minimized.

Copy link
Member Author

commented Feb 15, 2015

Original comment by Aleksandar Petrovic (Bitbucket: PythonSerb, GitHub: Unknown):


It's very small ellipse, its very diificult seen difference. Elipse for same size x and y are circle.I think there is no bug.

@illume

This comment has been minimized.

Copy link
Member Author

commented Feb 15, 2015

Original comment by donLorenzo (Bitbucket: donLorenzo, GitHub: donLorenzo):


Well here is a larger ellipse with a vertical line as visual aid.

#!python

import pygame
pygame.init()
screen = pygame.display.set_mode((200,200))
pygame.draw.line(screen, (127,127,127), (61,0), (61,120))
pygame.draw.ellipse(screen, (255,255,255), (0,0,61,61))
pygame.draw.ellipse(screen, (255,255,255), (0,62,60,60))
pygame.display.update()

Notice that the two ellipses have the same distance from the vertical line but the upper one should be one pixel larger (touching the line).

So, there is a bug unless you can show me otherwise and not just assert it.
issue233.png

@illume

This comment has been minimized.

Copy link
Member Author

commented Feb 15, 2015

Original comment by Aleksandar Petrovic (Bitbucket: PythonSerb, GitHub: Unknown):


Its not bug, its beacuce line of both ellipse have aliansing efects and diferenc of 1 pixel is invisible. If 2. elipse have size 59, difference is visible.

@illume

This comment has been minimized.

Copy link
Member Author

commented Feb 17, 2015

Original comment by donLorenzo (Bitbucket: donLorenzo, GitHub: donLorenzo):


I drew two ellipses one with 20x20 and one with 21x21 size.
I did this twice. once in the GIMP and once with pygame

issue233.png

By looking at the code I understand why pygame draws two ellipses of identical size. What I don't understand is why we consider this okay.

I think if the API was draw.ellipse(center, x_radius, y_radius) you might have a point that we cannot have floating point radii but given the current API of draw.ellipse(x,y,w,h) I certainly would expect a result similar to the GIMP's output.

@illume

This comment has been minimized.

Copy link
Member Author

commented Mar 16, 2015

Original comment by Florian Krause (Bitbucket: fladd, GitHub: fladd):


Absolutely, yes.

I would argue to either

  1. Make all valid diameter integer values produce the expected output

or

  1. Make the ellipse work with radii only

I strongly prefer 1) though.

@illume illume added this to the 1.9.2 milestone Mar 26, 2017

@illume illume added good first issue and removed 1.9.2 labels Aug 16, 2018

amosbastian added a commit to amosbastian/pygame that referenced this issue Aug 21, 2018

Update unit test pygame.draw.ellipse
Creates a surface containing an ellipse with equal width and
height. It then checks for each border if it contains the
colour of the ellipse, which it should.

Resolves unit test task of pygame#233

illume added a commit that referenced this issue Aug 23, 2018

amosbastian added a commit to amosbastian/pygame that referenced this issue Aug 25, 2018

Update test_ellipse for ellipses not same size surface
The unit test now includes a test that checks if at least two
sides of an ellipse, that has a width and height that is 1 pixel
smaller than the surface, is touching the surface's border.

It also cleans up the code slightly, including a better way of
retrieving the border's values and adds reusability.

See: pygame#233

amosbastian added a commit to amosbastian/pygame that referenced this issue Aug 27, 2018

Update test_ellipse for ellipses not same size surface
The unit test now includes a test that checks if at least two
sides of an ellipse, that has a width and height that is 1 pixel
smaller than the surface, is touching the surface's border.

It also cleans up the code slightly, including a better way of
retrieving the border's values and adds reusability.

See: pygame#233
@Earlo

This comment has been minimized.

Copy link

commented Feb 8, 2019

Did #511 fix this? Should this be closed?

@charlesej

This comment has been minimized.

Copy link
Contributor

commented Feb 16, 2019

Yes and yes. This issue has been resolved by PR #511.

@charlesej charlesej closed this Feb 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.