Mercurial > octave

changeset 30634:36d940c58c2e

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Improve input validation for patch function (bug #61782) * patch.m: Avoid call to gca() before executing __patch__ as input validation may stop patch from being created leaving empty figure and axes objects. Add BIST tests for input validation. * __patch__.m: Remove second output of function (failed). Detect 3-input case with bad color specification and issue and emit an error. Add comments to clarify code about which input combination is being processed in long if/elseif tree. Call gca() if patch is validated but no axes has been specified. * fill.m: Re-write call to __patch__ to match new function with only one output.
author Rik <rik@octave.org>
date Tue, 11 Jan 2022 12:17:16 -0800
parents f1cec1134dd1
children ad8c9d93b86d
files scripts/plot/draw/fill.m scripts/plot/draw/patch.m scripts/plot/draw/private/__patch__.m
diffstat 3 files changed, 128 insertions(+), 130 deletions(-) [+]
line wrap: on
line diff
--- a/scripts/plot/draw/fill.m	Tue Jan 11 20:43:58 2022 +0100
+++ b/scripts/plot/draw/fill.m	Tue Jan 11 12:17:16 2022 -0800
@@ -141,12 +141,9 @@
         ## For Matlab compatibility, return 1 patch object for each column
         for j = 1 : columns (x)
           if (one_color)
-            [htmp, err] = __patch__ (hax, x(:,j), y(:,j), cdata, opts{:});
+            htmp = __patch__ (hax, x(:,j), y(:,j), cdata, opts{:});
           else
-            [htmp, err] = __patch__ (hax, x(:,j), y(:,j), cdata(:,j), opts{:});
-          endif
-          if (err)
-            print_usage ();
+            htmp = __patch__ (hax, x(:,j), y(:,j), cdata(:,j), opts{:});
           endif
           hlist(end+1, 1) = htmp;
         endfor
--- a/scripts/plot/draw/patch.m	Tue Jan 11 20:43:58 2022 +0100
+++ b/scripts/plot/draw/patch.m	Tue Jan 11 12:17:16 2022 -0800
@@ -28,7 +28,7 @@
 ## @deftypefnx {} {} patch (@var{x}, @var{y}, @var{c})
 ## @deftypefnx {} {} patch (@var{x}, @var{y}, @var{z}, @var{c})
 ## @deftypefnx {} {} patch ("Faces", @var{faces}, "Vertices", @var{verts}, @dots{})
-## @deftypefnx {} {} patch (@dots{}, @var{prop}, @var{val}, @dots{})
+## @deftypefnx {} {} patch (@dots{}, "@var{prop}", @var{val}, @dots{})
 ## @deftypefnx {} {} patch (@dots{}, @var{propstruct}, @dots{})
 ## @deftypefnx {} {} patch (@var{hax}, @dots{})
 ## @deftypefnx {} {@var{h} =} patch (@dots{})
@@ -82,17 +82,11 @@
 
   [hax, varargin] = __plt_get_axis_arg__ ("patch", varargin{:});
 
-  if (isempty (hax))
-    hax = gca ();
-  else
+  if (! isempty (hax))
     hax = hax(1);
-  endif
+  endif 
 
-  [htmp, failed] = __patch__ (hax, varargin{:});
-
-  if (failed)
-    print_usage ();
-  endif
+  htmp = __patch__ (hax, varargin{:});
 
   if (nargout > 0)
     h = htmp;
@@ -308,3 +302,9 @@
 %! unwind_protect_cleanup
 %!   close (hf);
 %! end_unwind_protect
+
+## Test input validation
+%!error <invalid color specification C> patch (1, 1, 'x')
+%!error <invalid TrueColor data C> patch (1, 1, rand (1,2,3))
+%!error <size of X, Y, and C must be equal> patch (1, 1, [1, 2])
+%!error <invalid color specification C> patch (1, 1, {1})
--- a/scripts/plot/draw/private/__patch__.m	Tue Jan 11 20:43:58 2022 +0100
+++ b/scripts/plot/draw/private/__patch__.m	Tue Jan 11 12:17:16 2022 -0800
@@ -32,153 +32,154 @@
 ## Create patch object from x and y with color c and parent p.
 ## Return handle to patch object.
 
-function [h, failed] = __patch__ (p, varargin)
+function h = __patch__ (p, varargin)
 
   h = NaN;
-  failed = false;
+  nargin = nargin - 1;
 
   is_numeric_arg = cellfun (@isnumeric, varargin);
 
   if (isempty (varargin))
-    args = varargin;
+    args = {};
   elseif (is_numeric_arg(1))
     if (nargin < 3 || ! is_numeric_arg(2))
-      failed = true;
-    else
-      if (nargin > 4 && all (is_numeric_arg(1:4)))
-        x = varargin{1};
-        y = varargin{2};
+      print_usage ("patch");
+    endif
+
+    if (nargin >= 4 && all (is_numeric_arg(1:4)))
+      x = varargin{1};
+      y = varargin{2};
+      z = varargin{3};
+      c = varargin{4};
+      iarg = 5;
+    elseif (nargin >= 3 && all (is_numeric_arg(1:3)))
+      x = varargin{1};
+      y = varargin{2};
+      if (nargin > 3 && iscolorspec (varargin{4}))
         z = varargin{3};
         c = varargin{4};
         iarg = 5;
-      elseif (nargin > 3 && all (is_numeric_arg(1:3)))
-        x = varargin{1};
-        y = varargin{2};
-        if (nargin > 4 && iscolorspec (varargin{4}))
-          z = varargin{3};
-          c = varargin{4};
-          iarg = 5;
-        else
-          z = [];
-          c = varargin{3};
-          iarg = 4;
-        endif
-      elseif (nargin > 2 && all (is_numeric_arg(1:2)))
-        x = varargin{1};
-        y = varargin{2};
+      else
         z = [];
-        if (iscolorspec (varargin{3}))
-          c = varargin{3};
-          iarg = 4;
-        else
-          c = [];
-          iarg = 3;
-        endif
+        c = varargin{3};
+        iarg = 4;
       endif
+    elseif (nargin >= 3 && all (is_numeric_arg(1:2)))
+      x = varargin{1};
+      y = varargin{2};
+      z = [];
+      if (iscolorspec (varargin{3}))
+        c = varargin{3};
+        iarg = 4;
+      elseif (nargin == 3)
+        error ("patch: invalid color specification C");
+      else
+        c = [];
+        iarg = 3;
+      endif
+    endif
 
-      if (isvector (x))
-        x = x(:);
-        y = y(:);
-        z = z(:);
-        if (isnumeric (c))
-          if (isvector (c))
-            if (isequal (size (c), [1, 3]))
-              ## Do nothing, this is a single RGB color specification
-            elseif (numel (c) == numel (x))
-              c = c(:);
-            endif
-          elseif (rows (c) != numel (x) && columns (c) == numel (x))
-            c = c.';
+    if (isvector (x))
+      x = x(:);
+      y = y(:);
+      z = z(:);
+      if (isnumeric (c))
+        if (isvector (c))
+          if (isequal (size (c), [1, 3]))
+            ## Do nothing, this is a single RGB color specification
+          elseif (numel (c) == numel (x))
+            c = c(:);
           endif
+        elseif (rows (c) != numel (x) && columns (c) == numel (x))
+          c = c.';
         endif
       endif
-      args{1} = "xdata";
-      args{2} = x;
-      args{3} = "ydata";
-      args{4} = y;
-      args{5} = "zdata";
-      args{6} = z;
+    endif
+    args{1} = "xdata";
+    args{2} = x;
+    args{3} = "ydata";
+    args{4} = y;
+    args{5} = "zdata";
+    args{6} = z;
 
-      if (isnumeric (c))
+    if (isnumeric (c) && ! isempty (c))
 
-        if (ndims (c) == 3 && columns (c) == 1)
-          c = permute (c, [1, 3, 2]);
-        endif
+      if (ndims (c) == 3 && columns (c) == 1)
+        c = permute (c, [1, 3, 2]);
+      endif
 
-        if (isvector (c) && numel (c) == columns (x))
-          if (isnan (c))
-            args{7} = "facecolor";
-            args{8} = [1, 1, 1];
-            args{9} = "cdata";
-            args{10} = c;
-          elseif (isnumeric (c))
-            args{7} = "facecolor";
-            args{8} = "flat";
-            args{9} = "cdata";
-            args{10} = c;
-          else
-            error ("patch: color data C must be numeric");
-          endif
-        elseif (isrow (c) && numel (c) == 3)
+      if (isvector (c) && numel (c) == columns (x))
+        ## One color per face
+        if (isnan (c))
+          args{7} = "facecolor";
+          args{8} = [1, 1, 1];
+        else
           args{7} = "facecolor";
-          args{8} = c;
+          args{8} = "flat";
+        endif
+        args{9} = "cdata";
+        args{10} = c;
+      elseif (isrow (c) && numel (c) == 3)
+        ## One RGB color
+        args{7} = "facecolor";
+        args{8} = c;
+        args{9} = "cdata";
+        args{10} = [];
+      elseif (ndims (c) == 3 && size (c, 3) == 3)
+        ## CDATA is specified as 3-D RGB data
+        if ((rows (c) == 1 && columns (c) == 1) ...
+            || (rows (c) == 1 && columns (c) == columns (x)))
+          ## Single patch color or per-face color
+          args{7} = "facecolor";
+          args{8} = "flat";
+          args{9} = "cdata";
+          args{10} = c;
+        elseif (rows (c) == rows (x) && columns (c) == columns (x))
+          ## Per-vertex color
+          args{7} = "facecolor";
+          args{8} = "interp";
+          args{9} = "cdata";
+          args{10} = c;
+        else
+          error ("patch: invalid TrueColor data C");
+        endif
+      else
+        ## Color vectors
+        if (isempty (c))
+          args{7} = "facecolor";
+          args{8} = "interp";
           args{9} = "cdata";
           args{10} = [];
-        elseif (ndims (c) == 3 && size (c, 3) == 3)
-          ## CDATA is specified as RGB data
-          if ((rows (c) == 1 && columns (c) == 1) ...
-              || (rows (c) == 1 && columns (c) == columns (x)))
-            ## Single patch color or per-face color
-            args{7} = "facecolor";
-            args{8} = "flat";
-            args{9} = "cdata";
-            args{10} = c;
-          elseif (rows (c) == rows (x) && columns (c) == columns (x))
-            ## Per-vertex color
-            args{7} = "facecolor";
-            args{8} = "interp";
-            args{9} = "cdata";
-            args{10} = c;
-          else
-            error ("patch: Invalid TrueColor data C");
-          endif
+        elseif (size_equal (c, x) && size_equal (c, y))
+          args{7} = "facecolor";
+          args{8} = "interp";
+          args{9} = "cdata";
+          args{10} = c;
         else
-          ## Color Vectors
-          if (isempty (c))
-            args{7} = "facecolor";
-            args{8} = "interp";
-            args{9} = "cdata";
-            args{10} = [];
-          elseif (size_equal (c, x) && size_equal (c, y))
-            args{7} = "facecolor";
-            args{8} = "interp";
-            args{9} = "cdata";
-            args{10} = c;
-          else
-            error ("patch: size of X, Y, and C must be equal");
-          endif
+          error ("patch: size of X, Y, and C must be equal");
         endif
-      elseif (iscolorspec (c))
-        args{7} = "facecolor";
-        args{8} = tolower (c);
-        args{9} = "cdata";
-        args{10} = [];
-      else
-        args{7} = "facecolor";
-        args{8} = [0, 0, 0];
-        args{9} = "cdata";
-        args{10} = [];
       endif
+    elseif (iscolorspec (c))
+      ## Color specification is a string
+      args{7} = "facecolor";
+      args{8} = tolower (c);
+      args{9} = "cdata";
+      args{10} = [];
+    elseif (! isempty (c))
+      error ("patch: invalid color specification C");
+    endif
 
-      args = [args, varargin(iarg:end)];
-    endif
+    args = [args, varargin(iarg:end)];
+
   else
+    ## "Property"/Value pair input
     args = varargin;
   endif
 
-  if (! failed)
-    h = __go_patch__ (p, args{:});
+  if (isempty (p))
+    p = gca ();
   endif
+  h = __go_patch__ (p, args{:});
 
 endfunction