Hi,
Eric Kidd <eric.kidd@xxxxxxxxx> writes:
> The attached patch implementes "Grey" addition and subtraction, both as a
> layer mode and a brush mode. I've tested it with GIMP 1.2.2, but I'd like
> to hear any comments before porting it to 1.3.x.
I have some comments on the implementation that you'll find further down.
it would be nice if you could illustrate the effect with some images so
we can see if it is worth to add new LayerModeEffects for the result.
> The names of the transfer modes are highly provisional!
well, yes, I think we need better names.
> diff -ur gimp-1.2.2-old/app/apptypes.h gimp-1.2.2/app/apptypes.h
> --- gimp-1.2.2-old/app/apptypes.h Sat Dec 16 15:33:41 2000
> +++ gimp-1.2.2/app/apptypes.h Wed Sep 26 19:18:40 2001
> @@ -78,8 +78,15 @@
> DIVIDE_MODE,
> ERASE_MODE, /*< skip >*/
> REPLACE_MODE, /*< skip >*/
> - ANTI_ERASE_MODE /*< skip >*/
> + ANTI_ERASE_MODE, /*< skip >*/
> + GREY_ADDITION_MODE,
> + GREY_SUBTRACTION_MODE
> } LayerModeEffects;
you should probably move the new modes up before the special modes.
That said, we should probably let the special modes start with a value
large enough to give room for future additions. For 1.2 hwoever you
don't have much choice if you don't want to break scripts and plug-ins.
> diff -ur gimp-1.2.2-old/app/brush_select_cmds.c gimp-1.2.2/app/brush_select_cmds.c
> diff -ur gimp-1.2.2-old/app/brushes_cmds.c gimp-1.2.2/app/brushes_cmds.c
> diff -ur gimp-1.2.2-old/app/layer_cmds.c gimp-1.2.2/app/layer_cmds.c
> diff -ur gimp-1.2.2-old/app/tools_cmds.c gimp-1.2.2/app/tools_cmds.c
all these files are autogenerated PDB wrappers. If someone applies your
patch and runs make in the toplevel dir, the changes will get overwritten.
Most of the changes (the addition of new enums) will however automatically
propagate to the C files. Other changes have to be made in the
tools/pdbgen/pdb/ directory.
> +void
> +grey_add_pixels (const unsigned char *src1,
> + const unsigned char *src2,
> + unsigned char *dest,
> + int length,
> + int bytes1,
> + int bytes2,
> + int has_alpha1,
> + int has_alpha2)
> +{
> + int alpha, b;
> + int sum;
> +
> + alpha = (has_alpha1 || has_alpha2) ? MAX (bytes1, bytes2) - 1 : bytes1;
> +
> + while (length --)
> + {
> + for (b = 0; b < alpha; b++)
> + {
> + /* Add, re-center and clip. */
> + sum = src1[b] + src2[b] - 128;
> + dest[b] = MAX(MIN(sum, 255), 0);
you should use CLAMP(sum, 0, 255) here instead of combining MAX and MIN.
> +void
> +grey_subtract_pixels (const unsigned char *src1,
> + const unsigned char *src2,
> + unsigned char *dest,
> + int length,
> + int bytes1,
> + int bytes2,
> + int has_alpha1,
> + int has_alpha2)
> +{
> + int alpha, b;
> + int diff;
> +
> + alpha = (has_alpha1 || has_alpha2) ? MAX (bytes1, bytes2) - 1 : bytes1;
> +
> + while (length --)
> + {
> + for (b = 0; b < alpha; b++)
> + {
> + diff = src1[b] - src2[b] + 128;
> + dest[b] = MAX(MIN(diff, 255), 0);
again, use CLAMP().
Salut, Sven
