Commit fe6ee9d1 authored by Garry R. Osgood's avatar Garry R. Osgood Committed by Garry R. Osgood

app/gimpimage.c app/undo.c app/undo_types.h docs/undo.txt Changed sanity

1999-12-10 Garry R. Osgood <gosgood@idt.net>
	* app/gimpimage.c
	* app/undo.c
        * app/undo_types.h
        * docs/undo.txt
        Changed sanity checks in undo_pop/free_layer_mask() to LAYER_MASK_ADD_UNDO
	and LAYER_MASK_REMOVE_UNDO to be consistent with undo_push_layer_mask()
	These now nvoke proper cleanup and release of GimpLayerMasks.
	* docs/undo.txt: New file, an overview of undo logic written by
	Austin Donnelly
	* app/undo.c
	* app/undo_types.h
	* app/gimpimage.c : Introduced a new UndoType, UNDO_NULL, which maps
	to zero, introducing that value into the enumerated types. Use the
        type to signal type unknown/error/untyped conditions.

	Full patch documentation at
	http://idt.net/~gosgood/gimp-patch/patch02.html
parent 4f1e315a
1999-12-10 Garry R. Osgood <gosgood@idt.net>
* app/undo.c : An inadvertent substitution of UndoTypes LAYER_ADD_UNDO
and LAYER_REMOVE_UNDO in undo_pop_layer_mask() and undo_free_layer_mask()
sanity checks prevented the proper disposal of GimpLayerMasks and
associated tile managers and tiles. Changed to LAYER_MASK_ADD_UNDO
and LAYER_MASK_REMOVE_UNDO to be consistent with the undo_push_layer_mask()
function and to invoke proper cleanup and release of retiring alpha layer masks.
* docs/undo.txt: New file, an overview of undo logic written by
Austin Donnelly so that I could write this change log entry with
panache and flair. ;)
* app/undo.c
* app/undo_types.h
* app/gimpimage.c : Introduced a new UndoType, UNDO_NULL, which maps
to zero, introducing that value into the enumerated type and preventing
strict ANSI compilers from complaining about mixing enumerated and
unenumerated types. Use the type to signal type unknown/error/untyped
conditions.
Full patch documentation at http://idt.net/~gosgood/gimp-patch/patch02.html
1999-12-08 Michael Natterer <mitch@gimp.org>
* plug-ins/common/jpeg.c: The plugin allocated memory chunks of
......
......@@ -181,7 +181,7 @@ gimp_image_init (GimpImage *gimage)
gimage->undo_bytes = 0;
gimage->undo_levels = 0;
gimage->group_count = 0;
gimage->pushing_undo_group = 0;
gimage->pushing_undo_group = UNDO_NULL;
gimage->comp_preview_valid[0] = FALSE;
gimage->comp_preview_valid[1] = FALSE;
gimage->comp_preview_valid[2] = FALSE;
......
......@@ -181,7 +181,7 @@ gimp_image_init (GimpImage *gimage)
gimage->undo_bytes = 0;
gimage->undo_levels = 0;
gimage->group_count = 0;
gimage->pushing_undo_group = 0;
gimage->pushing_undo_group = UNDO_NULL;
gimage->comp_preview_valid[0] = FALSE;
gimage->comp_preview_valid[1] = FALSE;
gimage->comp_preview_valid[2] = FALSE;
......
......@@ -181,7 +181,7 @@ gimp_image_init (GimpImage *gimage)
gimage->undo_bytes = 0;
gimage->undo_levels = 0;
gimage->group_count = 0;
gimage->pushing_undo_group = 0;
gimage->pushing_undo_group = UNDO_NULL;
gimage->comp_preview_valid[0] = FALSE;
gimage->comp_preview_valid[1] = FALSE;
gimage->comp_preview_valid[2] = FALSE;
......
......@@ -181,7 +181,7 @@ gimp_image_init (GimpImage *gimage)
gimage->undo_bytes = 0;
gimage->undo_levels = 0;
gimage->group_count = 0;
gimage->pushing_undo_group = 0;
gimage->pushing_undo_group = UNDO_NULL;
gimage->comp_preview_valid[0] = FALSE;
gimage->comp_preview_valid[1] = FALSE;
gimage->comp_preview_valid[2] = FALSE;
......
......@@ -181,7 +181,7 @@ gimp_image_init (GimpImage *gimage)
gimage->undo_bytes = 0;
gimage->undo_levels = 0;
gimage->group_count = 0;
gimage->pushing_undo_group = 0;
gimage->pushing_undo_group = UNDO_NULL;
gimage->comp_preview_valid[0] = FALSE;
gimage->comp_preview_valid[1] = FALSE;
gimage->comp_preview_valid[2] = FALSE;
......
......@@ -316,7 +316,7 @@ undo_push (GImage *gimage,
gimage->dirty = 10000;
}
if (!gimage->pushing_undo_group)
if (gimage->pushing_undo_group ==UNDO_NULL )
if (! undo_free_up_space (gimage))
return NULL;
......@@ -325,7 +325,7 @@ undo_push (GImage *gimage,
gimage->undo_bytes += size;
/* only increment levels if not in a group */
if (!gimage->pushing_undo_group)
if (gimage->pushing_undo_group == UNDO_NULL)
gimage->undo_levels ++;
gimage->undo_stack = g_slist_prepend (gimage->undo_stack, (void *) new);
......@@ -333,7 +333,7 @@ undo_push (GImage *gimage,
/* lastly, tell people about the newly pushed undo (must come after
* modification of undo_stack). */
if (!gimage->pushing_undo_group)
if (gimage->pushing_undo_group == UNDO_NULL)
gimp_image_undo_event (gimage, UNDO_PUSHED);
return new;
......@@ -456,7 +456,7 @@ undo_pop (GImage *gimage)
* leave unbalanced group start marker earlier in the stack too,
* causing much confusion when it's later reached and
* mis-interpreted as a group start. */
g_return_val_if_fail (gimage->pushing_undo_group == 0, FALSE);
g_return_val_if_fail (gimage->pushing_undo_group == UNDO_NULL, FALSE);
return pop_stack (gimage, &gimage->undo_stack, &gimage->redo_stack, UNDO);
}
......@@ -466,7 +466,7 @@ int
undo_redo (GImage *gimage)
{
/* ditto for redo stack */
g_return_val_if_fail (gimage->pushing_undo_group == 0, FALSE);
g_return_val_if_fail (gimage->pushing_undo_group == UNDO_NULL, FALSE);
return pop_stack (gimage, &gimage->redo_stack, &gimage->undo_stack, REDO);
}
......@@ -501,7 +501,7 @@ undo_get_topitem_type (GSList *stack)
Undo *object;
if (!stack)
return 0;
return UNDO_NULL;
object = stack->data;
......@@ -529,11 +529,11 @@ undo_get_undo_name (GImage *gimage)
UndoType
undo_get_undo_top_type (GImage *gimage)
{
g_return_val_if_fail (gimage != NULL, 0);
g_return_val_if_fail (gimage != NULL, UNDO_NULL);
/* don't want to encourage undo while a group is open */
if (gimage->pushing_undo_group != 0)
return 0;
if (gimage->pushing_undo_group != UNDO_NULL)
return UNDO_NULL;
return undo_get_topitem_type (gimage->undo_stack);
}
......@@ -545,7 +545,7 @@ undo_get_redo_name (GImage *gimage)
g_return_val_if_fail (gimage != NULL, NULL);
/* don't want to encourage redo while a group is open */
if (gimage->pushing_undo_group != 0)
if (gimage->pushing_undo_group != UNDO_NULL)
return NULL;
return undo_get_topitem_name (gimage->redo_stack);
......@@ -594,7 +594,7 @@ undo_map_over_undo_stack (GImage *gimage,
void *data)
{
/* shouldn't have group open */
g_return_if_fail (gimage->pushing_undo_group == 0);
g_return_if_fail (gimage->pushing_undo_group == UNDO_NULL);
undo_map_over_stack (gimage->undo_stack, fn, data);
}
......@@ -604,7 +604,7 @@ undo_map_over_redo_stack (GImage *gimage,
void *data)
{
/* shouldn't have group open */
g_return_if_fail (gimage->pushing_undo_group == 0);
g_return_if_fail (gimage->pushing_undo_group == UNDO_NULL);
undo_map_over_stack (gimage->redo_stack, fn, data);
}
......@@ -707,7 +707,7 @@ undo_push_group_end (GImage *gimage)
boundary_marker);
gimage->undo_bytes += boundary_marker->bytes;
gimage->pushing_undo_group = 0;
gimage->pushing_undo_group = UNDO_NULL;
/* Do it here, since undo_push doesn't emit this event while in the
* middle of a group */
......@@ -1637,8 +1637,8 @@ undo_pop_layer_mask (GImage *gimage,
lmu = (LayerMaskUndo *) lmu_ptr;
/* remove layer mask */
if ((state == UNDO && type == LAYER_ADD_UNDO) ||
(state == REDO && type == LAYER_REMOVE_UNDO))
if ((state == UNDO && type == LAYER_MASK_ADD_UNDO) ||
(state == REDO && type == LAYER_MASK_REMOVE_UNDO))
{
/* remove the layer mask */
lmu->layer->mask = NULL;
......@@ -1690,8 +1690,8 @@ undo_free_layer_mask (UndoState state,
* stack and it's a layer add, or if we're freeing from
* the undo stack and it's a layer remove
*/
if ((state == REDO && type == LAYER_ADD_UNDO) ||
(state == UNDO && type == LAYER_REMOVE_UNDO))
if ((state == REDO && type == LAYER_MASK_ADD_UNDO) ||
(state == UNDO && type == LAYER_MASK_REMOVE_UNDO))
layer_mask_delete (lmu->mask);
g_free (lmu);
......@@ -2852,7 +2852,7 @@ static struct undo_name_t {
UndoType type;
const char *name;
} undo_name[] = {
{0, N_("<<invalid>>")},
{UNDO_NULL, N_("<<invalid>>")},
{IMAGE_UNDO, N_("image")},
{IMAGE_MOD_UNDO, N_("image mod")},
{MASK_UNDO, N_("mask")},
......
......@@ -181,7 +181,7 @@ gimp_image_init (GimpImage *gimage)
gimage->undo_bytes = 0;
gimage->undo_levels = 0;
gimage->group_count = 0;
gimage->pushing_undo_group = 0;
gimage->pushing_undo_group = UNDO_NULL;
gimage->comp_preview_valid[0] = FALSE;
gimage->comp_preview_valid[1] = FALSE;
gimage->comp_preview_valid[2] = FALSE;
......
......@@ -181,7 +181,7 @@ gimp_image_init (GimpImage *gimage)
gimage->undo_bytes = 0;
gimage->undo_levels = 0;
gimage->group_count = 0;
gimage->pushing_undo_group = 0;
gimage->pushing_undo_group = UNDO_NULL;
gimage->comp_preview_valid[0] = FALSE;
gimage->comp_preview_valid[1] = FALSE;
gimage->comp_preview_valid[2] = FALSE;
......
......@@ -181,7 +181,7 @@ gimp_image_init (GimpImage *gimage)
gimage->undo_bytes = 0;
gimage->undo_levels = 0;
gimage->group_count = 0;
gimage->pushing_undo_group = 0;
gimage->pushing_undo_group = UNDO_NULL;
gimage->comp_preview_valid[0] = FALSE;
gimage->comp_preview_valid[1] = FALSE;
gimage->comp_preview_valid[2] = FALSE;
......
......@@ -316,7 +316,7 @@ undo_push (GImage *gimage,
gimage->dirty = 10000;
}
if (!gimage->pushing_undo_group)
if (gimage->pushing_undo_group ==UNDO_NULL )
if (! undo_free_up_space (gimage))
return NULL;
......@@ -325,7 +325,7 @@ undo_push (GImage *gimage,
gimage->undo_bytes += size;
/* only increment levels if not in a group */
if (!gimage->pushing_undo_group)
if (gimage->pushing_undo_group == UNDO_NULL)
gimage->undo_levels ++;
gimage->undo_stack = g_slist_prepend (gimage->undo_stack, (void *) new);
......@@ -333,7 +333,7 @@ undo_push (GImage *gimage,
/* lastly, tell people about the newly pushed undo (must come after
* modification of undo_stack). */
if (!gimage->pushing_undo_group)
if (gimage->pushing_undo_group == UNDO_NULL)
gimp_image_undo_event (gimage, UNDO_PUSHED);
return new;
......@@ -456,7 +456,7 @@ undo_pop (GImage *gimage)
* leave unbalanced group start marker earlier in the stack too,
* causing much confusion when it's later reached and
* mis-interpreted as a group start. */
g_return_val_if_fail (gimage->pushing_undo_group == 0, FALSE);
g_return_val_if_fail (gimage->pushing_undo_group == UNDO_NULL, FALSE);
return pop_stack (gimage, &gimage->undo_stack, &gimage->redo_stack, UNDO);
}
......@@ -466,7 +466,7 @@ int
undo_redo (GImage *gimage)
{
/* ditto for redo stack */
g_return_val_if_fail (gimage->pushing_undo_group == 0, FALSE);
g_return_val_if_fail (gimage->pushing_undo_group == UNDO_NULL, FALSE);
return pop_stack (gimage, &gimage->redo_stack, &gimage->undo_stack, REDO);
}
......@@ -501,7 +501,7 @@ undo_get_topitem_type (GSList *stack)
Undo *object;
if (!stack)
return 0;
return UNDO_NULL;
object = stack->data;
......@@ -529,11 +529,11 @@ undo_get_undo_name (GImage *gimage)
UndoType
undo_get_undo_top_type (GImage *gimage)
{
g_return_val_if_fail (gimage != NULL, 0);
g_return_val_if_fail (gimage != NULL, UNDO_NULL);
/* don't want to encourage undo while a group is open */
if (gimage->pushing_undo_group != 0)
return 0;
if (gimage->pushing_undo_group != UNDO_NULL)
return UNDO_NULL;
return undo_get_topitem_type (gimage->undo_stack);
}
......@@ -545,7 +545,7 @@ undo_get_redo_name (GImage *gimage)
g_return_val_if_fail (gimage != NULL, NULL);
/* don't want to encourage redo while a group is open */
if (gimage->pushing_undo_group != 0)
if (gimage->pushing_undo_group != UNDO_NULL)
return NULL;
return undo_get_topitem_name (gimage->redo_stack);
......@@ -594,7 +594,7 @@ undo_map_over_undo_stack (GImage *gimage,
void *data)
{
/* shouldn't have group open */
g_return_if_fail (gimage->pushing_undo_group == 0);
g_return_if_fail (gimage->pushing_undo_group == UNDO_NULL);
undo_map_over_stack (gimage->undo_stack, fn, data);
}
......@@ -604,7 +604,7 @@ undo_map_over_redo_stack (GImage *gimage,
void *data)
{
/* shouldn't have group open */
g_return_if_fail (gimage->pushing_undo_group == 0);
g_return_if_fail (gimage->pushing_undo_group == UNDO_NULL);
undo_map_over_stack (gimage->redo_stack, fn, data);
}
......@@ -707,7 +707,7 @@ undo_push_group_end (GImage *gimage)
boundary_marker);
gimage->undo_bytes += boundary_marker->bytes;
gimage->pushing_undo_group = 0;
gimage->pushing_undo_group = UNDO_NULL;
/* Do it here, since undo_push doesn't emit this event while in the
* middle of a group */
......@@ -1637,8 +1637,8 @@ undo_pop_layer_mask (GImage *gimage,
lmu = (LayerMaskUndo *) lmu_ptr;
/* remove layer mask */
if ((state == UNDO && type == LAYER_ADD_UNDO) ||
(state == REDO && type == LAYER_REMOVE_UNDO))
if ((state == UNDO && type == LAYER_MASK_ADD_UNDO) ||
(state == REDO && type == LAYER_MASK_REMOVE_UNDO))
{
/* remove the layer mask */
lmu->layer->mask = NULL;
......@@ -1690,8 +1690,8 @@ undo_free_layer_mask (UndoState state,
* stack and it's a layer add, or if we're freeing from
* the undo stack and it's a layer remove
*/
if ((state == REDO && type == LAYER_ADD_UNDO) ||
(state == UNDO && type == LAYER_REMOVE_UNDO))
if ((state == REDO && type == LAYER_MASK_ADD_UNDO) ||
(state == UNDO && type == LAYER_MASK_REMOVE_UNDO))
layer_mask_delete (lmu->mask);
g_free (lmu);
......@@ -2852,7 +2852,7 @@ static struct undo_name_t {
UndoType type;
const char *name;
} undo_name[] = {
{0, N_("<<invalid>>")},
{UNDO_NULL, N_("<<invalid>>")},
{IMAGE_UNDO, N_("image")},
{IMAGE_MOD_UNDO, N_("image mod")},
{MASK_UNDO, N_("mask")},
......
......@@ -26,10 +26,11 @@
* the bottom of undo.c as well. */
typedef enum
{
/* Type 0 is special - in the gimpimage structure it means
/* Type UNDO_NULL (0) is special - in the gimpimage structure it means
* there is no undo group currently being added to. */
IMAGE_UNDO = 1,
UNDO_NULL = 0, /* Picky compilers demand this in the enumeration - gosgood@idt.net */
IMAGE_UNDO,
IMAGE_MOD_UNDO,
MASK_UNDO,
LAYER_DISPLACE_UNDO,
......
A quick overview of the undo system
-----------------------------------
Actions on the image by the user are pushed onto an undo stack. Each
action object includes all the information needed to undo or redo an
operation, plus an UndoType. The type can be converted to text to
show to the user. Actions may be run forwards (UndoState == REDO) or
backwards (UndoState == UNDO). As the action is run, it swaps the
image's current state and the recorded state. A run action is moved
from the undo stack to the redo stack (or vice-versa if UndoState ==
REDO). Pushing something onto the undo stack causes the redo stack to
be cleared, since the actions on the redo stack may depend on the
image being in a particular state (eg consider: layer add, rename,
undo rename, layer delete. If the redo stack weren't cleared on undo,
then there would still be a "rename" operation on the redo stack which
could be run on a non-existent layer. Bad news.)
Undo groups
-----------
In order to group many basic operations together into a more useful
whole, code can push group start and end markers. A group is treated
as a single action for the purposes of the undo and redo user
commands. It is legal to nest groups, in which case the outermost
group is the only user-visible one.
Groups boundaries used to be implemented by pushing a NULL pointer on
the undo (or redo) stack. Now they are a special action which has the
"group_boundary" bit set. This allows the group boundaries to include
the undo type associated with the whole group. The individual actions
need to preserve their own undo type since the undo_free_* functions
sometimes need to know which action is being freed.
Undo events
-----------
Images emit UNDO_EVENT signals, to say that the user has performed an
undo or redo action on that image. This allows interested parties to
track image mutation actions. So far, only the undo history dialog
uses this feature. The other way to discover the undo status of an
image is to use the iterator functions undo_map_over_undo_stack() and
undo_map_over_redo_stack(). These call your function on each action
(or group) on the stack. There is also undo_get_undo_name() and
undo_get_redo_name() to peek at the top items on each stack. This
could be used (eg) to change the undo/redo menu strings to something
more meaningful, but currently lack synchronisation.
Dirtying images
---------------
NOTE about the gimage->dirty counter:
If 0, then the image is clean (ie, copy on disk is the same as the one
in memory).
If positive, then that's the number of dirtying operations done
on the image since the last save.
If negative, then user has hit undo and gone back in time prior
to the saved copy. Hitting redo will eventually come back to
the saved copy.
The image is dirty (ie, needs saving) if counter is non-zero.
If the counter is around 10000, this is due to undo-ing back
before a saved version, then mutating the image (thus destroying
the redo stack). Once this has happened, it's impossible to get
the image back to the state on disk, since the redo info has been
freed. See undo.c for the gorey details.
NEVER CALL gimp_image_dirty() directly!
If your code has just dirtied the image, push an undo instead.
Failing that, push the trivial undo which tells the user the
command is not undoable: undo_push_cantundo() (But really, it would
be best to push a proper undo). If you just dirty the image
without pushing an undo then the dirty count is increased, but
popping that many undo actions won't lead to a clean image.
Austin
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment