Closed jplloyd closed 4 years ago
@briend Unless you think that switching the FLIP
state once for each prepare_and_draw_dab
is incorrect (or have any other issues with these changes), I'm probably merging this tomorrow evening (CET, so in 24 hours).
Gave it a spin, looks good to me! Thanks @jplloyd !
This is primarily an effort to make the code in
mypaint-brush.c
more readable. It does the following:Use macros to make state/setting/input accesses via the generated enums shorter, by being able to omit the (necessary) prefixes (
MYPAINT_BRUSH_XYZ_
) and factoring out access patterns. Although one should always be wary of using macros, these are single line substitutions that massively improve code readability by reducing the length of the expressions. One argument against not using the full enums is that they become harder to grep, but since the enums are only used in this file anyway, it doesn't really matter.Clear out commented-out code and rhetorical questions.
Split out the smudge color update and blending into separate functions. These were rather massive conditional blocks in
prepare_and_draw_dab
that made the function harder to parse (for humans).Add const to const'able local variables and initialize them closer to where they are actually used. When the code (mostly) conformed to C89 this was not legal (generally), but now it's better to make it clear when something is immutable - it makes it a lot easier to follow the logic and equations.
One (intentional) semantic change: The flip state (switching between -1 and 1), is now only switched prior to the
prepare_and_draw_dab
call, and not in theupdate_states_and_settings
function. The reason being that the latter can be called more than once for some dabs.Some of the commits could be squashed, but I think it's better to have a decent granularity for changes like these. If a bug was introduced, it's usually a lot nicer to face a
+50, -62
diff at the end of a bisect, rather than one that is+462, -498
.