Enable Undo/Redo for Granularity Values
ClosedPublic

Authored by hellozee on Jan 21 2019, 1:07 PM.

Diff Detail

Repository
R37 Krita
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
hellozee requested review of this revision.Jan 21 2019, 1:07 PM
hellozee created this revision.
dkazakov requested changes to this revision.Wed, Jan 23, 7:55 AM

Hi, @hellozee!

There is a problem inherited from the previous patch: copy-constructor and assignment operator do not copy the values for the precision. Therefore undo/redo will not work properly. Please add copying to ToolTransformArgs::init() function.

I think the idea of the general requirements for config saving/reusing should look like that:

  1. The current precision values are stored in ToolTransformArgs. The args also track history of changes of this value (undo points are shared with the current undo point).
  2. The default precision values are saved in config (not implemented yet, postponed till later)
  3. Default values (until point2), just 8 and 16) should be loaded into the initial transform args object on tool activation (when the action is not "continued").

Test plan:

  1. Create cage
  2. Move some points without finishing it
  3. Change precision values
  4. Press Ctrl+Z, both steps 2) and 3) should be undone
  5. Move some points, change precision of the final composition, press Enter

// the image will become transformed

  1. Press on the canvas with the transform tool
  2. All the points and the precision value selected on step 5) should be recovered.
This revision now requires changes to proceed.Wed, Jan 23, 7:55 AM
hellozee updated this revision to Diff 50484.Tue, Jan 29, 3:55 PM

Sadly it still doesn't work, need to poke around a little more I guess

hellozee retitled this revision from Saving the pixel precision and preview pixel precision to xml to Enable Undo/Redo for Granularity Values.Tue, Jan 29, 3:56 PM
hellozee updated this revision to Diff 50587.Thu, Jan 31, 9:19 AM

Added a test, it doesn't fail, I am probably looking at the wrong place for the missing key

hellozee updated this revision to Diff 50591.Thu, Jan 31, 11:41 AM

Yay it works now

dkazakov accepted this revision.Fri, Feb 1, 2:08 PM

Hi, @hellozee!

Congratulations! Your patch seem to work perfectly fine now! Please push :)

This revision is now accepted and ready to land.Fri, Feb 1, 2:08 PM
This revision was automatically updated to reflect the committed changes.