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

Removed IMG_Init() and IMG_Quit() #482

Merged
merged 3 commits into from
Dec 6, 2024
Merged

Conversation

slouken
Copy link
Collaborator

@slouken slouken commented Dec 3, 2024

No description provided.

@slouken slouken added this to the 3.2.0 milestone Dec 3, 2024
@slouken slouken requested review from icculus and sezero December 3, 2024 00:16
@sezero
Copy link
Contributor

sezero commented Dec 3, 2024

avif side looks broken to me:

--- a/src/IMG_avif.c
+++ b/src/IMG_avif.c
@@ -214,3 +214,3 @@ bool IMG_isAVIF(SDL_IOStream *src)
         /* This might be AVIF, do more thorough checks */
-        if (!IMG_InitAVIF()) {
+        if (IMG_InitAVIF()) {
             avifROData header;
@@ -368,3 +368,3 @@ SDL_Surface *IMG_LoadAVIF_IO(SDL_IOStream *src)
 
-    if ((IMG_InitAVIF()) {
+    if (!IMG_InitAVIF()) {
         return NULL;

@slouken
Copy link
Collaborator Author

slouken commented Dec 3, 2024

avif side looks broken to me:

Yes indeed, thank you!

@slouken
Copy link
Collaborator Author

slouken commented Dec 3, 2024

@madebr, did I break CI here?

@madebr
Copy link
Contributor

madebr commented Dec 3, 2024

@madebr, did I break CI here?

No, it's the latest libjxl that decodes jxl images in another way, with a delta-color bigger then our tests allow.
https://github.com/libsdl-org/SDL_image/actions/runs/12130547154/job/33821102023?pr=482#step:13:357

@madebr
Copy link
Contributor

madebr commented Dec 3, 2024

On the other hand, MSVC also fails which uses our vendored libjxl library. (We vendor an older version)
So that points at something weird going on.

@sezero
Copy link
Contributor

sezero commented Dec 3, 2024

@madebr, did I break CI here?

--- a/src/IMG_avif.c
+++ b/src/IMG_avif.c
@@ -118,3 +118,3 @@ static bool IMG_InitAVIF(void)
 
-    return false;
+    return true;
 }
diff --git a/src/IMG_webp.c b/src/IMG_webp.c
index 2541d86..57e76d3 100644
--- a/src/IMG_webp.c
+++ b/src/IMG_webp.c
@@ -100,3 +100,3 @@ static bool IMG_InitWEBP(void)
 
-    return false;
+    return true;
 }

IMG_Init() and IMG_Quit() are no longer necessary. If an image format requires dynamically loading a support library, that will be done automatically.
Copy link
Collaborator

@icculus icculus left a comment

Choose a reason for hiding this comment

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

Getting various drivers to init on demand is good, but how do they clean up without an explicit quit? I saw one of these calls CoCreateInstance during init, for example.

@slouken
Copy link
Collaborator Author

slouken commented Dec 3, 2024

Getting various drivers to init on demand is good, but how do they clean up without an explicit quit? I saw one of these calls CoCreateInstance during init, for example.

They don't, but in practice I don't think unloading the image DLLs is that important these days. The WIC driver is the only one that does CoCreateInstance(), and I think we can potentially just drop that.

@slouken slouken merged commit e815e79 into libsdl-org:main Dec 6, 2024
3 of 5 checks passed
@slouken slouken deleted the update branch December 6, 2024 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants