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

Print filenames in irrlicht png warnings #14525

Merged
merged 2 commits into from Apr 7, 2024

Conversation

Desour
Copy link
Member

@Desour Desour commented Apr 6, 2024

Warnings like these are useless, because you don't know which file causes it:
WARNING[Main]: Irrlicht: PNG warning: iCCP: known incorrect sRGB profile
With PR, it looks like this:
WARNING[Main]: Irrlicht: PNG warning for abriglass_clearglass.png: iCCP: known incorrect sRGB profile

(libpng docs: http://www.libpng.org/pub/png/libpng-manual.txt)

The change in CNullDriver::createImageFromFile is needed because the jpg loader complains with an error if the file doesn't have the jpg signature.
isALoadableFileFormat only has false negatives in the case of "very old tgas", according to tga loader code comment.
Do we support those?

To do

This PR is a Ready for Review.

How to test

@Zughy Zughy added Code quality Trivial The change is a trivial bug fix, documentation or maintenance change, as per the Git Guidelines @ Documentation Textures labels Apr 6, 2024
irr/src/CImageLoaderPNG.cpp Outdated Show resolved Hide resolved
Copy link
Member

@sfan5 sfan5 left a comment

Choose a reason for hiding this comment

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

lgtm

@Desour Desour merged commit 1d673ce into minetest:master Apr 7, 2024
13 checks passed
@Desour Desour deleted the irrlicht_png_warn_filenames branch April 7, 2024 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code quality @ Documentation Textures Trivial The change is a trivial bug fix, documentation or maintenance change, as per the Git Guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants