# HG changeset patch
# User Rik <rik@octave.org>
# Date 1378855894 25200
# Node ID 5e552cd9315a1cc4fba53d6f5a7d6bc844cc89cc
# Parent  db8b90a56298aee3adb84347bddba7529539cf05
Overhaul scatter family of functions

* scripts/plot/module.mk: Remove unused function __color_str_rgb__.m from build.

* scripts/plot/private/__color_str_rgb__.m: Remove unnecessary function.

* scripts/plot/scatter.m: Put input validation first.  Adjust indentation.

* scripts/plot/scatter3.m: Put input validation first.  Adjust indentation.
Use htmp instead of tmp for temporary graphics handle.

* scripts/plot/private/__scatter__.m: Update "markeredgecolor" in child patch
objects when same property is updated in hggroup.  Accept "fill" for "filled"
for Matlab compatibility.  Remove call to __color_str_rgb__.  Don't bother
saving graphics handle from __go_patch__ since it is never used.  Use
cellfun instead of for loop for input processing.

diff --git a/scripts/plot/module.mk b/scripts/plot/module.mk
--- a/scripts/plot/module.mk
+++ b/scripts/plot/module.mk
@@ -13,7 +13,6 @@
   plot/private/__axis_label__.m \
   plot/private/__bar__.m \
   plot/private/__clabel__.m \
-  plot/private/__color_str_rgb__.m \
   plot/private/__contour__.m \
   plot/private/__default_plot_options__.m \
   plot/private/__errcomm__.m \
diff --git a/scripts/plot/private/__color_str_rgb__.m b/scripts/plot/private/__color_str_rgb__.m
deleted file mode 100644
--- a/scripts/plot/private/__color_str_rgb__.m
+++ /dev/null
@@ -1,51 +0,0 @@
-## Copyright (C) 2010-2012 David Bateman
-##
-## This file is part of Octave.
-##
-## Octave is free software; you can redistribute it and/or modify it
-## under the terms of the GNU General Public License as published by
-## the Free Software Foundation; either version 3 of the License, or (at
-## your option) any later version.
-##
-## Octave is distributed in the hope that it will be useful, but
-## WITHOUT ANY WARRANTY; without even the implied warranty of
-## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-## General Public License for more details.
-##
-## You should have received a copy of the GNU General Public License
-## along with Octave; see the file COPYING.  If not, see
-## <http://www.gnu.org/licenses/>.
-
-## -*- texinfo -*-
-## @deftypefn {Function File} {@var{rgb} =} __color_str_rgb__ (@var{str})
-## Undocumented internal function.
-## @end deftypefn
-
-function rgb = __color_str_rgb__ (str)
-
-  if (ischar (str))
-    if (strncmpi (str, "black", 5))
-      rgb = [0, 0, 0];
-    elseif (strncmpi (str, "red", 3))
-      rgb = [1, 0, 0];
-    elseif (strncmpi (str, "green", 5))
-      rgb = [0, 1, 0];
-    elseif (strncmpi (str, "blue", 4))
-      rgb = [0, 0, 1];
-
-    elseif (strncmpi (str, "yellow", 6))
-      rgb = [1, 1, 0];
-    elseif (strncmpi (str, "magenta", 7))
-      rgb = [1, 0, 1];
-    elseif (strncmpi (str, "cyan", 4))
-      rgb = [0, 1, 1];
-    elseif (strncmpi (str, "white", 5))
-      rgb = [1, 1, 1];
-    else
-      rgb = [0, 0, 0];
-    endif
-  else
-    error ("__color_str_rgb__: expecting a string argument");
-  endif
-endfunction
-
diff --git a/scripts/plot/private/__scatter__.m b/scripts/plot/private/__scatter__.m
--- a/scripts/plot/private/__scatter__.m
+++ b/scripts/plot/private/__scatter__.m
@@ -23,34 +23,33 @@
 
 function hg = __scatter__ (varargin)
 
-  h = varargin{1};
-  nd = varargin{2};
+  hax = varargin{1};  # We don't do anything with this.  Could remove it.
+  nd  = varargin{2};
   fcn = varargin{3};
-  x = varargin{4}(:);
-  y = varargin{5}(:);
-  istart = 6;
+  x   = varargin{4}(:);
+  y   = varargin{5}(:);
 
