← Back to team overview

kicad-developers team mailing list archive

Re: [RFC] Undo buffer refactoring

 

On 5/13/2016 9:08 AM, Maciej Sumiński wrote:
> I would like to solve once and for all the problem of GAL view/ratsnest
> not being occasionally updated.
> 
> I am still getting related bug reports, as there are just many places in
> the source code that need to have "item->ViewUpdate();
> ratsnest->Update(item)" added.
> 
> 1. Replace UR_MODEDIT with UR_CHANGED, to avoid repeating
> if(IsType(FRAME_PCB)) and stop treating modules in a special way. Make
> UR_CHANGED create a MODULE copy?

This makes sense to me.  I seem to remember doing the same thing with
UR_ROTATE so I don't see why changing a MODULE would be a special undo
action.

> 
> 2. Include OnModify() in SaveCopyInUndoList(), as it is called every
> time anyway.

Include them where?  Please be careful to avoid coupling the UI with the
undo/redo buffer.  I seem to remember this being a stumbling block as
part of the Eeschema refactor so anything you can do to avoid coupling
would be a good thing.

> 
> 3. Add an Observer interface, so other objects interested in model
> changes (e.g. VIEW & RATSNEST) might be notified, when there is an undo
> buffer entry created/restored. Observers would have to register
> themselves to EDA_DRAW_FRAME, as registering to every single BOARD_ITEM
> does not make sense. Notification type can be described with UNDO_REDO_T
> enum.

OK.

> 
> 4. It would be also required to notify observers when there is no undo
> buffer entry created (e.g. mass track removal, SPECCTRA import), so they
> have a chance to react properly.

OK

> 
> 5. During certain operations (e.g. replacing a footprint), it is
> necessary to notify observer twice: when the old BOARD_ITEM is removed
> and when the new is added. Otherwise observers will keep stale pointers.

Be careful here to prevent any events from happening that could access a
state pointer before the callbacks occur.

> 
> 
> A few more steps to make code more generic:
> 
> 1. Create BOARD_ITEM_CONTAINER class as a common base class for BOARD &
> MODULE.
> 
> class BOARD_ITEM_CONTAINER {
>     Add(BOARD_ITEM*);
>     Remove/Delete(BOARD_ITEM*);
>     Remove/DeleteType(KICAD_T);
>     Count(KICAD_T);
> };
> 
> 2. Add BOARD_ITEM_CONTAINER* PCB_BASE_FRAME::GetModel()
> 
> 3. Unify undo buffer code, so items are added to/removed (UR_NEW,
> UR_DELETED) from the current BOARD_ITEM_CONTAINER.
> 
> 4. Get rid of UR_MOVED/UR_ROTATED, etc. and store an item copy instead.

Looks good.  If you happen to find any UI/undo buffer coupling please
try to weed it out if possible.  This will make the long term
maintenance of this code more palatable.

Thanks,

Wayne

> 
> Regards,
> Orson
> 
> 
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
> 



Follow ups

References