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).
This commit is contained in:
Jehan 2025-04-25 00:05:13 +02:00 committed by Alx Sa
parent a188a8db93
commit 8dbedcf723
2 changed files with 28 additions and 144 deletions

View file

@ -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,

View file

@ -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;