-  if (nd == 3)
+  if (nd == 2)
+    idx = isnan (x) | isnan (y);
+    x(idx) = [];
+    y(idx) = [];
+    z = zeros (length (x), 0);
+    istart = 6;
+  else
     z = varargin{6}(:);
     idx = isnan (x) | isnan (y) | isnan (z);
-    x (idx) = [];
-    y (idx) = [];
-    z (idx) = [];
+    x(idx) = [];
+    y(idx) = [];
+    z(idx) = [];
     istart = 7;
-  else
-    idx = isnan (x) | isnan (y);
-    x (idx) = [];
-    y (idx) = [];
-    z = zeros (length (x), 0);
   endif
 
-  firstnonnumeric = Inf;
-  for i = istart:nargin
-    if (! isnumeric (varargin{i}))
-      firstnonnumeric = i;
-      break;
-    endif
-  endfor
+  firstnonnumeric = find (! cellfun ("isnumeric", varargin(istart:nargin)), 1);
+  if (isempty (firstnonnumeric))
+    firstnonnumeric = Inf;
+  else
+    firstnonnumeric += istart - 1;
+  endif
 
   if (istart <= nargin)
     s = varargin{istart};
@@ -66,13 +65,12 @@
 
   if (istart <= nargin && firstnonnumeric > istart)
     c = varargin{istart};
-    if (isvector (c))
-      if (columns (c) != 3)
-        c = c(:);
-      endif
+    if (isvector (c) && columns (c) != 3)
+      c = c(:);
     endif
+  ## Compare only first 4 letters of "fill" as that is what Matlab uses.
   elseif (firstnonnumeric == istart && ischar (varargin{istart})
-          && ! strcmpi (varargin{istart}, "filled"))
+          && ! strncmpi (varargin{istart}, "filled", 4))
     c = varargin{istart};
     firstnonnumeric++;
   else
@@ -86,18 +84,18 @@
   iarg = firstnonnumeric;
   while (iarg <= nargin)
     arg = varargin{iarg++};
-    if (ischar (arg) && strncmpi (arg, "filled", 6))
+    if (ischar (arg) && strncmpi (arg, "filled", 4))
       filled = true;
     elseif ((ischar (arg) || iscell (arg)) && ! have_marker)
       [linespec, valid] = __pltopt__ (fcn, arg, false);
       if (valid)
         have_marker = true;
         marker = linespec.marker;
-        if (strncmp (marker, "none", 4))
+        if (strcmp (marker, "none"))
           marker = "o";
         elseif (isempty (marker))
           have_marker = false;
-          [dummy, marker] = __next_line_style__ ();
+          [~, marker] = __next_line_style__ ();
         endif
       else
         error ("%s: invalid linespec", fcn);
@@ -116,13 +114,14 @@
 
   hg = hggroup ();
   newargs = __add_datasource__ (fcn, hg, {"x", "y", "z", "c", "size"},
-                             newargs{:});
+                                newargs{:});
 
   addproperty ("xdata", hg, "data", x);
   addproperty ("ydata", hg, "data", y);
   addproperty ("zdata", hg, "data", z);
   if (ischar (c))
-    addproperty ("cdata", hg, "data", __color_str_rgb__ (c));
+    ## For single explicit color, cdata is unused
+    addproperty ("cdata", hg, "data", []);
   else
     addproperty ("cdata", hg, "data", c);
   endif
@@ -146,45 +145,44 @@
     if (one_explicit_color)
       for i = 1 : numel (x)
         if (filled)
-          h = __go_patch__ (hg, "xdata", x(i), "ydata", y(i), "zdata", z(i,:),
-                            "faces", 1, "vertices", [x(i), y(i), z(i,:)],
-                            "facecolor", "none", "edgecolor", "none",
-                            "marker", marker,  "markersize", s(i),
-                            "markeredgecolor", c, "markerfacecolor", c,
-                            "linestyle", "none");
+          __go_patch__ (hg, "xdata", x(i), "ydata", y(i), "zdata", z(i,:),
+                        "faces", 1, "vertices", [x(i), y(i), z(i,:)],
+                        "facecolor", "none", "edgecolor", "none",
+                        "marker", marker,  "markersize", s(i),
+                        "markeredgecolor", c, "markerfacecolor", c,
+                        "linestyle", "none");
         else
