[changeset] - bug and fix for surfnorm()

[Ben Abbott]

On Oct 20, 2008, at 10:46 AM, John W. Eaton wrote:

> On 19-Oct-2008, Ben Abbott wrote:
>
> |
> | On Oct 18, 2008, at 6:19 PM, Ben Abbott wrote:
> |
> | > I noticed that the lines below produced multiple surfaces
> | >
> | > 	surfnorm (peaks (10))
> | > 	surf (peaks)
> | >
> | > The problem was with the "hold on" that was needed to add the  
> normal-
> | > vectors to the surface ... and no "hold off" followed.
> | >
> | > The attached changeset fixes that.
> | >
> | > Ben
> | >
> | > <changeset-surfnorm.patch>
> |
> | Dan,
> |
> | I've merged your changeset with mine.
> |
> | There are three differences between yours and mine. First you did a
> | nicer job of using the handle properties. Second, your placement
> | before the surf() command is more proper (imo). Third, I tried to
> | preserve the hold state for "oldh" as well. The one instance I can
> | imagine where that matters is when h==oldh and an error is  
> encountered
> | during the "unwind_protect".
> |
> | I've also added an extra demo which would illustrate that the hold
> | state was left on.
> |
> | I've attached the changeset credited to you.
>
> I made the following change:
>
>  # HG changeset patch
>  # User John W. Eaton <jwe at octave.org>
>  # Date 1224513720 14400
>  # Node ID 1e1e88bcc733f7069f8fbd88556a0a476e26ca5b
>  # Parent  5cfeb7bc497a663a3321b15c0055e16f95dc5606
>  surfnorm.m: save and restore hold state
>
>  diff --git a/scripts/ChangeLog b/scripts/ChangeLog
>  --- a/scripts/ChangeLog
>  +++ b/scripts/ChangeLog
>  @@ -1,3 +1,9 @@
>  +2008-10-20  John W. Eaton  <jwe at octave.org>
>  +
>  +       * plot/surfnorm.m: Save and restore hold state.
>  +       From Daniel J. Sebald <daniel.sebald at ieee.org> and
>  +       Ben Abbott <bpabbott at mac.com>.
>  +
>   2008-10-20  Ben Abbott  <bpabbott at mac.com>
>
> 	  * plot/__stem__.m: Respect new ordering of children when setting
>  diff --git a/scripts/plot/surfnorm.m b/scripts/plot/surfnorm.m
>  --- a/scripts/plot/surfnorm.m
>  +++ b/scripts/plot/surfnorm.m
>  @@ -114,11 +114,16 @@
> 	 axes (h);
> 	 newplot ();
> 	 surf (x, y, z, varargin{ioff:end});
>  -      hold on;
>  -      plot3 ([x(:)'; x(:).' + nx(:).' ; NaN(size(x(:).'))](:),
>  -            [y(:)'; y(:).' + ny(:).' ; NaN(size(y(:).'))](:),
>  -            [z(:)'; z(:).' + nz(:).' ; NaN(size(z(:).'))](:),
>  -            varargin{ioff:end});
>  +      old_hold_state = get (h, "nextplot");
>  +      unwind_protect
>  +       set (h, "nextplot", "add");
>  +       plot3 ([x(:)'; x(:).' + nx(:).' ; NaN(size(x(:).'))](:),
>  +              [y(:)'; y(:).' + ny(:).' ; NaN(size(y(:).'))](:),
>  +              [z(:)'; z(:).' + nz(:).' ; NaN(size(z(:).'))](:),
>  +              varargin{ioff:end});
>  +      unwind_protect_cleanup
>  +       set (h, "nextplot", old_hold_state);
>  +      end_unwind_protect
>       unwind_protect_cleanup
> 	 axes (oldh);
>       end_unwind_protect
>
> I don't see the point of saving and restoring the hold state of OLDH
> since we aren't changing the hold state on those axes.
>
> Also, note the use of unwind_protect here.  Without that, it would be
> possible for an intterupt to happen during the plot3 call that would
> leave the hold state for H set to "add".
>
> jwe

The reason I was concerned about the old state was for a special case  
where oldh==h and an error occurred after "hold on". In such an event,  
I thought it appropriate to restore oldh to its original hold state.

Does that make sense, or have I missed something that makes such an  
event impossible?

Ben