[Krita] Disable the brush size/opacity shortcuts when the respective sliders are disabled
ClosedPublic

Authored by nicholasl on Dec 10 2017, 12:41 PM.

Details

Summary

This patch is for bug 379958.
In its current state, it only fixes the bug for the brush size shortcuts.

I'm interested in fixing the opacity shortcuts as well, but I'm unsure of how to do so.
I've thought of three ways, all of which modify KisCanvasControlsManager::stepAlpha():

  • Don't change the opacity when the active tool's flags() have the KisTool::FLAG_USES_CUSTOM_COMPOSITEOP bit cleared. This is how the opacity slider is disabled by KisPaintopBox::slotToolChanged(), but I question it being the right way to go since "USES_CUSTOM_COMPOSITEOP" may not strictly mean "has adjustable opacity."
  • Store the value given to KisPaintopBox::setWidgetState(int flags) (flags here is not the the same as KisTool::flags()) and add a function to get the stored value. Don't change the opacity if the DISABLE_OPACITY bit was set. If I go this route, I wonder if there's any chance that the value will ever be out of sync with the actual state of the widgets.
  • Add a function to KisPaintopBox that returns a flags value based on the current isEnabled() state of all the widgets that are set by KisPaintopBox::setWidgetState(). This introduces extra maintenance since the new function should be updated any time setWidgetState() is modified.

The final issue I mentioned in the bug report was regarding shift+drag modifying the brush size when the slider was disabled.
I mistakenly linked to the wrong bug, which should have been 379564 .
Before that bug was fixed, you could switch to the smart patch tool (which disabled the size slider), switch to the freehand tool, and then be able to shift+drag to change the size even though the slider was still disabled.
Tools appear to get shift+drag functionality when they inherit from KisToolFreehand, which, according to Doxygen, would be the "Freehand Brush Tool", "Multibrush Tool", "Dynamic Brush Tool", and the "Colorize Mask Editing Tool".
All those have an enabled size slider, so I still think it's not much of an issue at the moment.

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.
nicholasl created this revision.Dec 10 2017, 12:41 PM
Restricted Application added a subscriber: woltherav. · View Herald TranscriptDec 10 2017, 12:41 PM
nicholasl requested review of this revision.Dec 10 2017, 12:41 PM
dkazakov accepted this revision.Dec 11 2017, 1:36 PM
dkazakov added a subscriber: dkazakov.

Hi, @nicholasl!

The patch looks fine, please push!

Regarding your questions:

I've thought of three ways, all of which modify KisCanvasControlsManager::stepAlpha():

I guess the best option would be to connect the KisCanvasControlsManager to the toolChanged(id) signal and enable/disable the actions for every tool depending on its flags. It will be the clearest way to do that.

I question it being the right way to go since "USES_CUSTOM_COMPOSITEOP" may not strictly mean "has adjustable opacity."

I guess you can just add one more flag for tools and use it.

The final issue I mentioned in the bug <...>

Speaking truly I cannot fully understand what is the issue with Shift+Gesture. Right now it is implemented only for the tools inheriting KisToolFreehand. If needed, I guess it is okay to move this feature to KisToolPaint (or something else) and check the flag of tool. This way it would eliminate a small code duplication among KisToolTransform::LiquifyTransformStrategy and the freehand tool. Though I'm not sure if it is worth the effort, because it might introduce some bugs with the pseudo-link between brush breset and the liquify tool (the latter onedoesn't use the presets).

This revision is now accepted and ready to land.Dec 11 2017, 1:36 PM
This revision was automatically updated to reflect the committed changes.