Skip to content

FrameBuffer.ellipse goes into infinite loop with 0 radius #16053

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.

Sign up for GitHub

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

Jump to bottom
Closed
corranwebster opened this issue Oct 21, 2024 · 3 comments
Closed

FrameBuffer.ellipse goes into infinite loop with 0 radius #16053

corranwebster opened this issue Oct 21, 2024 · 3 comments
Labels
bug

Comments

@corranwebster
Copy link
Contributor

Port, board and/or hardware

unix, RP2

MicroPython version

MicroPython v1.23.0 on 2024-06-02; Raspberry Pi Pico with RP2040
MicroPython v1.23.0 on 2024-05-31; darwin [GCC 4.2.1] version

Reproduction

In an interactive session (or otherwise), run the following code:

import framebuf
buf = bytearray(10*10*2)
f = framebuf.FrameBuffer(buf, 10, 10, framebuf.RGB565)
f.ellipse(5, 5, 0, 0, 0xffff, True)

Expected behaviour

Nothing should be drawn. Code should complete normally.

Maybe an exception should be raised, but I think it's OK to just draw nothing.

Observed behaviour

Micropython hangs; 100% CPU usage observed in unix port. Strong suspicion this is an infinite loop.

Additional Information

I'm very suspicious of the following code:

micropython/extmod/modframebuf.c

Lines 542 to 554 in 1b89c50

while (stoppingx >= stoppingy) { // 1st set of points, y' > -1
draw_ellipse_points(self, args[0], args[1], x, y, args[4], mask);
y += 1;
stoppingy += two_asquare;
ellipse_error += ychange;
ychange += two_asquare;
if ((2 * ellipse_error + xchange) > 0) {
x -= 1;
stoppingx -= two_bsquare;
ellipse_error += xchange;
xchange += two_bsquare;
}
}

In this case two_asquare and two_bsquare are both 0, so the stoppingx and stoppingy look like they are never changed, but I haven't 100% proven it to myself.

Code of Conduct

Yes, I agree
@corranwebster
Copy link
Contributor Author

Some more thought about this, and I think the expected behaviour should be to draw a single pixel at the centre.

corranwebster added a commit to unital/micropython that referenced this issue Nov 14, 2024
@corranwebster
extmod/modframebuf.c: Fix 0 radius bug in FrameBuffer.ellipse.
202b3e4 
This fixes a bug in FrameBuffer.ellipse where it goes into an infinite loop
if both radii are 0.

This fixes the bug with a simple pre-check to see if both radii are 0, and
in that case sets a single pixel at the center. This is consistent with the
behaviour of the method when called with just one of the radii set to 0,
where it will draw a horizontal or vertical line of 1 pixel width.

The pixel is set with setpixel_checked so it should handle out-of-bounds
drawing correctly.

This fix also includes two new tests: one for the default behaviour, and
one for drawing out-of-bounds.

Fixes micropython#16053.

Signed-off-by: Corran Webster <cwebster@unital.dev>
@corranwebster corranwebster mentioned this issue Nov 14, 2024
Closed
@mattytrentini
Copy link
Contributor

Some more thought about this, and I think the expected behaviour should be to draw a single pixel at the centre.

Either a single pixel or no pixel would be fine IMO. But infinite loop=bad. 😜

corranwebster added a commit to unital/micropython that referenced this issue Nov 14, 2024
@corranwebster
extmod/modframebuf.c: Fix 0 radius bug in FrameBuffer.ellipse.
581a590 
This fixes a bug in FrameBuffer.ellipse where it goes into an infinite loop
if both radii are 0.

This fixes the bug with a simple pre-check to see if both radii are 0, and
in that case sets a single pixel at the center. This is consistent with the
behaviour of the method when called with just one of the radii set to 0,
where it will draw a horizontal or vertical line of 1 pixel width.

The pixel is set with setpixel_checked so it should handle out-of-bounds
drawing correctly.

This fix also includes two new tests: one for the default behaviour, and
one for drawing out-of-bounds.

Fixes micropython#16053.

Signed-off-by: Corran Webster <cwebster@unital.dev>
corranwebster added a commit to unital/micropython that referenced this issue Nov 14, 2024
@corranwebster
extmod/modframebuf.c: Fix 0 radius bug in FrameBuffer.ellipse.
51b2569 
This fixes a bug in FrameBuffer.ellipse where it goes into an infinite loop
if both radii are 0.

This fixes the bug with a simple pre-check to see if both radii are 0, and
in that case sets a single pixel at the center. This is consistent with the
behaviour of the method when called with just one of the radii set to 0,
where it will draw a horizontal or vertical line of 1 pixel width.

The pixel is set with setpixel_checked so it should handle out-of-bounds
drawing correctly.

This fix also includes two new tests: one for the default behaviour, and
one for drawing out-of-bounds.

Fixes micropython#16053.

Signed-off-by: Corran Webster <cwebster@unital.dev>
corranwebster added a commit to unital/micropython that referenced this issue Nov 21, 2024
@corranwebster
extmod/modframebuf.c: Fix 0 radius bug in FrameBuffer.ellipse.
87c3ac3 
This fixes a bug in FrameBuffer.ellipse where it goes into an infinite loop
if both radii are 0.

