# HG changeset patch
# User Markus Mützel <markus.muetzel@gmx.de>
# Date 1504206656 -7200
#      Thu Aug 31 21:10:56 2017 +0200
# Node ID 5e35751f6c13391f10a40dd9e9ad6914b02109a0
# Parent  665507c3c29dad2180d65059af3fb1f49c2bd276
Auto-adjust limits that span 0 on logarithmic axes (bug #43606).

* graphics.in.h (update_[xyz]lim): Check limits on update.

* graphics.cc (get_axis_limits): Change Inf limits to tight limits. Check manual
limits on linear and logarithmic axes. Add some BIST tests.

* __axis_limits__.m: Remove check for Inf from high level function.

diff -r 665507c3c29d -r 5e35751f6c13 libinterp/corefcn/graphics.cc
--- a/libinterp/corefcn/graphics.cc	Thu Aug 31 10:30:01 2017 -0700
+++ b/libinterp/corefcn/graphics.cc	Thu Aug 31 21:10:56 2017 +0200
@@ -6975,7 +6975,7 @@
 Matrix
 axes::properties::get_axis_limits (double xmin, double xmax,
                                    double min_pos, double max_neg,
-                                   bool logscale)
+                                   const bool logscale)
 {
   Matrix retval;
 
@@ -6997,9 +6997,7 @@
               // FIXME: max_neg is needed for "loglog ([0 -Inf])"
               //        This is the *only* place where max_neg is needed.
               //        Is there another way?
-              retval = default_lim ();
-              retval(0) = std::pow (10., retval(0));
-              retval(1) = std::pow (10., retval(1));
+              retval = default_lim (logscale);
               return retval;
             }
           if (min_val <= 0)
@@ -7075,6 +7073,203 @@
 }
 
 void
