Skip to content

Fix malfunction of x/y position spin boxes in move tool's options

Mathias Wein requested to merge mwein/krita:move-tool-bug-423452 into master

This should fix the issue that the move tool's spinboxes in the options widget only work on the first use of the tool (bug 420329 and 423452, which look like duplicates to me)

That issue was merely disconnecting signals on every deactivate() that only got connected in the constructor, so I moved them to activate().

But as usual, testing the fix also uncovered a more tricky one, if you don't click on the canvas first and increase/decrease the x or y spinbox value by 1, you end up with 2*old_value +/-1. The reason is that starting a new stroke wipes m_handlesRect, which is required to transform the absolute value from the UI to relative values for the stroke.

I'm not sure if this is a good solution, but since the UI values were calculated with m_handlesRect before starting the stroke too, it seems okay to read it before they get wiped. There is theoretically a short period where they are unavailable until the stroke's init job recalculated them, but in practice this seems so quick that I couldn't trigger this case at all, in the worst case UI input is ignored for that short moment, still better than doubling its current position. But maybe someone has a more robust solution...if this tool somehow models a well defined state-machine, I'm obviously too confused to grasp it.

Btw. I saw there's a failing MoveStrokeTest bug, but I can't really figure out what this test tries to achieve in the first place...

Test Plan

Part 1 (second issue):

  1. Open/create a document with a layer or selection whose top-left bound is not (0,0)
  2. Activate the move tool on this layer, do NOT click on the canvas
  3. increase/decrease the x or y Position in the Tool Options docker by one and make sure the layer/selection actually moves by 1 pixel and not some huge amount

Part 2 (the actually reported issue):

  1. End the stroke e.g. by switching to the Brush tool
  2. Switch back to the Move tool
  3. make sure the spin boxes still work

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