py-sdl / py-sdl2

Python ctypes wrapper around SDL2
Other
303 stars 49 forks source link

[RFC] Could not draw a diagonal line on a surface with 1 bpp. #242

Closed A-Wpro closed 2 years ago

A-Wpro commented 2 years ago

PR Description

Could not draw a diagonal line on a surface with 1 bpp. I had a problem to draw a diagonal line, probably I should have using byte-wises access. But for a bpp == 1. I think is easier to load a : pxbuf = ctypes.cast(rtarget.pixels, ctypes.POINTER(ctypes.c_uint8))

pxbuf was a int cause of : pxbuf = rtarget.pixels # byte-wise access. But later we ask for pxbuf[int(y1 * frac + x1)] = color

Now pxbuf for bpp == 1, pxbuf is a sdl2.audio.LP_c_ubyte. Btw, if someone know why we use sdl2.audio for graphical purpose, I would love to know.

Merge Checklist

a-hurst commented 2 years ago

@A-Wpro Thanks for this! Would you be willing to add a unit test for this as well, just so I can be sure this doesn't get broken again in the future? Also, can you remove the whitespace change on line 159?

Now pxbuf for bpp == 1, pxbuf is a sdl2.audio.LP_c_ubyte. Btw, if someone know why we use sdl2.audio for graphical purpose, I would love to know.

Where are you seeing this? That sounds like a mistake somewhere in PySDL2, but I can't seem to find anything in the code that would cause it. Regardless, that return type is just an alias for ctypes.POINTER(ctypes.c_ubyte) so it shouldn't actually cause problems apart from some user confusion.

A-Wpro commented 2 years ago

@A-Wpro Thanks for this! Would you be willing to add a unit test for this as well, just so I can be sure this doesn't get broken again in the future?

Yes, I will do it asap, in which file should I write this unit test? sdl2ext_draw_test ?

Also, can you remove the whitespace change on line 159?

Done.

Where are you seeing this? That sounds like a mistake somewhere in PySDL2, but I can't seem to find anything in the code that would cause it. Regardless, that return type is just an alias for ctypes.POINTER(ctypes.c_ubyte) so it shouldn't actually cause problems apart from some user confusion.

When you try to print type right after pxbufinitialization, after line 158 for example, you can see sdl2.audio.LP_c_ubyte as output.

a-hurst commented 2 years ago

@A-Wpro Sorry for the delay! Got busy with other things and haven't had a ton of time for PySDL2. To avoid adding new test images to the repository I wrote a different unit test for the bug, but I've applied that and your fix to the main branch here: https://github.com/py-sdl/py-sdl2/commit/3ee0ae4aa7f5501f544223fe1eeaf59a9ea57789

Thanks again for finding and fixing this!