-          h = __go_patch__ (hg, "xdata", x(i), "ydata", y(i), "zdata", z(i,:),
-                            "faces", 1, "vertices", [x(i), y(i), z(i,:)],
-                            "facecolor", "none", "edgecolor", "none",
-                            "marker", marker,  "markersize", s(i),
-                            "markeredgecolor", c, "markerfacecolor", "none",
-                            "linestyle", "none");
+          __go_patch__ (hg, "xdata", x(i), "ydata", y(i), "zdata", z(i,:),
+                        "faces", 1, "vertices", [x(i), y(i), z(i,:)],
+                        "facecolor", "none", "edgecolor", "none",
+                        "marker", marker,  "markersize", s(i),
+                        "markeredgecolor", c, "markerfacecolor", "none",
+                        "linestyle", "none");
         endif
       endfor
     else
       if (rows (c) == 1)
-        c = ones (rows (x), 1) * c;
+        c = repmat (c, rows (x), 1);
       endif
       for i = 1 : numel (x)
         if (filled)
-          h = __go_patch__ (hg, "xdata", x(i), "ydata", y(i), "zdata", z(i,:),
-                            "faces", 1, "vertices", [x(i), y(i), z(i,:)],
-                            "facecolor", "none", "edgecolor", "none",
-                            "marker", marker, "markersize", s(i),
-                            "markeredgecolor", "none",
-                            "markerfacecolor", "flat",
-                            "cdata", c(i,:), "facevertexcdata", c(i,:),
-                            "linestyle", "none");
+          __go_patch__ (hg, "xdata", x(i), "ydata", y(i), "zdata", z(i,:),
+                        "faces", 1, "vertices", [x(i), y(i), z(i,:)],
+                        "facecolor", "none", "edgecolor", "none",
+                        "marker", marker, "markersize", s(i),
+                        "markeredgecolor", "none",
+                        "markerfacecolor", "flat",
+                        "cdata", c(i,:), "facevertexcdata", c(i,:),
+                        "linestyle", "none");
         else
-          h = __go_patch__ (hg, "xdata", x(i), "ydata", y(i), "zdata", z(i,:),
-                            "faces", 1, "vertices", [x(i), y(i), z(i,:)],
-                            "facecolor", "none", "edgecolor", "none",
-                            "marker", marker, "markersize", s(i),
-                            "markeredgecolor", "flat",
-                            "markerfacecolor", "none",
-                            "cdata", c(i,:), "facevertexcdata", c(i,:),
-                            "linestyle", "none");
-
+          __go_patch__ (hg, "xdata", x(i), "ydata", y(i), "zdata", z(i,:),
+                        "faces", 1, "vertices", [x(i), y(i), z(i,:)],
+                        "facecolor", "none", "edgecolor", "none",
+                        "marker", marker, "markersize", s(i),
+                        "markeredgecolor", "flat",
+                        "markerfacecolor", "none",
+                        "cdata", c(i,:), "facevertexcdata", c(i,:),
+                        "linestyle", "none");
         endif
       endfor
     endif
@@ -195,23 +193,23 @@
 
     vert = [x, y, z];
     if (one_explicit_color)
-      h = render_size_color (hg, vert, s, c, marker, filled, true);
+      render_size_color (hg, vert, s, c, marker, filled, true);
     else
       if (rows (c) == 1)
-        c = ones (rows (x), 1) * c;
+        c = repmat (c, rows (x), 1);
       endif
-      ## We want to group points by colour. So first get all the unique colours
+      ## We want to group points by color.  So first get all the unique colors
       [cc, ~, c_to_cc] = unique (c, "rows");
 
-      for i = 1:rows (cc)
-        ## Now for each possible unique colour, get the logical index of
-        ## points that correspond to that colour
+      for i = 1 : rows (cc)
+        ## Now for each possible unique color, get the logical index of
+        ## points that correspond to that color
         idx = (i == c_to_cc);
         if (isscalar (s))
