Draft: PNG: refactor, modernize, upgrade, optimize, and implement missing features
This MR is the result of the efforts during the last days of 2022. A friend sent me some months ago a couple PNG samplers intended to stress test our memory management and I/O performance in the PNG format. We failed
As a result, I dived into the plugin and reworked it as much as I could, while bringing it to the C++14 standard, and also implementing missing features.
Features
New features
- Implemented progress reporting for both reading and writing
- Factored the converter class out of kritaui, like it was done for TIFF-PSD and psdutils, for ease of use and better separation of concerns
⚠ Breaking changes
With this MR, we would now follow the official PNG standard and Exiv2 behaviour for the metadata storage.
- The plugin no longer relies in the undocumented base-16, 72 characters per line encoding of Cyrille Berger. I found quite a few unheeded calls for help about this online:
- Metadata chunks are now saved and loaded in cleartext
- Exif data is now saved in its official chunk (s. 11.3.4.5)
- If not supported by the underlying libpng version, a compressed cleartext chunk of
Raw profile type exif
is used (see Exiv2) - When reading, the
eXIf
chunk will be preferred over the legacy chunk, if both are present
- If not supported by the underlying libpng version, a compressed cleartext chunk of
- XMP data is now saved as an uncompressed text chunk with the official key,
XMP:com.apple.xmp
(s. 11.3.3.1)- When reading, the text chunk with the
XMP:com.apple.xmp
key will be preferred over the legacy chunk, if both are present
- When reading, the text chunk with the
- IPTC data is now saved as a compressed cleartext chunk of
Raw profile text iptc
(see again Exiv2)
Example output |
---|
Performance improvements
- The image is now loaded at once into a temporary buffer
- All transformations (alpha trimming or insertion, palette expansion, channel swapping, and bit depth reduction) are now passed over to libpng
- This lets libpng handle them as part of the image compression and writing
- Krita only handles export of palette images (creation and application of the palette).
- After this change, for the rest of the types, loading an image into Krita space is as simple as blasting the rows into the tiles (see !1694 (merged) for the demonstration!)
- Applied the HEIF/JXL template pattern to remove all conditionals (bit depth and transform-- the rest were removed in the first item above) from the hot path
- Optimized metadata accessing iterators
- Optimized creation and filling of the ICC profile and row structures
- Replaced manual memory handling in favour of
unique_ptr
andstd::vector
- This help the compiler vectorize the code by avoiding
detach()
whenever something is accessed, and skipping container initialization shaves off as much as 11% of the image reading time
- This help the compiler vectorize the code by avoiding
- Applied clang-tidy throughout the code in separate commits, for ease of rev-skipping
Bug fixes
- Potential crash when the palette overflowed and we didn't reset the color type appropriately
- Fixed incorrect return value when allocating the buffer fails due to memory pressure
Results
With a 15k x 7k image, load times went from hanging Krita (without !1694 (merged)) to 5s (post clang-tidy, pre tiling) to 2.2s (post tiling and transform, with individually managed rows) to 1.9s (with unique_ptr
ing a single chunk of memory).
Some examples below:
Tiling | After transform | Fixing allocations |
---|---|---|
Test Plan
Build Krita and test PNG importing and exporting. For the full performance, merge !1694 (merged) into the branch prior to testing.
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.