Re: revamping the undo infrastructure in Dia

[Lars Clausen <lars raeder dk>]

On Fri, 2007-10-19 at 20:05 +0530, Sameer Sahasrabuddhe wrote:
> Hi,
>
> The current state of the Undo/Redo infrastructure in Dia seriously
> lacks maintenance. A quick search for "undo" in bugs showed up some
> long-standing bugs, fixing which would require a lot of work and some
> fresh thinking. Here's a summary, sorted by what I think is more
> important:
>
> Bugs that highlight problems in the core:
>
> http://bugzilla.gnome.org/show_bug.cgi?id=318182
> Undo does not restore size of automatically resized shapes
>
> http://bugzilla.gnome.org/show_bug.cgi?id=158570
> change diagram background colour should be undoable
>
> http://bugzilla.gnome.org/show_bug.cgi?id=488269
> properties dialog creates empty undo actions when no properties are changed
>
> Bugs that are just enhancements and/or will get fixed as a side-effect:
>
> http://bugzilla.gnome.org/show_bug.cgi?id=482490
> Undo & redo menu labels need detail I.e. "Undo move", "Undo delete"
>
> http://bugzilla.gnome.org/show_bug.cgi?id=488270
> properties dialog should have a button to undo all changes made while
> interacting with the dialog
>
> And here's my take on how Undo/Redo needs improvement:
>
> 1) The _Change struct in app/undo.h should have a facility to track
> side-effects. For example, when changing a text-object results in a
> resizing of the object, the resize should be recorded as a side-effect
> of the main text-related Change.

Sounds necessary, given that many different objects use the same text
change object.  Alternatively, they could come as a chain of changes,
since the prev/next links are right there.

> 2) Every function that modifies an object in any way needs to be
> tracked down, and rewritten to suitably return a Change object. For
> example, there is one place where obj->ops->move() is called simply
> with the intention that is best described as
> obj->ops->update_yourself_and_report_side_effects().

That's where the hard work comes in.

> Also there is a hidden problem with the way the properties dialog
> tracks undo history. It records the previous state of _all_ known
> properties, whether or not they are changed. This can lead to
> scalability problems if we dare to think of huge groups of objects
> with a huge list of properties. 

That could take a fair amount of space, yes.  We haven't seen it as a
problem I think mostly because mass change of properties has had the
problems of actually unifying all properties, and only now that you've
done the group properties update fixing are people likely to try
changing a large number of objects at the same time.

> Also, this cannot handle side-effects
> to the object that do not involve properties. For example, changing
> the font of a text object changes its size, but this is not tracked by
> undo.

Any changeable aspect of an object should be derivable from its
properties, otherwise it doesn't survive saving.

> This can only be fixed if the Undo/Redo infrastructure guarantees that
> side-effects are efficiently handled elsewhere. But the improvements
> required for Undo are quite extensive, and would require lots of
> planning as well as sufficient blessings from the gods of Dia.

I bless the change as such, but I'm not sure it'd be a good idea to
start on it now.  We have three major improvements in that should get
out to people, adding this would delay it yet further.  Time has already
flown much faster on getting things fixed than I was hoping for.

-Lars