-          h = render_size_color (hg, vert(idx, :), s, c(idx,:),
+          render_size_color (hg, vert(idx, :), s, c(idx,:),
                                  marker, filled, true);
         else
-          h = render_size_color (hg, vert(idx, :), s(idx), c(idx,:),
+          render_size_color (hg, vert(idx, :), s(idx), c(idx,:),
                                  marker, filled, true);
         endif
       endfor
@@ -259,20 +257,20 @@
 
 endfunction
 
-function h = render_size_color (hg, vert, s, c, marker, filled, isflat)
+function render_size_color (hg, vert, s, c, marker, filled, isflat)
   if (isscalar (s))
     x = vert(:,1);
     y = vert(:,2);
     z = vert(:,3:end);
     toolkit = get (ancestor (hg, "figure"), "__graphics_toolkit__");
     ## Does gnuplot only support triangles with different vertex colors ?
-    ## TODO - Verify gnuplot can only support one color. If RGB triplets
-    ##        can be assigned to each vertex, then fix __go_draw_axe__.m
+    ## TODO: Verify gnuplot can only support one color.  If RGB triplets
+    ##       can be assigned to each vertex, then fix __go_draw_axes__.m
     gnuplot_hack = (numel (x) > 1 && columns (c) == 3
                     && strcmp (toolkit, "gnuplot"));
     if (ischar (c) || ! isflat || gnuplot_hack)
       if (filled)
-        h = __go_patch__ (hg, "xdata", x, "ydata", y, "zdata", z,
+        __go_patch__ (hg, "xdata", x, "ydata", y, "zdata", z,
                           "faces", 1:numel (x), "vertices", vert,
                           "facecolor", "none", "edgecolor", "none",
                           "marker", marker,
@@ -280,7 +278,7 @@
                           "markerfacecolor", c(1,:),
                           "markersize", s, "linestyle", "none");
       else
-        h = __go_patch__ (hg, "xdata", x, "ydata", y, "zdata", z,
+        __go_patch__ (hg, "xdata", x, "ydata", y, "zdata", z,
                           "faces", 1:numel (x), "vertices", vert,
                           "facecolor", "none", "edgecolor", "none",
                           "marker", marker,
@@ -290,7 +288,7 @@
       endif
     else
       if (filled)
-        h = __go_patch__ (hg, "xdata", x, "ydata", y, "zdata", z,
+        __go_patch__ (hg, "xdata", x, "ydata", y, "zdata", z,
                           "faces", 1:numel (x), "vertices", vert,
                           "facecolor", "none", "edgecolor", "none",
                           "marker", marker, "markersize", s,
@@ -299,7 +297,7 @@
                           "cdata", c, "facevertexcdata", c,
                           "linestyle", "none");
       else
-        h = __go_patch__ (hg, "xdata", x, "ydata", y, "zdata", z,
+        __go_patch__ (hg, "xdata", x, "ydata", y, "zdata", z,
                           "faces", 1:numel (x), "vertices", vert,
                           "facecolor", "none", "edgecolor", "none",
                           "marker", marker, "markersize", s,
@@ -310,11 +308,11 @@
       endif
     endif
   else
-    ## FIXME: round the size to one decimal place. It's not quite right, though.
+    ## Round size to one decimal place.
     [ss, ~, s_to_ss] = unique (ceil (s*10) / 10);
     for i = 1:rows (ss)
       idx = (i == s_to_ss);
-      h = render_size_color (hg, vert(idx,:), ss(i), c,
+      render_size_color (hg, vert(idx,:), ss(i), c,
                              marker, filled, isflat);
     endfor
   endif
@@ -322,58 +320,44 @@
 
 function update_props (h, d)
   lw = get (h, "linewidth");
-  m = get (h, "marker");
+  m  = get (h, "marker");
   fc = get (h, "markerfacecolor");
   ec = get (h, "markeredgecolor");
   kids = get (h, "children");
 
-  for i = 1 : numel (kids)
-    set (kids (i), "linewidth", lw, "marker", m, "markerfacecolor", fc,
-         "edgecolor", ec);
-  endfor
+  set (kids, "linewidth", lw, "marker", m,
+             "markerfacecolor", fc, "markeredgecolor", ec);
 endfunction
 
 function update_data (h, d)
-  x1 = get (h, "xdata");
-  y1 = get (h, "ydata");
-  z1 = get (h, "zdata");
-  c1 = get (h, "cdata");
-  if (!ischar (c1) && rows (c1) == 1)
-    c1 = repmat (c1, numel (x1), 1);
+  x = get (h, "xdata");
+  y = get (h, "ydata");
+  z = get (h, "zdata");
+  c = get (h, "cdata");
+  if (rows (c) == 1)
+    c = repmat (c, numel (x), 1);
   endif
-  size1 = get (h, "sizedata");
-  if (numel (size1) == 1)
-    size1 = repmat (size1, numel (x1), 1);
+  s = get (h, "sizedata");
+  if (numel (s) == 1)
+    s = repmat (s, numel (x), 1);
   endif
   hlist = get (h, "children");
-  if (ischar (c1))
-    if (isempty (z1))
-      for i = 1 : length (hlist)
-        set (hlist(i), "vertices", [x1(i), y1(i)], "cdata", c1,
-             "markersize", size1(i));
-      endfor
-    else
-      for i = 1 : length (hlist)
-        set (hlist(i), "vertices", [x1(i), y1(i), z1(i)], "cdata", c1,
-             "markersize", size1(i));
-      endfor
-    endif
+
+  if (isempty (z))
+    for i = 1 : length (hlist)
+      set (hlist(i), "vertices", [x(i), y(i)],
+                     "cdata", reshape (c(i,:),[1, size(c)(2:end)]),
+                     "facevertexcdata", c(i,:),
+                     "markersize", s(i));
+    endfor
   else
-    if (isempty (z1))
-      for i = 1 : length (hlist)
-        set (hlist(i), "vertices", [x1(i), y1(i)], "cdata",
-             reshape (c1(i,:),[1, size(c1)(2:end)]),
-             "facevertexcdata", c1(i,:),
-             "markersize", size1(i));
-      endfor
-    else
-      for i = 1 : length (hlist)
-        set (hlist(i), "vertices", [x1(i), y1(i), z1(i)], "cdata",
-             reshape (c1(i,:),[1, size(c1)(2:end)]),
-             "facevertexcdata", c1(i,:),
-             "markersize", size1(i));
-      endfor
-    endif
+    for i = 1 : length (hlist)
+      set (hlist(i), "vertices", [x(i), y(i), z(i)],
+                     "cdata", reshape (cd(i,:),[1, size(cd)(2:end)]),
+                     "facevertexcdata", cd(i,:),
+                     "markersize", s(i));
+    endfor
   endif
+
 endfunction
 
diff --git a/scripts/plot/scatter.m b/scripts/plot/scatter.m
--- a/scripts/plot/scatter.m
+++ b/scripts/plot/scatter.m
@@ -74,21 +74,21 @@
 
   if (nargin < 2)
     print_usage ();
-  else
+  endif
+
   oldfig = [];
   if (! isempty (hax))
     oldfig = get (0, "currentfigure");
   endif
-    unwind_protect
-      hax = newplot (hax);
-      
-      htmp = __scatter__ (hax, 2, "scatter", varargin{:});
-    unwind_protect_cleanup
+  unwind_protect
+    hax = newplot (hax);
+    
+    htmp = __scatter__ (hax, 2, "scatter", varargin{:});
+  unwind_protect_cleanup
     if (! isempty (oldfig))
       set (0, "currentfigure", oldfig);
     endif
-    end_unwind_protect
-  endif
+  end_unwind_protect
 
   if (nargout > 0)
     retval = htmp;
diff --git a/scripts/plot/scatter3.m b/scripts/plot/scatter3.m
--- a/scripts/plot/scatter3.m
+++ b/scripts/plot/scatter3.m
@@ -71,29 +71,29 @@
 
   if (nargin < 2)
     print_usage ();
-  else
+  endif
+
   oldfig = [];
   if (! isempty (hax))
     oldfig = get (0, "currentfigure");
   endif
-    unwind_protect
-      hax = newplot (hax);
-      
-      tmp = __scatter__ (hax, 3, "scatter3", varargin{:});
+  unwind_protect
+    hax = newplot (hax);
+    
+    htmp = __scatter__ (hax, 3, "scatter3", varargin{:});
 
-      if (! ishold (hax))
-        set (hax, "view", [-37.5, 30],
-                  "xgrid", "on", "ygrid", "on", "zgrid", "on");
-      endif
-    unwind_protect_cleanup
-      if (! isempty (oldfig))
-        set (0, "currentfigure", oldfig);
-      endif
-    end_unwind_protect
-  endif
+    if (! ishold (hax))
+      set (hax, "view", [-37.5, 30],
+                "xgrid", "on", "ygrid", "on", "zgrid", "on");
+    endif
+  unwind_protect_cleanup
+    if (! isempty (oldfig))
+      set (0, "currentfigure", oldfig);
+    endif
+  end_unwind_protect
 
   if (nargout > 0)
-    retval = tmp;
+    retval = htmp;
   endif
 
 endfunction