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

Should clear errors after successful load with STB backend? #293

Open
a-hurst opened this issue Jul 25, 2022 · 2 comments
Open

Should clear errors after successful load with STB backend? #293

a-hurst opened this issue Jul 25, 2022 · 2 comments

Comments

@a-hurst
Copy link

a-hurst commented Jul 25, 2022

This is pretty minor, but I think it's worth reporting regardless: with the new STB PNG/JPG back end, reading in a JPEG image results in IMG_GetError() being populated with "Not a PNG" despite the image loading successfully (even with IMG_LoadJPEG_RW).

Not sure if it's a quirk with how stb_image identifies file formats or not, but a one-line fix would presumably be to just always call IMG_ClearError right before an image is successfully returned to avoid any confusion (e.g. encountering a different problem later that doesn't set an error, and getting a head-scratching "Not a PNG" when checking GetError for information). Would that approach work, or are there legitimate cases where you'd want an error set while still returning a valid image surface?

Thanks in advance!

@slouken
Copy link
Collaborator

slouken commented Jul 25, 2022

I'm fine with adding that, but in general SDL functions may set errors even if they eventually return successfully. The application shouldn't check the error message to determine if a function succeeds or fails, only to find more detail if a function returns failure.

@a-hurst
Copy link
Author

a-hurst commented Jul 25, 2022

That makes sense (and is usually how I write things), the only time I check for error text before return value is in various PySDL2 unit tests just to make sure I get any error message before asserting a return value (how I caught this), and it's easy enough to ignore non-fatal errors in that context when I hit them.

I'm mostly reporting this because this specific error text is misleading and confusing; I spent a while trying to figure out whether I'd accidentally saved my JPEG with a .png extension before looking at the source and realizing it was an internal STB message.

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

No branches or pull requests

2 participants