This fixes the bug with a simple pre-check to see if both radii are 0, and
in that case sets a single pixel at the center. This is consistent with the
behaviour of the method when called with just one of the radii set to 0,
where it will draw a horizontal or vertical line of 1 pixel width.

The pixel is set with setpixel_checked so it should handle out-of-bounds
drawing correctly.

This fix also includes three new tests: one for the default behaviour, one
for drawing out-of-bounds, and one for when the sector mask is 0.

Fixes micropython#16053.

Signed-off-by: Corran Webster <cwebster@unital.dev>
corranwebster added a commit to unital/micropython that referenced this issue Nov 21, 2024
@corranwebster
extmod/modframebuf.c: Fix 0 radius bug in FrameBuffer.ellipse.
5a306f6 
This fixes a bug in FrameBuffer.ellipse where it goes into an infinite loop
if both radii are 0.

This fixes the bug with a simple pre-check to see if both radii are 0, and
in that case sets a single pixel at the center. This is consistent with the
behaviour of the method when called with just one of the radii set to 0,
where it will draw a horizontal or vertical line of 1 pixel width.

The pixel is set with setpixel_checked so it should handle out-of-bounds
drawing correctly.

This fix also includes three new tests: one for the default behaviour, one
for drawing out-of-bounds, and one for when the sector mask is 0.

Fixes micropython#16053.

Signed-off-by: Corran Webster <cwebster@unital.dev>
dpgeorge pushed a commit that referenced this issue Nov 28, 2024
@corranwebster @dpgeorge
extmod/modframebuf: Fix 0 radius bug in FrameBuffer.ellipse.
e70048c 
This fixes a bug in FrameBuffer.ellipse where it goes into an infinite loop
if both radii are 0.

This fixes the bug with a simple pre-check to see if both radii are 0, and
in that case sets a single pixel at the center. This is consistent with the
behaviour of the method when called with just one of the radii set to 0,
where it will draw a horizontal or vertical line of 1 pixel width.

The pixel is set with setpixel_checked so it should handle out-of-bounds
drawing correctly.

This fix also includes three new tests: one for the default behaviour, one
for drawing out-of-bounds, and one for when the sector mask is 0.

Fixes issue #16053.

Signed-off-by: Corran Webster <cwebster@unital.dev>
@dpgeorge
Copy link
Member

Fixed by e70048c

dpgeorge pushed a commit that referenced this issue Nov 28, 2024
@corranwebster @dpgeorge
extmod/modframebuf: Fix 0 radius bug in FrameBuffer.ellipse.
948863c 
This fixes a bug in FrameBuffer.ellipse where it goes into an infinite loop
if both radii are 0.

This fixes the bug with a simple pre-check to see if both radii are 0, and
in that case sets a single pixel at the center. This is consistent with the
behaviour of the method when called with just one of the radii set to 0,
where it will draw a horizontal or vertical line of 1 pixel width.

The pixel is set with setpixel_checked so it should handle out-of-bounds
drawing correctly.

This fix also includes three new tests: one for the default behaviour, one
for drawing out-of-bounds, and one for when the sector mask is 0.

Fixes issue #16053.

Signed-off-by: Corran Webster <cwebster@unital.dev>
wiznet-grace pushed a commit to wiznet-grace/micropython that referenced this issue Feb 27, 2025
@corranwebster @wiznet-grace
extmod/modframebuf: Fix 0 radius bug in FrameBuffer.ellipse.
f8b2148 
This fixes a bug in FrameBuffer.ellipse where it goes into an infinite loop
if both radii are 0.

This fixes the bug with a simple pre-check to see if both radii are 0, and
in that case sets a single pixel at the center. This is consistent with the
behaviour of the method when called with just one of the radii set to 0,
where it will draw a horizontal or vertical line of 1 pixel width.

The pixel is set with setpixel_checked so it should handle out-of-bounds
drawing correctly.

This fix also includes three new tests: one for the default behaviour, one
for drawing out-of-bounds, and one for when the sector mask is 0.

Fixes issue micropython#16053.

Signed-off-by: Corran Webster <cwebster@unital.dev>
wiznet-grace pushed a commit to WIZnet-ioNIC/WIZnet-ioNIC-micropython that referenced this issue Feb 28, 2025
@corranwebster @wiznet-grace
extmod/modframebuf: Fix 0 radius bug in FrameBuffer.ellipse.
d52281e 
This fixes a bug in FrameBuffer.ellipse where it goes into an infinite loop
if both radii are 0.

This fixes the bug with a simple pre-check to see if both radii are 0, and
in that case sets a single pixel at the center. This is consistent with the
behaviour of the method when called with just one of the radii set to 0,
where it will draw a horizontal or vertical line of 1 pixel width.

The pixel is set with setpixel_checked so it should handle out-of-bounds
drawing correctly.

This fix also includes three new tests: one for the default behaviour, one
for drawing out-of-bounds, and one for when the sector mask is 0.

Fixes issue micropython#16053.

Signed-off-by: Corran Webster <cwebster@unital.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

extmod/modframebuf.c: Fix 0 radius bug in FrameBuffer.ellipse. unital/micropython
3 participants
@mattytrentini @corranwebster @dpgeorge