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 behaves inconsistently when provided rect width or height are negative #2727

Open
denballakh opened this issue Feb 25, 2024 · 7 comments · May be fixed by #3158
Open

pygame.draw.ellipse behaves inconsistently when provided rect width or height are negative #2727

denballakh opened this issue Feb 25, 2024 · 7 comments · May be fixed by #3158
Labels
bug Not working as intended draw pygame.draw good first issue

Comments

@denballakh
Copy link

Environment:

  • pygame-ce 2.4.1 (SDL 2.28.5, Python 3.12.0):

Current behavior:
Consider this code:

import pygame as pg
s = pg.display.set_mode((250, 250))
# draw the outline where other circles will (or will not) be drawn:
pg.draw.ellipse(s, 'darkgrey', ( 50,  50,  50,  50), width=1)
pg.draw.ellipse(s, 'darkgrey', (150,  50,  50,  50), width=1)
pg.draw.ellipse(s, 'darkgrey', ( 50, 150,  50,  50), width=1)
pg.draw.ellipse(s, 'darkgrey', (150, 150,  50,  50), width=1)
# actual bug:
pg.draw.ellipse(s, 'red',      ( 50,  50,  50,  50), width=1) # normal
pg.draw.ellipse(s, 'green',    (150,  50,  50, -50), width=1) # not drawn
pg.draw.ellipse(s, 'blue',     ( 50, 150, -50,  50), width=1) # drawn, but to the left and filled
pg.draw.ellipse(s, 'yellow',   (150, 150, -50, -50), width=1) # not drawn

pg.display.update()
while True: pg.event.get()

When I run it I see this:
image

2/3 weird circles are not draws (its ok), one of them is drawn to the left and is filled.

If I do not provide width=1, then same issue appears:
image

Expected behavior:
Either all weird-circles are draw, or none of them.

@denballakh denballakh added the bug Not working as intended label Feb 25, 2024
@Gabryel-lima
Copy link

I'm trying to see if I can fix this, for me it gave me the same problem as you reported.

@Gabryel-lima
Copy link

So, is the negative value really necessary? I think there is no need for a negative value. And in the #Normal annotation you are superimposing the red color on top of the dark gray

@oddbookworm
Copy link
Member

I think the solution I would prefer to go with is to just disallow negative rect width and negative rect height here. That scenario doesn't really make any sense anyway IMO and I don't know what the "best" solution would be here. A rect constructed from (50, 50, -50, -50) for example is supposed to have it's top-left corner at (50, 50). But, with a width and height of -50, the only reasonable way I can interpret that is "go 50 pixels left and 50 pixels up to get the bottom right corner relative to the top left corner". A better way to get that IMO would be just to create the rect (0, 0, 50, 50)

@Gabryel-lima
Copy link

Acho que a solução que eu preferiria é apenas proibir largura reta negativa e altura reta negativa aqui. De qualquer forma, esse cenário não faz sentido, IMO e não sei qual seria a "melhor" solução aqui. Um retângulo construído a partir de (50, 50, -50, -50), por exemplo, deve ter seu canto superior esquerdo em (50, 50). Mas, com largura e altura de -50, a única maneira razoável de interpretar isso é "ir 50 pixels para a esquerda e 50 pixels para cima para obter o canto inferior direito em relação ao canto superior esquerdo". A melhor maneira de obter esse IMO seria apenas criar o retângulo (0, 0, 50, 50)

I agree

@ankith26
Copy link
Member

ankith26 commented Feb 29, 2024

Marking this as a good first issue.

Things to do to resolve this

  • write C code that raises python exception if rect with negative width or height is passed
  • update docs for this change (with versionchanged tag)
  • update tests to test for the newly added codepath

@denballakh
Copy link
Author

i dont like the idea of raising exception in this case
i would prefer it to be silently ignored

such rects can pop up as result of some complicated calculations, and such random exception might be surprising

on the other side, pep20 says that errors should never pass silently

@Gabryel-lima
Copy link

Marking this as a good first issue.

Things to do to resolve this

  • write C code that raises python exception if rect with negative width or height is passed
  • update docs for this change (with versionchanged tag
  • update tests to test for the newly added codepath

I want to try to solve this problem. I'm going to do it using Linux on an old pc that I'm installing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Not working as intended draw pygame.draw good first issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants