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

Bump SDL_image to 2.7.0 #2198

Closed
wants to merge 4 commits into from
Closed

Bump SDL_image to 2.7.0 #2198

wants to merge 4 commits into from

Conversation

yunline
Copy link
Contributor

@yunline yunline commented May 26, 2023

Continue from #2182
To support animation

@yunline yunline added buildconfig dependencies CI Issue with the Continuous Integration (CI), the actions/bots that test things labels May 26, 2023
@yunline yunline mentioned this pull request May 26, 2023
@yunline yunline marked this pull request as ready for review May 27, 2023 01:37
@yunline yunline requested a review from a team as a code owner May 27, 2023 01:37
@yunline
Copy link
Contributor Author

yunline commented May 27, 2023

@oddbookworm
Copy link
Member

>>> pygame.print_debug_info()
Platform:               Windows-10-10.0.22621-SP0
System:                 Windows
System Version:         10.0.22621
Processor:              AMD64 Family 23 Model 8 Stepping 2, AuthenticAMD
Architecture:           Bits: 64bit     Linkage: WindowsPE
Driver:                 Display Not Initialized

Python:                 CPython
pygame version:         2.3.0.dev3
python version:         3.11.1

SDL versions:           Linked: 2.26.4  Compiled: 2.26.4
SDL Mixer versions:     Linked: 2.6.3   Compiled: 2.6.3
SDL Font versions:      Linked: 2.20.2  Compiled: 2.20.2
SDL Image versions:     Linked: 2.7.0   Compiled: 2.7.0
Freetype versions:      Linked: 2.11.1  Compiled: 2.11.1

@oddbookworm
Copy link
Member

Seems to work perfectly locally

@oddbookworm
Copy link
Member

I'll test further on other OS's at some point today

with:
vs-version: '17'

- name: Install CMake
Copy link
Member

Choose a reason for hiding this comment

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

@@ -25,7 +23,6 @@ DEBUG =
#everything you can, but you can ignore ones you don't have
#dependencies for, just comment them out

imageext src_c/imageext.c $(SDL) $(IMAGE) $(PNG) $(JPEG) $(DEBUG)
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather not split this into win/non-win versions.

Jpeg is not used directly by imageext anymore (removed by me a while back), it's just vestigial in buildconfig. So it should work to only link libpng both places.

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.

Looks like this needs a bit of love to keep up with main, and some adjustments from @Starbuck5's feedback. I think @ankith26 should also look it over as I know they are doing stuff with windows dependencies at the moment.

@MyreMylar
Copy link
Member

Seems like this is superseded by moving to version 2.8.0 in this PR: #2596

@MyreMylar MyreMylar marked this pull request as draft December 10, 2023 16:54
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.

We will be moving away from the old python buildconfig, and on windows we still want to keep deps precompiled and not compiled at any pygame dev machines.

This PR did some nice work, but we are on SDL_image 2.8.0 on main, doing things differently than this PR. Thanks for the work anyways.

@ankith26 ankith26 closed this Dec 11, 2023
@Starbuck5
Copy link
Member

Yes, thank you yunline for working on tackling this important item. I just wasn't sure what the right strategy was.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buildconfig CI Issue with the Continuous Integration (CI), the actions/bots that test things dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants