Skip to content

454116 454241 TIFF: refactor, cleanup, and enable YCbCr + JPEG

Amy spark requested to merge lsegovia/krita:work/amyspark/tiff-2 into master

This PR started as a fix to enable correct support of YCbCr and JPEG compression. But I quickly realised we would be better served with a complete overhaul of the plugin.

For this reason, what I'm sending here is an extensive refactor and a comprehensive set of fixes, which I've taken pains to lay out in separate commits.

These cover:

  • Removal of manual management of TIFF strips and file handles
  • Removal of manual management of TIFF reader and buffers
  • Fixed leak of line size coefficients and channel indexes
  • Fixed incorrect writing of metadata on export
  • Fixed incorrect handling of compression options
    • alpha (YCbCr / CMYK), JPEG with U8, etc.
  • Fixed range detection and support for round-trip YCbCr conversions
  • Separation of concerns between the importer, exporter, and common components
    • aka: I removed the converter class, put each bit where it belongs, and factored the common PSD bits into a standalone shared library

Of course, this PR finally enables import and export of YCbCr TIFFs with (and without) JPEG compression, with the relevant options being enabled in the export dialog. This was achieved with the libjpeg-turbo API, along with a simple extension to enable loading the encoding tables from the TIFF file. This has also been sent upstream: https://github.com/libjpeg-turbo/libjpeg-turbo/pull/605. The only supported export subsampling is 4:4:4, because it's what we can load without libjpeg-turbo; for more information, see https://www.asmail.be/msg0054686955.html and https://www.asmail.be/msg0054881320.html.

This refactor makes use of STL containers and managed pointers, because not only they were far more easily accessible at debug time, they showed to be extremely reliable to catch existing crashes and instances of data corruption. It also makes use of clang-tidy (thanks @dragonmux!) to catch all unnecessary conversions. These affected especially the buffer management.

Finally, this PR also enables @woltherav's tests and integrates the lonely quad-jpeg.tif file that was outside the sources/ folder.

Test Plan

Build Krita and play with exporting YCbCr images. Check that YCbCr can be exported with JPEG only if the depth is 8-bit integer. Check that all the exports can be imported again successfully.

Formalities Checklist

  • I confirmed this builds.
  • I confirmed Krita ran and the relevant functions work.
  • I tested the relevant unit tests and can confirm they are not broken. (If not possible, don't hesitate to ask for help!)
  • I made sure my commits build individually and have good descriptions as per KDE guidelines.
  • I made sure my code conforms to the standards set in the HACKING file.
  • I can confirm the code is licensed and attributed appropriately, and that unattributed code is mine, as per KDE Licensing Policy.

Merge request reports