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

Port font to SDL3(_ttf) #3326

Merged
merged 1 commit into from
Feb 10, 2025
Merged

Port font to SDL3(_ttf) #3326

merged 1 commit into from
Feb 10, 2025

Conversation

ankith26
Copy link
Member

@ankith26 ankith26 commented Feb 3, 2025

SDL3_ttf preview release: https://github.com/libsdl-org/SDL_ttf/releases/tag/preview-3.1.0

With this PR pygame.font compiles on SDL3(_ttf)

@ankith26 ankith26 requested a review from a team as a code owner February 3, 2025 08:45
src_c/font.c Show resolved Hide resolved
Copy link
Member

@damusss damusss left a comment

Choose a reason for hiding this comment

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

Edit: slightly incorrect, check further comments, point is still valid
A lot if not all TTF functions in sdl3 (I assume?) have 0 on success and -1 on error, like TTF_SizeText or TTF_Init, so checking if (TTF_X) {error} is actually wrong and should be if (TTF_X < 0) {error}. I know this PR is supposed to compile it, but without this an extra PR can't be avoided to fix this so you might aswell change it now.

@gresm
Copy link
Contributor

gresm commented Feb 3, 2025

Correction:
SDL3 uses more of the bool type and in many places it returns true (1) to indicate success and false (0) for failure. In SDL2 it is (0) for success (no error) and (-1) for failure, so if (SDL_Func() < 0) {err} and if (SDL_Func()) {err} works for SDL2 and if (!SDL_Func()) {err} for SDL3, so there's no version that would work for both SDL2 and SDL3, different code paths are needed.

@damusss
Copy link
Member

damusss commented Feb 3, 2025

Correction: SDL3 uses more of the bool type and in many places it returns true (1) to indicate success and false (0) for failure. In SDL2 it is (0) for success (no error) and (-1) for failure, so if (SDL_Func() < 0) {err} and if (SDL_Func()) {err} works for SDL2 and if (!SDL_Func()) {err} for SDL3, so there's no version that would work for both SDL2 and SDL3, different code paths are needed.

I said my 0 and -1 returns while looking at my IDE docstrings, but my IDE was actually looking at SDL2 functions, so no < 0 is incorrect and will never raise (that's why those bug errors disappeared) so yeah a not operator is needed like you said.

@damusss damusss added font pygame.font sdl3 labels Feb 3, 2025
gresm
gresm previously requested changes Feb 3, 2025
Copy link
Contributor

@gresm gresm left a comment

Choose a reason for hiding this comment

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

Some things that I've caught, definitely not checked exhaustively.

src_c/font.c Outdated Show resolved Hide resolved
src_c/font.c Outdated Show resolved Hide resolved
src_c/font.c Outdated Show resolved Hide resolved
src_c/font.c Outdated Show resolved Hide resolved
src_c/font.c Outdated Show resolved Hide resolved
Copy link
Member

@Starbuck5 Starbuck5 left a comment

Choose a reason for hiding this comment

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

I haven't gone into the SDL3_ttf semantics as deeply as others have, but this all looks good to me, it compiles, I don't believe it will change SDL2 behavior.

@ankith26
Copy link
Member Author

Just force pushed with fixes to your 2 review points, without any other changes.

@ankith26 ankith26 requested review from damusss and gresm February 10, 2025 04:52
Copy link
Member

@damusss damusss left a comment

Choose a reason for hiding this comment

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

Alright, looking good now, thanks :3

@ankith26 ankith26 dismissed gresm’s stale review February 10, 2025 14:04

Review comments addressed.

@ankith26 ankith26 merged commit 0bd23aa into main Feb 10, 2025
23 of 25 checks passed
@ankith26 ankith26 deleted the ankith26-sdl3-ttf branch February 10, 2025 14:04
@ankith26 ankith26 added this to the 2.5.4 milestone Feb 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
font pygame.font sdl3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants