Add an option to draw ruler tips as a power of 2
ClosedPublic
Actions

Authored by zachmann on Oct 10 2017, 2:52 PM.

Details

Reviewers

Group Reviewers

Commits

R37:21579ff8c9e6: Add an option to draw ruler tips as a power of 2
R37:2a1329ec20a3: Add an option to draw ruler tips as a power of 2

Summary

Added option to show pixel rules with a width of 64 pixel.
The config setting is saved to the document.

This implements the feature described in

https://phabricator.kde.org/T4874

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.

zachmann created this revision.Oct 10 2017, 2:52 PM

Hi, @zachmann!

I have tested your patch and there seems to be a few bugs/features (I don't understand :) ):

When enabling "Multiple of 2" mode, I cannot see the logic, which units use it, and which not... As far as I can see:
- px, pt, cm, mm --- use multiples of two
- pi, cc, in --- don't use the multiple

As far as I can guess, only px should use this multiplier...

When saving/reloading the image, the option is not activated. I don't know why. I can see the code doing some XML saving in the patch, but it seems it fails to pass this value to the GUI

If we save "use multiple of 2" value to .kra file, I guess we should also save the choice of the unit... It was not in the original task, but I guess it would be logical.

Here is a video recording showing the issue (when I close the file the canvas is not updated, it is a known bug of my video driver):

krita_rulers_bug.mkv4 MBDownload

This revision now requires changes to proceed.Oct 11 2017, 8:47 PM

I have tested your patch and there seems to be a few bugs/features (I don't understand :) ):

When enabling "Multiple of 2" mode, I cannot see the logic, which units use it, and which not... As far as I can see:

px, pt, cm, mm --- use multiples of two

pi, cc, in --- don't use the multiple

As far as I can guess, only px should use this multiplier...

The feature is only there for pixel. The other units don't use it. It is just that the show the values but that they do also without t

When saving/reloading the image, the option is not activated. I don't know why. I can see the code doing some XML saving in the patch, but it seems it fails to pass this value to the GUI

The problem was that the stuff was only saved when guides where used.

If we save "use multiple of 2" value to .kra file, I guess we should also save the choice of the unit... It was not in the original task, but I guess it would be logical.

This is the question. The show rulers seems to be saved in the krita config and not saved int document as the multiple of two. Should this and the unit saved in the document?

Here is a video recording showing the issue (when I close the file the canvas is not updated, it is a known bug of my video driver):

krita_rulers_bug.mkv4 MBDownload

Save the guides and the multiple of two when there are changes to the config. This also fixes the problem that the Show guides, Snap to guides, Lock guides was only saved when the there was a guide in the document.

In D8237#156532, @zachmann wrote:

When enabling "Multiple of 2" mode, I cannot see the logic, which units use it, and which not... As far as I can see:

px, pt, cm, mm --- use multiples of two

pi, cc, in --- don't use the multiple

The feature is only there for pixel. The other units don't use it. It is just that the show the values but that they do also without t

It sounds like a bug :(

If we save "use multiple of 2" value to .kra file, I guess we should also save the choice of the unit... It was not in the original task, but I guess it would be logical.

This is the question. The show rulers seems to be saved in the krita config and not saved int document as the multiple of two. Should this and the unit saved in the document?

I guess both the options (unit and use-power-of-two) should be saved into .kra. If you really like it, you can make KisConfig store the last manually switched by the user value and use it as the default for newly cretaed documents. That would be the best option from usability point of view. Right now, power-of-two option seem to be saved/loaded correctly (tested), but the units not (obvoiusly)

There is still one blocking bug with the power-of-two option:

Create a document, make sure that power-of-two options is switched off
Create the second document and enable power-of-two option
Press Ctrl+W to close the second document

Now you see the first document, with the rulers' units rendered as if the power-of-two option is enabled, although it is not enabled.

This revision now requires changes to proceed.Oct 23 2017, 7:39 AM

Fixed updating on switching different images.
Saved unit to the kra file.

Hi, @zachmann!

The patch looks and works perfectly fine now! Please push!

And thank you very much for your work! :)

This revision is now accepted and ready to land.Dec 11 2017, 10:14 AM

Closed by commit R37:21579ff8c9e6: Add an option to draw ruler tips as a power of 2 (authored by zachmann). · Explain WhyDec 13 2017, 4:28 AM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			krita/krita.action (12 lines)
M			libs/ui/KisViewManager.cpp (1 line)
M			libs/ui/canvas/kis_guides_config.h (9 lines)
M			libs/ui/canvas/kis_guides_config.cpp (52 lines)
M			libs/ui/canvas/kis_guides_manager.h (6 lines)
M			libs/ui/canvas/kis_guides_manager.cpp (27 lines)
M			libs/ui/kis_zoom_manager.h (1 line)
M			libs/ui/kis_zoom_manager.cc (21 lines)
M			libs/widgets/KoRuler.h (2 lines)
M			libs/widgets/KoRuler.cpp (15 lines)
M			libs/widgets/KoRuler_p.h (2 lines)
M			plugins/impex/libkra/kis_kra_saver.cpp (2 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	20567			Oct 10 2017, 2:49 PM	★	★
Diff 2	20910			Oct 17 2017, 4:19 PM	★	★
Diff 3	23622			Dec 7 2017, 5:56 PM	★	★
Diff 4	23841	b0dbfeb	R37:21579ff8c9e6e36d8562df5467e6be1902f2fea9	Dec 13 2017, 4:26 AM	★	★