Fix Wand Iteration and Insert/Add handling

diff --git a/MagickWand/magick-image.c b/MagickWand/magick-image.c
index 3a0fb6842fb00bf008adda036f154ed64230c2f2..526214d671615e4efff2511daa69f6fcac084130 100644 (file)
--- a/MagickWand/magick-image.c
+++ b/MagickWand/magick-image.c
@@ -375,8 +375,16 @@ WandExport MagickBooleanType MagickAdaptiveThresholdImage(MagickWand *wand,
 %  MagickAddImage() adds a clone of the images in the second wand and
 %  inserts them into the first wand, at the current image location.
 %
-%  Use MagickSetFirstIterator(), to insert new images before all the current
-%  images in the wand, otherwise image is placed after the current image.
+%  Use MagickSetLastIterator(), to append new images into an existing wand,
+%  current image will be set to last image so later adds with also be
+%  appened to end of wand.
+%
+%  Use MagickSetFirstIterator() to prepend new images into wand. Later images
+%  added will also be prepended.
+%
+%  Otherwise the new images will be inserted just after the current image,
+%  and later image will also be added after current image but before the
+%  just added images.
 %
 %  The format of the MagickAddImage method is:
 %
@@ -387,65 +395,44 @@ WandExport MagickBooleanType MagickAdaptiveThresholdImage(MagickWand *wand,
 %
 %    o wand: the magick wand.
 %
-%    o add_wand: A wand that contains images to add at the current image
-%      location.
+%    o add_wand: A wand that contains image list to be added
 %
 */
 
 static inline MagickBooleanType InsertImageInWand(MagickWand *wand,
   Image *images)
 {
-  Image
-    *current;
-
-  current=wand->images;  /* note the current image */
-
-  /* if no images in wand, just add them and set first image as current */
-  if (current == (Image *) NULL)
+  /* if no images in wand, just add them, set wand->images as appropriate */
+  if (wand->images == (Image *) NULL)
     {
-      wand->images=GetFirstImageInList(images);
+      if (wand->insert_before != MagickFalse)
+        wand->images=GetFirstImageInList(images);
+      else
+        wand->images=GetLastImageInList(images);
       return(MagickTrue);
     }
 
   /* user jumped to first image, so prepend new images - remain active */
-  if ((wand->set_first != MagickFalse) &&
-       (current->previous == (Image *) NULL) )
+  if ((wand->insert_before != MagickFalse) &&
+       (wand->images->previous == (Image *) NULL) )
     {
-      PrependImageToList(&current,images);
+      PrependImageToList(&wand->images,images);
       wand->images=GetFirstImageInList(images);
       return(MagickTrue);
     }
-  wand->set_first = MagickFalse; /* flag no longer valid */
-
-  /* Current image was flagged as 'pending' iterative processing. */
-  if (wand->image_pending != MagickFalse)
-    {
-      /* current pending image is the last, append new images */
-      if (current->next == (Image *) NULL)
-        {
-          AppendImageToList(&current,images);
-          wand->images=GetLastImageInList(images);
-          return(MagickTrue);
-        }
-      /* current pending image is the first image, prepend it */
-      if (current->previous == (Image *) NULL)
-        {
-          PrependImageToList(&current,images);
-          wand->images=GetFirstImageInList(images);
-          return(MagickTrue);
-        }
-    }
+  /* Note you should never have 'insert_before' true when wand->images image
+     is not the first image in the wand!  That is no insert before
+     wand->images image, only after current image */
 
   /* if at last image append new images */
-  if (current->next == (Image *) NULL)
+  if (wand->images->next == (Image *) NULL)
     {
-      InsertImageInList(&current,images);
+      InsertImageInList(&wand->images,images);
       wand->images=GetLastImageInList(images);
       return(MagickTrue);
     }
-  /* otherwise just insert image, just after the current image */
-  InsertImageInList(&current,images);
-  wand->images=GetFirstImageInList(images);
+  /* otherwise insert new images, just after the wand->images image */
+  InsertImageInList(&wand->images,images);
   return(MagickTrue);
 }
 
@@ -6845,9 +6832,13 @@ WandExport MagickBooleanType MagickNewImage(MagickWand *wand,const size_t width,
 %                                                                             %
 %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 %
-%  MagickNextImage() associates the next image in the image list with a magick
-%  wand.  It returns true if the it succeeds, meaning the current image is the
-%  next image to be iterated over.
+%  MagickNextImage() sets the next image in the wand as the current image.
+%  It returns MagickTrue if their is another image to be processed.
+%  
+%  Returns MagickFalse when current image is already the last image
+%  in the wand (no more next images), at whcih point you can use
+%  MagickPreviousImage() to again iterate over the images in the reverse
+%  direction, starting with the last image (again).
 %
 %  The format of the MagickNextImage method is:
 %
@@ -6866,16 +6857,17 @@ WandExport MagickBooleanType MagickNextImage(MagickWand *wand)
     (void) LogMagickEvent(WandEvent,GetMagickModule(),"%s",wand->name);
   if (wand->images == (Image *) NULL)
     ThrowWandException(WandError,"ContainsNoImages",wand->name);
-  /* If current image is 'pending' just return true.  */
+  wand->insert_before=MagickFalse; /* Inserts is now appended */
   if (wand->image_pending != MagickFalse)
     {
       wand->image_pending=MagickFalse;
       return(MagickTrue);
     }
-  /* If there is no next image, (Iterator is finished) */
   if (GetNextImageInList(wand->images) == (Image *) NULL)
+    {
+      wand->image_pending=MagickTrue; /* No image, PreviousImage re-gets */
       return(MagickFalse);
-  /* just move to next image - current image is not 'pending' */
+    }
   wand->images=GetNextImageInList(wand->images);
   return(MagickTrue);
 }
@@ -7445,6 +7437,11 @@ WandExport MagickWand *MagickPreviewImages(MagickWand *wand,
 %  MagickPreviousImage() assocates the previous image in an image list with
 %  the magick wand.
 %
+%  Returns MagickFalse when current image is the first image (no more previous
+%  images). The Iterator is than reset (as per MagickResetIterator()) ready
+%  to again process images in the forward direction, again starting with the
+%  first image in list. Images added at this point are prepended.
+%
 %  The format of the MagickPreviousImage method is:
 %
 %      MagickBooleanType MagickPreviousImage(MagickWand *wand)
@@ -7463,11 +7460,17 @@ WandExport MagickBooleanType MagickPreviousImage(MagickWand *wand)
   if (wand->images == (Image *) NULL)
     ThrowWandException(WandError,"ContainsNoImages",wand->name);
 
-  wand->image_pending=MagickFalse;  /* pending status has no meaning */
-  /* If there is no prev image, return false (Iterator is finished) */
+  if (wand->image_pending != MagickFalse)
+    {
+      wand->image_pending=MagickFalse;  /* image returned no longer pending */
+      return(MagickTrue);
+    }
   if (GetPreviousImageInList(wand->images) == (Image *) NULL)
+    {
+      wand->image_pending=MagickTrue;   /* Next now re-gets first image */
+      wand->insert_before=MagickTrue;   /* insert/add prepends new images */
       return(MagickFalse);
-  /* just do it - current image is not 'pending' */
+    }
   wand->images=GetPreviousImageInList(wand->images);
   return(MagickTrue);
 }

diff --git a/MagickWand/magick-wand-private.h b/MagickWand/magick-wand-private.h
index 96d38ad5ab070f6333aea3e5f009e38d250e7630..af4c45be1956449896392f3a2ca4b9f2431e9d0f 100644 (file)
--- a/MagickWand/magick-wand-private.h
+++ b/MagickWand/magick-wand-private.h
@@ -47,8 +47,8 @@ struct _MagickWand
     *exception;
 
   MagickBooleanType
-    set_first,        /* wand set to first image, prepend new images */
-    image_pending,    /* this image is pending Next Iteration */
+    insert_before,    /* wand set to first image, prepend new images */
+    image_pending,    /* this image is pending Next/Previous Iteration */
     debug;            /* Log calls to MagickWand library */
 
   size_t

diff --git a/MagickWand/magick-wand.c b/MagickWand/magick-wand.c
index eac8fe1412d85759a2125fa6251e8b0bdd3c3d88..2fd8ce15f26b370f7638e36159cce6dab4810fc3 100644 (file)
--- a/MagickWand/magick-wand.c
+++ b/MagickWand/magick-wand.c
@@ -825,8 +825,8 @@ WandExport void MagickResetIterator(MagickWand *wand)
   if (wand->debug != MagickFalse)
     (void) LogMagickEvent(WandEvent,GetMagickModule(),"%s",wand->name);
   wand->images=GetFirstImageInList(wand->images);
-  wand->set_first=MagickFalse;    /* we did not jump to the first image */
-  wand->image_pending=MagickTrue; /* pointed image is the 'next' image */
+  wand->insert_before=MagickFalse; /* Insert/add after current (first) image */
+  wand->image_pending=MagickTrue;  /* NextImage will remain this image */
 }
 \f
 /*
@@ -843,8 +843,8 @@ WandExport void MagickResetIterator(MagickWand *wand)
 %  MagickSetFirstIterator() sets the wand iterator to the first image.
 %
 %  Flags are set to point not only to the 'next' image to be processed,
-%  but also define where InsertImageInWand() (such as from MagickReadImage())
-%  should instert new images.
+%  but also sets that MagickAddImage() (such as from MagickReadImage())
+%  should add prepend images.
 %
 %  The format of the MagickSetFirstIterator method is:
 %
@@ -862,8 +862,8 @@ WandExport void MagickSetFirstIterator(MagickWand *wand)
   if (wand->debug != MagickFalse)
     (void) LogMagickEvent(WandEvent,GetMagickModule(),"%s",wand->name);
   wand->images=GetFirstImageInList(wand->images);
-  wand->set_first=MagickTrue;       /* we did jumped to the first image */
-  wand->image_pending=MagickFalse;  /* but we are not iterating */
+  wand->insert_before=MagickTrue;   /* Insert/add before the first image */
+  wand->image_pending=MagickFalse;  /* NextImage will set next image */
 }
 \f
 /*
@@ -912,8 +912,8 @@ WandExport MagickBooleanType MagickSetIteratorIndex(MagickWand *wand,
       return(MagickFalse);
     }
   wand->images=image;
-  wand->set_first=MagickFalse;     /* we are not at very start of list */
-  wand->image_pending=MagickFalse;  /* but we are not directly iterating */
+  wand->insert_before=MagickFalse;  /* Insert/Add after (this) image */
+  wand->image_pending=MagickFalse;  /* NextImage will set next image */
   return(MagickTrue);
 }
 /*
@@ -949,8 +949,8 @@ WandExport void MagickSetLastIterator(MagickWand *wand)
   if (wand->debug != MagickFalse)
     (void) LogMagickEvent(WandEvent,GetMagickModule(),"%s",wand->name);
   wand->images=GetLastImageInList(wand->images);
-  wand->set_first=MagickFalse;     /* we are not at very start of list */
-  wand->image_pending=MagickFalse;  /* but we are not iterating */
+  wand->insert_before=MagickFalse;  /* Insert/add after current (last) image */
+  wand->image_pending=MagickFalse;  /* PreviousImage will set previous image */
 }
 \f
 /*