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

[denballakh]

trafficstars

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:

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:

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

[Gabryel-lima]

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

[Gabryel-lima]

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]

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]

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]

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]

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]

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