Preset widgets refactoring

Stefano Bonicatti smjert at gmail.com
Sun Aug 30 15:08:33 UTC 2015


So here i'm again, time to think "out loud" again (as if i don't do this
often enough!).

More than a month passed (almost two!) and i'm slowly trying, especially
due to the new work, to do the refactoring, right now i almost removed all
the code which KisPaintOpBox shouldn't execute and moved it to a new
KisPresetManager class, though i'm facing several issues that i cannot
solve properly with the current state of the code.

Unfortunately things are much more intertwined than i thought initially and
basically all the GUI classes that deal with presets do "too much things",
step beyond their logic boundaries and complicate everything.
So either i keep running around trying to fight against these to maybe then
find a way to make things work, but that it won't last, or i go back to the
drawing board and actually start to thinking to do a bigger refactoring.

Given that i don't have in depth knowledge of every class involved i just
tried to retain the same functionality and order of calls that
KisPaintOpBox had, while trying to unravel the mess with preset events and
such.
Though this doesn't work, i see a lot of duplicated code, code that is too
reused and then leads to have calls that are not needed, and it's not
(only) a performance problem, but they do something they shouldn't always
do, leading to the issues we have now.

What's giving me issues for instance is KisPaintOpSettingsWidget who is
supposed to signal stuff when its configuration changes, but those are most
of the time blocked (and i don't find exactly a logic about that, if one
has to always block the signals except in rare cases, better not
use signals at all then... or use different code paths for different
operations, one that launches them and one that doesn't, so that they "self
document" themselves instead of always relying on someone else knowing when
to block and when not).
Also here i see setConfiguration called for cases where, probably, a more
precise call (to change only one value) would be needed.

Other things are compositeOps, when a preset is set, an "update composite
op" procedure has to be called, though the things that have to be done are
slightly different than in the case where only the composite op id is
changed, so the code written before cannot really be reused.
At the same time, given that one thinks such a small change should lead to
few other changes around, this actually leads in calling setConfiguration
in the option widget as if the entire config changed, and obviously this is
guarded by signals blocker (but it only changed one value in the settings,
why a big update like setConfiguration?).

Then to help with all this comes events launched by the canvas resource
manager, here we should only hear about preset changes, as in a new preset
has been set.
Though given that my vision is that every widget able to let the user
select a preset has to be wired up to the KisPresetManager through another
much more controlled way, the one i'm using with the KisPaintOpBox, who can
change presets beyond those widgets?
I don't know, and i don't know how to properly support this event without
knowing. As far as i know right now those events are triggered by the other
preset choosing widgets selecting some preset and setting it in the
resource provider.
Though again in my vision if the widgets gets their preset updated not from
a user interaction, they should not notify KisPresetManager, otherwise this
would leads to loops.
This would be tough though, because widgets are just windows on the loaded
presets, but they react to any preset add and there's no way to make
difference from a user interaction and from a non user interaction, because
in both cases the updates comes from the underlying widget/model update
code.
So this means changing the code that loads presets and or models that those
widgets depends on so that they react differently or signal one case
differently from the other one.

So in the end, as i said, back to the drawing board, listing all the
classes involved, what they are supposed to do and have to do, drawing the
interactions and unravel them since there are a lot of things that have to
be streamlined and should have a somewhat fixed order of execution
(especially needed with GUI interaction).
I think that the bigger refactoring cannot be avoided.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kimageshop/attachments/20150830/ad39ed75/attachment.html>


More information about the kimageshop mailing list