Skip to content

412720 441605 Fix loading Photoshop files with layer-local resources

Amy spark requested to merge lsegovia/krita:work/amyspark/fix-psd-loading-master into master

This MR fixes a longstanding issue with Photoshop gradients, where those that are defined locally to a layer would cause a safe assert on load:

SAFE ASSERT (krita): "!localResources.contains(KoResourceSP())" in file /home/tymon/kritadev/krita/libs/resources/KisLocalStrokeResources.cpp, line 78

In my initial triage, I realised the first issue on master was that layer styles were silently dropped due to the PSD loader never marking them as valid. That was fixed in 6039f5b4.

However, the rabbit hole didn't end up there. The second issue was that PSD layer styles rely on a custom serialization to calculate their MD5, as written by @tymond in f671aa5e, and this behaviour was silently dropped by @rempt in 90072c9a in favour of saveToDevice, an API that is unimplemented in KisPSDLayerStyle.

With those two issues fixed, I could finally figure out what was causing the safe assert. In 2d426ac0, @dkazakov refactored the load so that layer styles would insert their resources on global storage instead of keeping them in their local stashes. However, this did not take in account that layers can also have locally defined e.g. gradients. These will not be found if the resource interface is overriden because they are resident on the root layer's localStrokeStorage.

Upon a complete review of the code, the true reason for this failure lied not only in the (in)validity of the layer style, which as said before was fixed in 6039f5b4, but also on the fact that gradients and patterns are uploaded before they are read (!!), in the embedded pattern registration loop.

This MR fixes the two outstanding issues described above by:

  • reimplementing saveToDevice() to follow @tymond's way of calculating MD5
  • moving the two upload loops to inside the layer style deserialization, and making sure they upload only missing resources (to avoid accidentally quadratic behaviour).

Additionally, I fixed two typos from @dkazakov I found during review of the afflicted code.

BUG: 412720

BUG: 441605

Test Plan

Build Krita and test loading this file: https://dribbble.com/shots/1129892-King-Free-Photoshop-Layer-Style .

No assertions or errors should be logged.

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