+axes::properties::check_axis_limits (Matrix &limits, const Matrix kids,
+                                     const bool logscale, char &update_type)
+{
+  double min_val = octave::numeric_limits<double>::Inf ();
+  double max_val = -octave::numeric_limits<double>::Inf ();
+  double min_pos = octave::numeric_limits<double>::Inf ();
+  double max_neg = -octave::numeric_limits<double>::Inf ();
+  double eps = std::numeric_limits<double>::epsilon ();
+  bool do_update = false, have_children_limits = false;
+
+  // check whether we need to get children limits
+  if (! octave::math::isfinite (limits(0)) ||
+      ! octave::math::isfinite (limits(1)))
+    {
+      get_children_limits (min_val, max_val, min_pos, max_neg, kids,
+                           update_type);
+      have_children_limits = true;
+    }
+  if (! octave::math::isfinite (limits(0)))
+    {
+      limits(0) = min_val;
+      do_update = true;
+    }
+  if (! octave::math::isfinite (limits(1)))
+    {
+      limits(1) = max_val;
+      do_update = true;
+    }
+  if (limits(0) == 0 && limits(1) == 0)
+    {
+      limits = default_lim (logscale);
+      do_update = true;
+    }
+  // FIXME: maybe this test should also be relative?
+  else if (std::abs (limits(0) - limits(1)) < sqrt (eps))
+    {
+      if (logscale)
+        {
+          limits(0) = (limits(0) < 0 ? 10.0 * limits(0) : 0.1 * limits(0));
+          limits(1) = (limits(1) < 0 ? 0.1 * limits(1) : 10.0 * limits(1));
+        }
+      else
+        {
+          limits(0) -= 0.1 * std::abs (limits(0));
+          limits(1) += 0.1 * std::abs (limits(1));
+        }
+      do_update = true;
+    }
+
+  if (logscale && limits(0)*limits(1) <= 0)
+    {
+      if (! have_children_limits)
+        get_children_limits (min_val, max_val, min_pos, max_neg, kids,
+                             update_type);
+
+      if (limits(1) > 0)
+        {
+          warning_with_id ("octave:axis-non-positive-log-limits",
+                           "Non-positive limit for logarithmic axis ignored.\n");
+          if (octave::math::isfinite (min_pos))
+            limits(0) = min_pos;
+          else
+            limits(0) = 0.1 * limits(1);
+        }
+      else
+        {
+          warning_with_id ("octave:axis-non-negative-log-limits",
+                           "Non-negative limit for logarithmic axis ignored.\n");
+          if (octave::math::isfinite (max_neg))
+            limits(1) = max_neg;
+          else
+            limits(1) = 0.1 * limits(0);
+        }
+      // FIXME: maybe this test should also be relative?
+      if (std::abs (limits(0) - limits(1)) < sqrt (eps))
+        {
+          // Widen range when too small
+          if (limits(0) > 0)
+            {
+              limits(0) *= 0.9;
+              limits(1) *= 1.1;
+            }
+          else
+            {
+              limits(0) *= 1.1;
+              limits(1) *= 0.9;
+            }
+        }
+      do_update = true;
+    }
+
+  if (! do_update)
+    update_type = 0;
+
+}
+
+/*
+## Test validation of auto and manual axis limits
+%!test
+%! hf = figure ("visible", "off");
+%! unwind_protect
+%!   hax = axes ("parent", hf);
+%!   plot (0, pi);
+%!   assert (get (hax, "xlim"), [-1, 1]);
+%!   assert (get (hax, "xlimmode"), "auto");
+%!   assert (get (hax, "ylim"), [2.8, 3.5], 2*eps);
+%!   assert (get (hax, "ylimmode"), "auto");
+%!   set (hax, "xlim", [1, 1], "ylim", [0, 0]);
+%!   assert (get (hax, "xlim"), [0.9, 1.1]);
+%!   assert (get (hax, "xlimmode"), "manual");
+%!   assert (get (hax, "ylim"), [0, 1]);
+%!   assert (get (hax, "ylimmode"), "manual");
+%!   set (hax, "xlim", [-Inf, Inf], "ylim", [-Inf, Inf]);
+%!   ## Matlab does not update the properties
+%!   assert (get (hax, "xlim"), [0, 1]);
+%!   assert (get (hax, "ylim"), [0.9, 1.1]*pi, 2*eps);
+%! unwind_protect_cleanup
+%!   delete (hf);
+%! end_unwind_protect
+
+%!test
+%! hf = figure ("visible", "off");
+%! unwind_protect
+%!   hax = axes ("parent", hf);
+%!   plot ([0, exp(1)], [-pi, 3]);
+%!   assert (get (hax, "xlim"), [0, 3]);
+%!   assert (get (hax, "xlimmode"), "auto");
+%!   assert (get (hax, "ylim"), [-4, 3]);
+%!   assert (get (hax, "ylimmode"), "auto");
+%!   set (hax, "xlim", [-Inf, Inf], "ylim", [-Inf, Inf]);
+%!   ## Matlab does not update the properties but uses tight limits on screen
+%!   assert (get (hax, "xlim"), [0, exp(1)]);
+%!   assert (get (hax, "xlimmode"), "manual");
+%!   assert (get (hax, "ylim"), [-pi, 3]);
+%!   assert (get (hax, "ylimmode"), "manual");
+%! unwind_protect_cleanup
+%!   delete (hf);
+%! end_unwind_protect
+
+%!test
+%! hf = figure ("visible", "off");
+%! unwind_protect
+%!   hax = axes ("parent", hf);
+%!   loglog (0, pi);
+%!   assert (get (hax, "xlim"), [0.1, 1.0]);
+%!   assert (get (hax, "xlimmode"), "auto");
+%!   assert (get (hax, "ylim"), [1, 10]);
+%!   assert (get (hax, "ylimmode"), "auto");
+%!   set (hax, "xlim", [1, 1], "ylim", [0, 0]);
+%!   assert (get (hax, "xlim"), [0.1, 10]);
+%!   assert (get (hax, "xlimmode"), "manual");
+%!   assert (get (hax, "ylim"), [0.1, 1.0]);
+%!   assert (get (hax, "ylimmode"), "manual");
+%!   set (hax, "xlim", [-Inf, Inf], "ylim", [-Inf, Inf]);
+%!   ## Matlab does not update the properties
+%!   assert (get (hax, "xlim"), [0.1, 1.0]);
+%!   assert (get (hax, "ylim"), [0.1, 10]*pi);
+%! unwind_protect_cleanup
+%!   delete (hf);
+%! end_unwind_protect
+
+%!test
+%! hf = figure ("visible", "off");
+%! unwind_protect
+%!   hax = axes ("parent", hf);
+%!   loglog ([0 -1], [0 1]);
+%!   assert (get (hax, "xlim"), [-10, -0.1]);
+%!   assert (get (hax, "xlimmode"), "auto");
+%!   assert (get (hax, "ylim"), [0.1, 10]);
+%!   assert (get (hax, "ylimmode"), "auto");
+%!   set (hax, "xlim", [-Inf, Inf], "ylim", [-Inf, Inf]);
+%!   ## Matlab does not update the properties
+%!   assert (get (hax, "xlim"), [-1.1, -0.9]);
+%!   assert (get (hax, "ylim"), [0.9, 1.1]);
+%! unwind_protect_cleanup
+%!   delete (hf);
+%! end_unwind_protect
+
+%!test
+%! hf = figure ("visible", "off");
+%! unwind_protect
+%!   hax = axes ("parent", hf);
+%!   loglog ([1 -1], [1 pi]);
+%!   assert (get (hax, "xlim"), [0.1, 10]);
+%!   assert (get (hax, "xlimmode"), "auto");
+%!   assert (get (hax, "ylim"), [1, 10]);
+%!   assert (get (hax, "ylimmode"), "auto");
+%!   set (hax, "xlim", [-Inf, Inf], "ylim", [-Inf, Inf]);
+%!   ## Matlab does not update the properties but uses tight limits on screen
+%!   assert (get (hax, "xlim"), [0.9, 1.1]);
+%!   assert (get (hax, "ylim"), [1, pi]);
+%! unwind_protect_cleanup
+%!   delete (hf);
+%! end_unwind_protect
+*/
+
+void
 axes::properties::calc_ticks_and_lims (array_property& lims,
                                        array_property& ticks,
                                        array_property& mticks,
@@ -7520,47 +7715,54 @@
       || axis_type == "xlimmode" || axis_type == "xliminclude"
       || axis_type == "xlim")
     {
+      limits = xproperties.get_xlim ().matrix_value ();
+      FIX_LIMITS;
+
+      update_type = 'x';
       if (xproperties.xlimmode_is ("auto"))
         {
-          limits = xproperties.get_xlim ().matrix_value ();
-          FIX_LIMITS;
-
           get_children_limits (min_val, max_val, min_pos, max_neg, kids, 'x');
 
           limits = xproperties.get_axis_limits (min_val, max_val,
                                                 min_pos, max_neg,
                                                 xproperties.xscale_is ("log"));
-
-          update_type = 'x';
-        }
+        }
+      else
+        xproperties.check_axis_limits (limits, kids,
+                                       xproperties.xscale_is ("log"),
+                                       update_type);
     }
   else if (axis_type == "ydata" || axis_type == "yscale"
            || axis_type == "ylimmode" || axis_type == "yliminclude"
            || axis_type == "ylim")
     {
+      limits = xproperties.get_ylim ().matrix_value ();
+      FIX_LIMITS;
+
+      update_type = 'y';
       if (xproperties.ylimmode_is ("auto"))
         {
-          limits = xproperties.get_ylim ().matrix_value ();
-          FIX_LIMITS;
-
           get_children_limits (min_val, max_val, min_pos, max_neg, kids, 'y');
 
           limits = xproperties.get_axis_limits (min_val, max_val,
                                                 min_pos, max_neg,
                                                 xproperties.yscale_is ("log"));
-
-          update_type = 'y';
-        }
+        }
+      else
+        xproperties.check_axis_limits (limits, kids,
+                                       xproperties.yscale_is ("log"),
+                                       update_type);
     }
   else if (axis_type == "zdata" || axis_type == "zscale"
            || axis_type == "zlimmode" || axis_type == "zliminclude"
            || axis_type == "zlim")
     {
+      limits = xproperties.get_zlim ().matrix_value ();
+      FIX_LIMITS;
+
+      update_type = 'z';
       if (xproperties.zlimmode_is ("auto"))
         {
-          limits = xproperties.get_zlim ().matrix_value ();
-          FIX_LIMITS;
-
           get_children_limits (min_val, max_val, min_pos, max_neg, kids, 'z');
 
           xproperties.set_has3Dkids ((max_val - min_val) >
@@ -7569,8 +7771,6 @@
           limits = xproperties.get_axis_limits (min_val, max_val,
                                                 min_pos, max_neg,
                                                 xproperties.zscale_is ("log"));
-
-          update_type = 'z';
         }
       else
         {
@@ -7580,6 +7780,10 @@
 
           xproperties.set_has3Dkids ((max_val - min_val) >
                                       std::numeric_limits<double>::epsilon ());
+
+          xproperties.check_axis_limits (limits, kids,
+                                         xproperties.zscale_is ("log"),
+                                         update_type);
         }
     }
   else if (axis_type == "cdata" || axis_type == "climmode"
@@ -7648,24 +7852,31 @@
   frame.protect_var (updating_axis_limits);
 
   updating_axis_limits.insert (get_handle ().value ());
+  bool is_auto;
 
   switch (update_type)
     {
     case 'x':
+      is_auto = xproperties.xlimmode_is ("auto");
       xproperties.set_xlim (limits);
-      xproperties.set_xlimmode ("auto");
+      if (is_auto)
+        xproperties.set_xlimmode ("auto");
       xproperties.update_xlim ();
       break;
 
     case 'y':
+      is_auto = xproperties.ylimmode_is ("auto");
       xproperties.set_ylim (limits);
-      xproperties.set_ylimmode ("auto");
+      if (is_auto)
+        xproperties.set_ylimmode ("auto");
       xproperties.update_ylim ();
       break;
 
     case 'z':
+      is_auto = xproperties.zlimmode_is ("auto");
       xproperties.set_zlim (limits);
-      xproperties.set_zlimmode ("auto");
+      if (is_auto)
+        xproperties.set_zlimmode ("auto");
       xproperties.update_zlim ();
       break;
 
@@ -7710,6 +7921,7 @@
       || axis_type == "xlimmode" || axis_type == "xliminclude"
       || axis_type == "xlim")
     {
+      update_type = 'x';
       if (xproperties.xlimmode_is ("auto"))
         {
           get_children_limits (min_val, max_val, min_pos, max_neg, kids, 'x');
@@ -7717,14 +7929,20 @@
           limits = xproperties.get_axis_limits (min_val, max_val,
                                                 min_pos, max_neg,
                                                 xproperties.xscale_is ("log"));
-
-          update_type = 'x';
+        }
+      else
+        {
+          limits = xproperties.get_xlim ().matrix_value ();
+          xproperties.check_axis_limits (limits, kids,
+                                         xproperties.xscale_is ("log"),
+                                         update_type);
         }
     }
   else if (axis_type == "ydata" || axis_type == "yscale"
            || axis_type == "ylimmode" || axis_type == "yliminclude"
            || axis_type == "ylim")
     {
+      update_type = 'y';
       if (xproperties.ylimmode_is ("auto"))
         {
           get_children_limits (min_val, max_val, min_pos, max_neg, kids, 'y');
@@ -7732,26 +7950,31 @@
           limits = xproperties.get_axis_limits (min_val, max_val,
                                                 min_pos, max_neg,
                                                 xproperties.yscale_is ("log"));
-
-          update_type = 'y';
+        }
+      else
+        {
+          limits = xproperties.get_ylim ().matrix_value ();
+          xproperties.check_axis_limits (limits, kids,
+                                         xproperties.yscale_is ("log"),
+                                         update_type);
         }
     }
   else if (axis_type == "zdata" || axis_type == "zscale"
            || axis_type == "zlimmode" || axis_type == "zliminclude"
            || axis_type == "zlim")
     {
+      update_type = 'z';
       if (xproperties.zlimmode_is ("auto"))
         {
           get_children_limits (min_val, max_val, min_pos, max_neg, kids, 'z');
 
           xproperties.set_has3Dkids ((max_val - min_val) >
-                                      std::numeric_limits<double>::epsilon ());
+                                     std::numeric_limits<double>::epsilon ());
+
 
           limits = xproperties.get_axis_limits (min_val, max_val,
                                                 min_pos, max_neg,
                                                 xproperties.zscale_is ("log"));
-
-          update_type = 'z';
         }
       else
         {
@@ -7760,7 +7983,12 @@
           get_children_limits (min_val, max_val, min_pos, max_neg, kids, 'z');
 
           xproperties.set_has3Dkids ((max_val - min_val) >
-                                      std::numeric_limits<double>::epsilon ());
+                                     std::numeric_limits<double>::epsilon ());
+
+          limits = xproperties.get_zlim ().matrix_value ();
+          xproperties.check_axis_limits (limits, kids,
+                                         xproperties.zscale_is ("log"),
+                                         update_type);
         }
     }
   else if (axis_type == "cdata" || axis_type == "climmode"
@@ -7821,24 +8049,31 @@
   frame.protect_var (updating_axis_limits);
 
   updating_axis_limits.insert (get_handle ().value ());
+  bool is_auto;
 
   switch (update_type)
     {
     case 'x':
+      is_auto = xproperties.xlimmode_is ("auto");
       xproperties.set_xlim (limits);
-      xproperties.set_xlimmode ("auto");
+      if (is_auto)
+        xproperties.set_xlimmode ("auto");
       xproperties.update_xlim ();
       break;
 
     case 'y':
+      is_auto = xproperties.ylimmode_is ("auto");
       xproperties.set_ylim (limits);
-      xproperties.set_ylimmode ("auto");
+      if (is_auto)
+        xproperties.set_ylimmode ("auto");
       xproperties.update_ylim ();
       break;
 
     case 'z':
+      is_auto = xproperties.zlimmode_is ("auto");
       xproperties.set_zlim (limits);
-      xproperties.set_zlimmode ("auto");
+      if (is_auto)
+        xproperties.set_zlimmode ("auto");
       xproperties.update_zlim ();
       break;
 
diff -r 665507c3c29d -r 5e35751f6c13 libinterp/corefcn/graphics.in.h
--- a/libinterp/corefcn/graphics.in.h	Thu Aug 31 10:30:01 2017 -0700
+++ b/libinterp/corefcn/graphics.in.h	Thu Aug 31 21:10:56 2017 +0200
@@ -4093,10 +4093,15 @@
   public:
     Matrix get_axis_limits (double xmin, double xmax,
                             double min_pos, double max_neg,
-                            bool logscale);
+                            const bool logscale);
+
+    void check_axis_limits (Matrix &limits, const Matrix kids,
+                            const bool logscale, char &update_type);
 
     void update_xlim ()
     {
+      update_axis_limits ("xlim");
+
       calc_ticks_and_lims (xlim, xtick, xminortickvalues, xlimmode.is ("auto"),
                            xtickmode.is ("auto"), xscale.is ("log"));
       if (xticklabelmode.is ("auto"))
@@ -4116,6 +4121,8 @@
 
     void update_ylim (void)
     {
+      update_axis_limits ("ylim");
+
       calc_ticks_and_lims (ylim, ytick, yminortickvalues, ylimmode.is ("auto"),
                            ytickmode.is ("auto"), yscale.is ("log"));
       if (yticklabelmode.is ("auto"))
@@ -4135,6 +4142,8 @@
 
     void update_zlim (void)
     {
+      update_axis_limits ("zlim");
+
       calc_ticks_and_lims (zlim, ztick, zminortickvalues, zlimmode.is ("auto"),
                            ztickmode.is ("auto"), zscale.is ("log"));
       if (zticklabelmode.is ("auto"))
diff -r 665507c3c29d -r 5e35751f6c13 scripts/plot/appearance/private/__axis_limits__.m
--- a/scripts/plot/appearance/private/__axis_limits__.m	Thu Aug 31 10:30:01 2017 -0700
+++ b/scripts/plot/appearance/private/__axis_limits__.m	Thu Aug 31 21:10:56 2017 +0200
@@ -44,18 +44,10 @@
     else
       if (! isnumeric (arg) && any (size (arg(:)) != [2, 1]))
         error ("%s: LIMITS must be a 2-element vector", fcn);
-      else
-        if (arg(1) >= arg(2))
-          error ("%s: axis limits must be increasing", fcn);
-        else
-          autoscale = isinf (arg);
-          if (any (autoscale))
-            set (hax, fcnmode, "auto");
-            arg(autoscale) = get (hax, fcn)(autoscale);
-          endif
-          set (hax, fcn, arg(:));
-        endif
+      elseif (arg(1) >= arg(2))
+        error ("%s: axis limits must be increasing", fcn);
       endif
+      set (hax, fcn, arg(:));
     endif
   endif
 
