DavidKinder / Windows-Frotz

Z-code interpreter for Windows, based on Stefan Jokisch's Frotz interpreter core.
http://www.davidkinder.co.uk/frotz.html
GNU General Public License v2.0
58 stars 12 forks source link

Adaptive palettes are not applied with 8-bit PNG images #34

Closed cspiegel closed 1 week ago

cspiegel commented 3 weeks ago

The code in Win/FrotzGfx.cpp does this:

  if (m_adaptiveMode && color_type == PNG_COLOR_TYPE_PALETTE && bit_depth <= 4)

to handle adaptive palette images. But the Blorb spec says this about APal:

However, the following rules still apply from the PNG standard:

  • Any bit depth of PNG is valid (1, 2, 4, or 8 bits per pixel).

I think that the bit_depth check shouldn't be happening... but I'm not sure if there are consequences to it.

I'm attaching a Zork Zero Blorb that has some 8-bit images; you can see how the adaptive palette is not being applied to the compass rose, for example.

zork0.zip

dfabulich commented 3 weeks ago

See also https://gitlab.com/DavidGriffith/frotz/-/issues/288

For background, one of my goals is to build a single-file blorb containing the best known Infocom story file and graphics, and to link to that file from IFDB.

As it stands, IFDB currently links to two files (the Z6 file on Zarf's site and the graphics blorb on IF Archive), and we have to link to IFWiki to explain how to use them. It's all very finicky and complicated, and it would all be much easier if there were a single canonical blorb to link to that would Just Work in a variety of interpreters.

DavidKinder commented 2 weeks ago

This line

  if (m_adaptiveMode && color_type == PNG_COLOR_TYPE_PALETTE && bit_depth <= 4)

comes from this pull request, originally from Stefan Jokisch. Unfortunately that means I don't know exactly why it was written like that. Looking at the code slightly below that line, the only issue I can see is where png_read_image() is called. The way that that is written will require the bit depth to be no more than 8, because the decoded data has only one byte per pixel. Changing that test from 4 to 8 should be okay. I will do some experiments.

DavidKinder commented 1 week ago

Increasing the allowed bit depth to 8 makes the attached Blorb file work, and doesn't appear to cause any problems. Increasing it any further, or removing the check entirely would require further modification to the code, though.

Anyway, I'll consider this fixed by commit 1a86a07a845ba68a32a499fb3f3f80784fb8a7cb.