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

Add PG_CreateSurface(From) for SDL2/3 compat #2481

Merged
merged 1 commit into from
Oct 1, 2023

Conversation

Starbuck5
Copy link
Member

@Starbuck5 Starbuck5 commented Sep 30, 2023

This was the end goal of my previous PRs switching out SDL_CreateRGBSurface for SDL_CreateRGBSurfaceWithFormat. This allows us to write our create surface calls in "SDL3 form" on both on SDL2 and SDL3, because of the macro.

You might be wondering about the depth argument always being 0. This is okay. In SDL2's SDL_CreateRGBSurfaceWithFormat, the depth argument is ignored. (https://github.com/libsdl-org/SDL/blob/f9dc49c21c99cd6eb63d80d60b6e706a51234b46/src/video/SDL_surface.c#L72-L78). This makes sense, as the depth info is in the pixelformat.

I like this patch as well because it actually simplifies the calling code (at least makes it shorter), as you can see with the line count reduction in this PR.

There are still a small few SDL_CreateRGBSurface calls in the wilds of the codebase, they'll get handled eventually.

@Starbuck5 Starbuck5 requested a review from a team as a code owner September 30, 2023 23:06
@Starbuck5 Starbuck5 added the sdl3 label Sep 30, 2023
Copy link
Member

@MyreMylar MyreMylar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this looks a lot neater, nice refactoring. 👍

Copy link
Member

@ankith26 ankith26 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the PR 🥳

@ankith26 ankith26 added this to the 2.4.0 milestone Oct 1, 2023
@ankith26 ankith26 merged commit dfdf8b8 into pygame-community:main Oct 1, 2023
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants