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

Support libpng with no iTXt chunk support #571

Merged
merged 2 commits into from
May 25, 2018

Conversation

AbigailBuccaneer
Copy link
Contributor

@AbigailBuccaneer AbigailBuccaneer commented May 23, 2018

libpng's support for the iTXt chunk ('international textual data') is
optional, and off-by-default before libpng 1.4, which causes a compile
error. This writes an iTXt chunk only if supported, and writes a tEXt
chunk otherwise. (A tEXt chunk is identical but specified to contain
Latin-1 text instead of UTF-8, and has no language tag.)

libpng's support for the `iTXt` chunk ('international textual data') is
optional, and off-by-default before libpng 1.3, which causes a compile
error. This writes an iTXt chunk only if supported, and writes a tEXt
chunk otherwise. (A tEXt chunk is identical but specified to contain
Latin-1 text instead of UTF-8, and has no language tag.)
@Deledrius
Copy link
Member

Thank you for submitting this fix!

The switch to using iTXt rather than tEXt chunks happened pretty late in the work on this feature, and I wasn't aware of the version limitations. I don't see it in the libpng 1.2 documentation for these chunks at all. Do you have a reference for this information we can look to in the future?

I'll be able to give this a test in the next few days. Thanks again!

@Deledrius Deledrius requested review from zrax and Deledrius May 23, 2018 10:09
@AbigailBuccaneer
Copy link
Contributor Author

AbigailBuccaneer commented May 23, 2018

That's the documentation for the PNG file format itself, not for libpng. The libpng manual says:

Support for the sCAL, iCCP, iTXt, and sPLT chunks was added at libpng-1.0.6;
however, iTXt support was not enabled by default.
...
Support for the iTXt chunk has been enabled by default as of
version 1.2.41.
...
    Note that the itxt_length, lang, and lang_key
    members of the text_ptr structure only exist when the
    library is built with iTXt chunk support.  Prior to
    libpng-1.4.0 the library was built by default without
    iTXt support. Also note that when iTXt is supported,
    they contain NULL pointers when the "compression"
    field contains PNG_TEXT_COMPRESSION_NONE or
    PNG_TEXT_COMPRESSION_zTXt.

pngconf.h from libpng 1.2.59 [source tarball] says:

/* The size of the png_text structure changed in libpng-1.0.6 when
 * iTXt support was added.  iTXt support was turned off by default through
 * libpng-1.2.x, to support old apps that malloc the png_text structure
 * instead of calling png_set_text() and letting libpng malloc it.  It
 * will be turned on by default in libpng-1.4.0.
 */

#if defined(PNG_1_0_X) || defined (PNG_1_2_X)
#  ifndef PNG_NO_iTXt_SUPPORTED
#    define PNG_NO_iTXt_SUPPORTED
#  endif
#  ifndef PNG_NO_READ_iTXt
#    define PNG_NO_READ_iTXt
#  endif
#  ifndef PNG_NO_WRITE_iTXt
#    define PNG_NO_WRITE_iTXt
#  endif
#endif

@Deledrius
Copy link
Member

Oh, I was looking in the wrong place the whole time. 😳

text[num_txtfields].lang = "en-us"; // Language used in 'text' and 'lang_key'.
text[num_txtfields].lang_key = ""; // Translation of 'key' into 'lang', if needed.
text[num_txtfields].compression = PNG_ITXT_COMPRESSION_NONE;
#else
it->second = it->second.to_latin_1();
Copy link
Member

Choose a reason for hiding this comment

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

This can throw, since ST::string expects to hold UTF-8 data. Instead, the char_buffer should be stored off separately, and should point to either the UTF-8 data or Latin-1 data to be written by libpng.

@AbigailBuccaneer
Copy link
Contributor Author

@zrax LGTY?

@Deledrius Deledrius merged commit bcfa778 into H-uru:master May 25, 2018
@Deledrius
Copy link
Member

Looks great. 👍

@AbigailBuccaneer AbigailBuccaneer deleted the png-no-itxt branch May 25, 2018 07:40
Hoikas added a commit that referenced this pull request May 26, 2018
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.

None yet

3 participants