-
-
Notifications
You must be signed in to change notification settings - Fork 164
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
Move Surface towards compiling w/ SDL3 #3310
base: main
Are you sure you want to change the base?
Conversation
a6a7412
to
23be387
Compare
23be387
to
f02914d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, LGTM. On theme with the other SDL3 pixel format conversion PRs. The code makes sense to me and the tests pass locally.
@@ -775,7 +780,6 @@ surf_set_at(PyObject *self, PyObject *const *args, Py_ssize_t nargs) | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If anyone is worried about performance impact I used set_at here as a test of overhead.
Used this script
import time
import pygame
pygame.init()
screen = pygame.Surface((300,300))
start = time.time()
for _ in range(1000000):
screen.set_at((4,35), 0x12345678)
print(time.time() - start)
I saw no appreciable difference in runtime between main and this branch.
@@ -823,6 +836,8 @@ surf_set_at(PyObject *self, PyObject *const *args, Py_ssize_t nargs) | |||
break; | |||
case 3: | |||
byte_buf = (Uint8 *)(pixels + y * surf->pitch) + x * 3; | |||
// Shouldn't this be able to happen without awareness of shifts? | |||
// mapped color -> pixel and all. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this cannot be done because we don't have a Uint24
type, as most common architectures likely does not support 3-byte mov as a single instruction. So we have to break down the mapped color object back to 1-byte components for the assignment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good theory. Other places in the codebase use memcpys to do it, perhaps the author of this section didn't think of that.
Today I ported a few modules and worked on a few others.
I first did everything with explicit SDL2 vs SDL3 code to observe patterns, then I made a few new compatibility macros to match what I saw.
This PR is those macros + the low hanging fruit in surface.c.
This is my take on a #3215 PR.