kicad-developers team mailing list archive
-
kicad-developers team
-
Mailing list archive
-
Message #24662
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