adding dither function

[leolca]

I've made an implementation of the dither function, compatible with Matlab implementation (https://www.mathworks.com/help/matlab/ref/dither.html). In Matlab it seems not to be a part of Image package, so I guess it would also fit directly into octave core.

[mmuetzel]

Thank you very much for your contribution.

Sorry it took so long for someone to review your PR. This is just a read-only mirror of the upstream Mercurial repository. (But we try to port PR from here to the upstream repository if possible.)

I haven't checked the new function yet. But I left a few comments about the coding style that is used in Octave.

For the time being, I only took a look at the function documentation and had a very quick look at the function bodies (mostly for style).

I didn't bother to leave a comment at each and every line where the style should be adapted. But I tried to describe the coding style at some examples.

[mmuetzel]

Thank you again for the modifications.

Please, also add the new file to the build system. (So, it will be included when creating a source tarball). For that, add the new file to the list starting here:

octave/scripts/image/module.mk

Line 15 in c77d758

%canon_reldir%_FCN_FILES = \

(That list is in lexical order afaict.)

Also, please add the documentation for the new file to the manual. A good place might be around here:

octave/doc/interpreter/image.txi

Lines 143 to 154 in c77d758

	The following convenience functions are available for conversion between image
	formats.
	@DOCSTRING(im2double)
	@DOCSTRING(gray2ind)
	@DOCSTRING(ind2gray)
	@DOCSTRING(rgb2ind)
	@DOCSTRING(ind2rgb)

But maybe, you'll find a better location for that.

[mmuetzel]

Thank you again for all these nitty changes.

Did you read the above comment?
Are you comfortable making those changes?

[mmuetzel]

This part renders as the following as PDF:

Is that what you intended here?

[mmuetzel]

With the current iteration, this looks like this in the PDF:

Like this in the console output:

     ‘X = dither (RGB,MAP)’ creates an indexed image approximation.  It
     uses the color provided in the colormap, and uses dithering to
     increase apparent color resolution.  Floyd-Steinberg error filter
     is:
               x    7/16
          3/16 5/16 1/16

     It uses a raster scan and no weight renormalization at boundaries.
     The default values are used: QM=5 and QE=8.

And like this in the Qt documentation browser:

That looks reasonable to me now. 👍

[mmuetzel]

This renders like the following as PDF:

Maybe something along the lines of what is done in condest.m would look better?

octave/scripts/linear-algebra/condest.m

Lines 98 to 111 in 81aa793

	## References:
	##
	## @itemize
	## @item
	## @nospell{N.J. Higham and F. Tisseur}, @cite{A Block Algorithm
	## for Matrix 1-Norm Estimation, with an Application to 1-Norm
	## Pseudospectra}. SIMAX vol 21, no 4, pp 1185--1201.
	## @url{https://dx.doi.org/10.1137/S0895479899356080}
	##
	## @item
	## @nospell{N.J. Higham and F. Tisseur}, @cite{A Block Algorithm
	## for Matrix 1-Norm Estimation, with an Application to 1-Norm
	## Pseudospectra}. @url{https://citeseer.ist.psu.edu/223007.html}
	## @end itemize

[mmuetzel]

This looks like this in the PDF now:

Like this in the console:

     References:

        • Floyd, R. W. and Steinberg, L., ‘An Adaptive Algorithm for
          Spatial Gray Scale’.  International Symposium Digest of
          Technical Papers, Society for Information Displays, 1975, p.
          36.

        • Ulichney, R., ‘Digital Halftoning’.  The MIT Press, 1987.

And like this in the Qt documentation browser.

Looks good to me. 👍

[mmuetzel]

I finally came around to build Octave and its documentation with your changes locally.

All demos and tests pass for me. 👍

I made the following additional changes for integrating the new file into Octave:

diff --git a/doc/interpreter/image.txi b/doc/interpreter/image.txi
--- a/doc/interpreter/image.txi
+++ b/doc/interpreter/image.txi
@@ -153,6 +153,10 @@ formats.
 
 @DOCSTRING(ind2rgb)
 
+The following function allows processing an existing image.
+
+@DOCSTRING(dither)
+
 Octave also provides tools to produce and work with movie frame structures.
 Those structures encapsulate the image data (@qcode{"cdata"} field) together
 with the corresponding colormap (@qcode{"colormap"} field).

diff --git a/scripts/help/__unimplemented__.m b/scripts/help/__unimplemented__.m
--- a/scripts/help/__unimplemented__.m
+++ b/scripts/help/__unimplemented__.m
@@ -747,7 +747,6 @@ function rlist = missing_functions ()
   "discretize",
   "dissect",
   "distances",
-  "dither",
   "docsearch",
   "dragrect",
   "duration",

diff --git a/scripts/image/module.mk b/scripts/image/module.mk
--- a/scripts/image/module.mk
+++ b/scripts/image/module.mk
@@ -25,6 +25,7 @@ FCN_FILE_DIRS += \
   %reldir%/cool.m \
   %reldir%/copper.m \
   %reldir%/cubehelix.m \
+  %reldir%/dither.m \
   %reldir%/flag.m \
   %reldir%/frame2im.m \
   %reldir%/getframe.m \

I also had to make the following change or the documentation wouldn't build:

diff --git a/scripts/image/dither.m b/scripts/image/dither.m
--- a/scripts/image/dither.m
+++ b/scripts/image/dither.m
@@ -71,7 +71,7 @@
 ## Information Displays, 1975, p. 36.}
 ## @cite{Ulichney, R., Digital Halftoning, The MIT Press, 1987.}
 ##
-## @seealso{rgb2ind, imapprox}
+## @seealso{rgb2ind}
 ## @end deftypefn
 
 function X = dither (RGB, map, Qm = 5, Qe = 8)

Could you please integrate those changes (and address the other comments that I left in the last round of review)?

[leolca]

thanks for the feedback, suggestions and comments

[mmuetzel]

Thanks again for the follow-up. Could you please integrate the changes to the three files mentioned in the comment above?
Without them, I need to make those changes manually each time I try to build locally with the changes from this PR.

[mmuetzel]

I squashed the changes from this PR into a single commit, added a commit message, and pushed it with very minor changes to the upstream Mercurial repository.
It should be appearing in this mirror shortly.

Thank you again very much for your contribution.

[mmuetzel]

Synced here:
c014cb1