From 8dbedcf7230174f3a96dfd12b4428200550c6569 Mon Sep 17 00:00:00 2001 From: Jehan Date: Fri, 25 Apr 2025 00:05:13 +0200 Subject: [PATCH] app: review and improve over MR !151 for an "Overwrite" paint mode. I get why Woynert wanted to still interpolate both alpha and RGB channels here, avoiding some too harsh transition when using the brush tool. But the algorithm felt overly complicated (unless I misunderstood the reason). I think what we want is simply to use the mask value to interpolate, which allows the alpha and color to smoothly transition in parts where the brush designer was expecting transitionning, when used in a paintbrush-type tool. In particular the proposed algorithm was never using the layer[ALPHA] value which felt quite weird. Additionally to the algorithm update, this commit: * Makes the composite mode/space and blend space explicitly immutable. * Removes code from other composite modes, which was anyway just copy-paste remnants from gimp:normal operation and add some g_return_val_if_reached() call to protect these code paths. * Fix a code path where the out array was never filled (i.e. it was filled with random data). * Fix another code path where we could make a division by zero (in my case, it didn't crash, but I got nan values). --- app/operations/layer-modes/gimp-layer-modes.c | 4 +- .../layer-modes/gimpoperationoverwrite.c | 168 +++--------------- 2 files changed, 28 insertions(+), 144 deletions(-) diff --git a/app/operations/layer-modes/gimp-layer-modes.c b/app/operations/layer-modes/gimp-layer-modes.c index c3b78f835b..e7430fa6bb 100644 --- a/app/operations/layer-modes/gimp-layer-modes.c +++ b/app/operations/layer-modes/gimp-layer-modes.c @@ -835,7 +835,9 @@ static const GimpLayerModeInfo layer_mode_infos[] = { GIMP_LAYER_MODE_OVERWRITE, .op_name = "gimp:overwrite", - .flags = GIMP_LAYER_MODE_FLAG_BLEND_SPACE_IMMUTABLE | + .flags = GIMP_LAYER_MODE_FLAG_BLEND_SPACE_IMMUTABLE | + GIMP_LAYER_MODE_FLAG_COMPOSITE_MODE_IMMUTABLE | + GIMP_LAYER_MODE_FLAG_COMPOSITE_SPACE_IMMUTABLE | GIMP_LAYER_MODE_FLAG_TRIVIAL, .context = GIMP_LAYER_MODE_CONTEXT_PAINT, .paint_composite_mode = GIMP_LAYER_COMPOSITE_UNION, diff --git a/app/operations/layer-modes/gimpoperationoverwrite.c b/app/operations/layer-modes/gimpoperationoverwrite.c index 8d4a7a20c8..54d4e68c22 100644 --- a/app/operations/layer-modes/gimpoperationoverwrite.c +++ b/app/operations/layer-modes/gimpoperationoverwrite.c @@ -78,6 +78,8 @@ gimp_operation_overwrite_process (GeglOperation *op, gfloat *mask = mask_p; gfloat opacity = layer_mode->opacity; const gboolean has_mask = mask != NULL; + gfloat mask_value; + gfloat layer_alpha; switch (layer_mode->composite_mode) { @@ -85,41 +87,33 @@ gimp_operation_overwrite_process (GeglOperation *op, case GIMP_LAYER_COMPOSITE_AUTO: while (samples--) { + layer_alpha = layer[ALPHA] * opacity; + if (has_mask) + mask_value = *(mask++); + else + mask_value = 1.0; + + if (layer_alpha > 0.0f && mask_value > 0.0f) { - out[ALPHA] = in[ALPHA] + (*mask) * (opacity - in[ALPHA]); + gint b; - if (opacity + in[ALPHA] > -1e-5) - { - gfloat C; - gfloat lerp; - gint b; + /* We still interpolate both the alpha and RGB channels, + * using the mask value which gives us smoother transition + * in paintbrush style tools while the pencil tool + * provides hard pixel replacement as expected. + */ + out[ALPHA] = in[ALPHA] + mask_value * (layer_alpha - in[ALPHA]); - /* RGB interpolation */ + for (b = RED; b < ALPHA; b++) + out[b] = in[b] + mask_value * (layer[b] - in[b]); + } + else + { + gint b; - C = in[ALPHA] / ( opacity + in[ALPHA] ); - - if (*mask < C) - lerp = *mask * ((1 - C) / C); - else - lerp = (*mask - C) * (C / (1 - C)) + 1 - C; - - for (b = RED; b < ALPHA; b++) - { - out[b] = in[b] + (lerp) * (layer[b] - in[b]); - } - } - else - { - gint b; - - for (b = RED; b < ALPHA; b++) - { - out[b] = in[b]; - } - } - - mask++; + for (b = RED; b <= ALPHA; b++) + out[b] = in[b]; } in += 4; @@ -129,121 +123,9 @@ gimp_operation_overwrite_process (GeglOperation *op, break; case GIMP_LAYER_COMPOSITE_CLIP_TO_BACKDROP: - while (samples--) - { - gfloat layer_alpha; - - layer_alpha = layer[ALPHA] * opacity; - if (has_mask) - layer_alpha *= *mask; - - out[ALPHA] = in[ALPHA]; - - if (out[ALPHA]) - { - gint b; - - for (b = RED; b < ALPHA; b++) - { - out[b] = in[b] + (layer[b] - in[b]) * layer_alpha; - } - } - else - { - gint b; - - for (b = RED; b < ALPHA; b++) - { - out[b] = in[b]; - } - } - - in += 4; - layer += 4; - out += 4; - - if (has_mask) - mask++; - } - break; - case GIMP_LAYER_COMPOSITE_CLIP_TO_LAYER: - while (samples--) - { - gfloat layer_alpha; - - layer_alpha = layer[ALPHA] * opacity; - if (has_mask) - layer_alpha *= *mask; - - out[ALPHA] = layer_alpha; - - if (out[ALPHA]) - { - gint b; - - for (b = RED; b < ALPHA; b++) - { - out[b] = layer[b]; - } - } - else - { - gint b; - - for (b = RED; b < ALPHA; b++) - { - out[b] = in[b]; - } - } - - in += 4; - layer += 4; - out += 4; - - if (has_mask) - mask++; - } - break; - case GIMP_LAYER_COMPOSITE_INTERSECTION: - while (samples--) - { - gfloat layer_alpha; - - layer_alpha = layer[ALPHA] * opacity; - if (has_mask) - layer_alpha *= *mask; - - out[ALPHA] = in[ALPHA] * layer_alpha; - - if (out[ALPHA]) - { - gint b; - - for (b = RED; b < ALPHA; b++) - { - out[b] = layer[b]; - } - } - else - { - gint b; - - for (b = RED; b < ALPHA; b++) - { - out[b] = in[b]; - } - } - - in += 4; - layer += 4; - out += 4; - - if (has_mask) - mask++; - } - break; + g_return_val_if_reached (FALSE); } return TRUE;