This is a fix for Bug 388247 (https://bugs.kde.org/show_bug.cgi?id=388247)
This revision changes how the markers and numbers on the ruler (the ruler on the peripheral of the canvas, not the assistant ruler) are drawn so that the ruler changes its precision based on the zoom level of the canvas.
Details
- Reviewers
rempt - Group Reviewers
Krita - Commits
- R37:ba60778bd550: Change ruler subdivision based on zoom level
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.
Please refrain from stating only the bug number in the title, because it makes the title unmeaningful to anybody.
Nice! This works perfectly. Can you push it yourself or don't you've got commit access yet?
I do have some very minor nitpicks. Check my inline comments.
libs/widgets/KoRuler.cpp | ||
---|---|---|
281 | Minor nitpick: I would prefer to put the numberSeparation = minimalNumberSeparation assignment under this conditional block rather than having an empty block here. | |
480 | Ditto. Also, this looks like to be the same code as my other inline comment at my first glance. Can they be refactored into a function to reduce duplicate code? |
On the first matter, I'm doing that because I don't like declaration without initialization. I'd say it's just a matter of style: some people like it, others don't, but it doesn't really matter.
On the second matter, actually a ton of refactoring can be done to this class. If I'm going to refactor it, I'd rather do it to the whole class. Considering this class generally is working well and it's not likely to be changed frequently, I'd rather use the time on other bugs. I'll be glad to accept the task of refactoring this class when the next phase for large-scale refactoring comes.
Just a comment:
I have a feeling that there is no need to show subdivisions for half- and quarter- pixels. The point is a bit disputable, but it might be a nice topic for future fixes.
libs/widgets/KoRuler.cpp | ||
---|---|---|
281 | I would just add a // noop comment :) Anyway, it doesn't matter since the patch is already pushed :) |