Skip to content

app: make dynamics tool options a checkbox

Asa requested to merge Asalle/gimp:asalle/use-dynamics-flag into master

Hi all, this is my proposal on how to fix #4333 (closed). In short: I moved the dynamics options UI elements into a gimp_prop_expanding_frame_new, and added an if statement in app/paint/brushcore.c:gimp_brush_core_start that checks the current state of the checkbox.

If the checkbox is unchecked: dynamics falls back to "Dynamics Off", the current dynamics name and its options are hidden in the UI.

If the checkbox is checked: dynamics is set to previously used one or the default one, all dynamics options are seen in the UI.

It works, but I have a couple of questions that I need some help with:

  1. I added PROP_DYNAMICS_ENABLED, that is a bit redundant, because of the existing PROP_DYNAMICS_EXPANDED. The _ENABLED in my mind should be responsible for both expansion of the UI menu and enabling the dynamics. So, logically I would remove the _EXPANDED and substitute it with the new prop. However, it turned out that many other classes use that prop and it will cause a huge renaming... Not sure what to do here, both options are a bit messy.

  2. Is there any way to autoformat the code? Something like clang-format. I couldn't find anything :c

  3. After talking to @Jehan on irc, he advised to move the DYNAMICS_ENABLED flag to GimpContext instead of GimpPaintOptions, but following the "Apply Jitter" UI menu logic, I think it's better to leave it in PaintOptions and retrieve it with getter. WDYT?

  4. The newly added if in @@ -365,7 +365,12 @@ gimp_brush_core_start look a bit hacky, do you know any better way to set the dynamics?

Here is the new way dynamics tool options look minimized and expanded (expanded by default):

Screenshot_from_2022-01-02_11-46-10 Screenshot_from_2022-01-02_11-46-19

Edited by Asa

Merge request reports