Add initial support for the QOI image format

[laurelkeys]

This PR adds support for the new "Quite OK Image" format (qoi), targeting the new cpp20 branch.

[Tom94]

This looks perfect. I'm holding off with merging purely because QOI hasn't been finalized yet.

[laurelkeys]

This looks perfect. I'm holding off with merging purely because QOI hasn't been finalized yet.

Definitely! I'll wait for https://github.com/phoboslab/qoi/issues/37 to be closed before making further updates to the PR.

N.B. besides loading PNG images and saving them to QOI (and then loading back the result) to check if the code was working correctly, I have also tested:

• Loading QOI images exported from tev into qoiview;
• Loading the four test images from qoiview/images/ into tev.

[laurelkeys]

It looks like the format is now finalized (though, the issue still hasn't been closed). However, one change I noticed which might make it weird to support the format is regarding the added colorspace information:

// The colorspace in this qoi_desc is a bitmap with 0000rgba where a 0-bit 
// indicates sRGB and a 1-bit indicates linear colorspace for each channel. You 
// may use one of the predefined constants: QOI_SRGB, QOI_SRGB_LINEAR_ALPHA or 
// QOI_LINEAR. The colorspace is purely informative. It will be saved to the
// file header, but does not affect en-/decoding in any way.

#define QOI_SRGB 0x00
#define QOI_SRGB_LINEAR_ALPHA 0x01
#define QOI_LINEAR 0x0f

From what I understand, QOI_SRGB would mean a gamma encoded alpha channel, which I'm not sure would make sense (as opposed to the usual "gamma encoded RGB + linear alpha", represented by QOI_SRGB_LINEAR_ALPHA).

My understanding is that, internally, tev's images use "sRGB/Rec.709" primaries and have linear color channels (as a result of the toLinear() calls in *Loader.cpp files).

Hence, should we always encode QOI files as QOI_LINEAR when saving them? In my opinion the only other reasonable option would be to use QOI_SRGB_LINEAR_ALPHA, as handling per-channel linear/non-linear information would require changes to Image and ImageData (and this does not seem worth it only to support this format, specially at such an early stage).

Also, I'm not sure if storing the vector<char>& data as QOI_SRGB_LINEAR_ALPHA would require using toSRGB() in the saver (?)

[Tom94]

From what I understand, QOI_SRGB would mean a gamma encoded alpha channel, which I'm not sure would make sense (as opposed to the usual "gamma encoded RGB + linear alpha", represented by QOI_SRGB_LINEAR_ALPHA).

My understanding is that, internally, tev's images use "sRGB/Rec.709" primaries and have linear color channels (as a result of the toLinear() calls in *Loader.cpp files).

This is 100% correct!

Hence, should we always encode QOI files as QOI_LINEAR when saving them? In my opinion the only other reasonable option would be to use QOI_SRGB_LINEAR_ALPHA, as handling per-channel linear/non-linear information would require changes to Image and ImageData (and this does not seem worth it only to support this format, specially at such an early stage).

LDR image savers are already passed QOI_SRGB_LINEAR_ALPHA data by tev, so you can call the qoi_encode function without transforming any channels. See ImageCanvas::saveImage() , which calls ImageCanvas::getLdrImageData() that does the conversion. I totally agree that QOI_SRGB_LINEAR_ALPHA is the right format to use for maximum compatibility.

Thanks again!

[laurelkeys]

It looks like the format has been finalized (e.g. see this comment from the author https://github.com/floooh/qoiview/pull/2#issue-1065431602). Though, as mentioned in my last comment, QOI_SRGB doesn't make much sense compared to QOI_SRGB_LINEAR_ALPHA, so it's really unfortunate that it is the default 0x00 value.

Since it's noted in qoi.h that the colorspace information is "purely informative", I think tev can assume the zero case to mean QOI_SRGB_LINEAR_ALPHA. What do you think?

[Tom94]

My understanding is that "purely informative" merely refers to the fact that the qoi encoding routine doesn't care about the meaning of the channel.

As such, tev should treat the bitfield exactly as the spec says: if the bit is high, the channel should be treated as linear, and if the bit is low, srgb.

If tev (and other image viewers) started to ignore the spec, then all hell would break loose in terms of compatibility. :p

[laurelkeys]

As such, tev should treat the bitfield exactly as the spec says: if the bit is high, the channel should be treated as linear, and if the bit is low, srgb.

Ok! I'm just worried about images looking slightly wrong because initializing qoi_desc to zero leaves colorspace == QOI_SRGB, making it really easy to incorrectly represent an image which should actually be QOI_SRGB_LINEAR_ALPHA.

Should tev display any sort of "warnings" about this to notify users? If not, I think changing the code as you suggested (https://github.com/Tom94/tev/pull/148#discussion_r758315916) seems good to me.

[Tom94]

I think it's best to wait and see how much of an issue this becomes in the wild before emitting warnings.

Your concern is very real and valid, though... perhaps at some point in the future it will indeed make sense to ignore the alpha bit and just assume linear.

[laurelkeys]

Just made the change mentioned above and, funnily enough, already came across the linear vs. sRGB OETF encoded alpha problem using qoiview/images/ for testing.

The first image from Wikipedia's entry on PNG is used as dice.qoi, and it has been encoded with colorspace == QOI_SRGB.

Here's how it looks compared to the original .png file (downloaded directly from Wikipedia):

And here is how it looked before (i.e. assuming QOI_SRGB_LINEAR_ALPHA when colorspace == 0x00 as well):

Also, it seems like there might still be some changes to the format encoding (see https://github.com/nigeltao/qoi2-bikeshed/issues/14#issuecomment-981576429).

Thus, I think it might be best to give it a few more days before assuming the QOI format is finalized and merging this PR.

[Tom94]

Oh boy. Maybe worth opening an issue over at qoiview if they're incorrectly encoding their sample images.

I don't want to steal your credit -- but let me know if you don't want to. Then I'll go ahead.

[laurelkeys]

Oh boy. Maybe worth opening an issue over at qoiview if they're incorrectly encoding their sample images.

I don't want to steal your credit -- but let me know if you don't want to. Then I'll go ahead.

No worries! Feel free to open the issue, I think you are better suited to explain why they are incorrect (and maybe influence in modifications to the meaning of the default value, if the format still suffers any changes).

Here's the three images side-by-side for comparison (PNG from Wikipedia, QOI following the QOI_SRGB hint currently present in the file, QOI assuming QOI_SRGB_LINEAR_ALPHA for the 0x00 case instead):

We can clearly see that using QOI_SRGB is not what was originally intended.

[Tom94]

Actually, this may well be the other way around. QOI_SRGB looks correct to me, whereas the other two images have a visible halo. Perhaps tev is interpreting the alpha channel of PNG images wrong?

I will investigate further before making a ruckus.

Thank you for creating that side-by-side!

[laurelkeys]

QOI_SRGB looks correct to me, whereas the other two images have a visible halo.

I noticed this as well (especially in the green dice), but it seems to be a weird "artifact" from the original image itself: https://en.wikipedia.org/wiki/Portable_Network_Graphics#/media/File:PNG_transparency_demonstration_1.png (at least it seems so to me, looking at the image on Chrome). Though, it's worth it to make sure anyway.

[Tom94]

Digging a little deeper, the PNG image spec seems to mandate linear alpha. See section 2.7 of http://www.libpng.org/pub/png/spec/1.2/png-1.2-pdg.html#DR.Alpha-channel

Gamma correction is not applied to the alpha channel, if any. Alpha samples always represent a linear fraction of full opacity.

Also, see https://www.w3.org/TR/2003/REC-PNG-20031110/#4Concepts.Implied-alpha section section 12.2

Gamma does not apply to alpha samples; alpha is always represented linearly.

Now I'm starting to doubt the correctness of the PNG image on Wikipedia. Argh.

[laurelkeys]

This discussion reminds me of PNG + sRGB + cutout/decal AA = problematic and A PNG Puzzle. Although I'd have to read them again, here's an interesting bit that maybe stands out:

It turns out that PNG says that you need to unmultiply before converting to sRGB. This goes against theory, in that you normally take a premultiplied result and convert that to sRGB for display (composited against a black background). But it turns out that the proper sequence for PNG conversion is to un-premultiply and then convert to sRGB.

N.B. those posts are from 2016.

[Tom94]

\ I am very biased towards premultiplied alpha for exactly that reason! Any image format that uses un-premultiplied alpha is doing it wrong! :)

Imagine representing a candle flame as an image. It should have a yellow-ish color (near 0xFFFFFF, but not quite there), yet should be almost fully transparent (alpha near zero). In premultiplied alpha form, this is very easy to do.

But without premultiplied alpha, the flame needs exceedingly large pixel values (which are impossible to represent in 8bit), just so the multiply by alpha doesn't result in a near-black color. \

[laurelkeys]

I updated the qoi submodule so that it now uses the final specification and have tested it with the updated images from https://github.com/floooh/qoiview/pull/4 which are properly loaded, matching the original PNG file.

Also, a separate .c file to #define QOI_IMPLEMENTATION is no longer necessary, as it can now be compiled as C++20 code.

[Tom94]

Amazing, thank you so much! I ended up cleaning up a little (adding credits and rebasing), but accidentally force-pushed directly to the cpp20 branch rather than here. This ended up irrevocably closing this PR, my bad 😅, although this shouldn't make any difference to the final git history.

Your commits are all in and I'll push a new release soonish. :)