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

SDL_image 2.8.0 #2596

Merged
merged 8 commits into from
Dec 11, 2023
Merged

SDL_image 2.8.0 #2596

merged 8 commits into from
Dec 11, 2023

Conversation

Starbuck5
Copy link
Member

It's a long time coming, since we've decided we need SDL_image built with libpng, and SDL_image prebuilts after 2.0.5 don't come with that.

This PR hopes to get around that using CI on a pygame-community fork of SDL_image (https://github.com/pygame-community/SDL_image/releases/tag/2.8.0-pgce) to generate release bundles with our parameters.

This allows our Windows CI and local development workflow to remain almost exactly the same through this change.

@Starbuck5 Starbuck5 requested a review from a team as a code owner December 10, 2023 07:42
@Starbuck5 Starbuck5 marked this pull request as draft December 10, 2023 07:50
@MyreMylar
Copy link
Member

There was a change made in CPython 3.13.0 & 3.12.1, the latter of which came out this week, removing skipped tests from the total number of completed tests in the unit test module. This caused our check that the number of started tests equalled the number of completed tests to fail. See:

python/cpython#110890

I initially assumed it was threading related due to the print output - but actually none of the threading code in our test running code is actually being used, and it doesn't actually work anymore.

In conclusion; we should probably be using pytest.

@MyreMylar MyreMylar marked this pull request as ready for review December 10, 2023 15:19
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.

All looks good to me. I see 2.8.0 as the version locally - the tests all pass and it works with all my GUI library examples.

Though please squash and merge to hide my messy experimentation :)

@Starbuck5 Starbuck5 force-pushed the starbuck-sdl-image-bump branch from da5d6bb to 05487ea Compare December 10, 2023 18:51
@Starbuck5
Copy link
Member Author

Thanks so much for investigating and fixing this!

Though please squash and merge to hide my messy experimentation :)

I squashed all your commits in the branch together, since I don't want to have all this end up as one commit. This also unintentionally adds me as a coauthor, which I don't think I can get around.

@Starbuck5 Starbuck5 force-pushed the starbuck-sdl-image-bump branch from 346d3b2 to 04a439d Compare December 10, 2023 19:19
@Starbuck5 Starbuck5 force-pushed the starbuck-sdl-image-bump branch from 6e8cbcd to 2d5d14f Compare December 10, 2023 19:37
@Starbuck5 Starbuck5 added this to the 2.4.0 milestone Dec 11, 2023
Copy link
Member

@oddbookworm oddbookworm left a comment

Choose a reason for hiding this comment

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

LGTM!

test/image_test.py Show resolved Hide resolved
docs/reST/ref/image.rst Show resolved Hide resolved
src_c/imageext.c Show resolved Hide resolved
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.

Gave this PR a good spin locally on linux and all is working well!

I tested both, the CI wheel and built-locally wheel (that in my case, would build against SDL_image 2.6.3) and both pass all tests as well as my little image loader test script.

This is a good simplification of things and will also help us get a lot of new features and maybe bugfixes 🥳

@ankith26 ankith26 merged commit 5d3e845 into main Dec 11, 2023
29 checks passed
@ankith26 ankith26 deleted the starbuck-sdl-image-bump branch December 11, 2023 08:49
pmp-p added a commit that referenced this pull request Jan 4, 2024
since #2596 static.c build was silently ignored (commented by setuptools) because PNG/JPEG not set anymore in Setup
@pmp-p pmp-p mentioned this pull request Jan 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants