Hi all,
Ales Hvezda writes:
> [...]
> Okay, It is the weekend and I am itching to do some merging. :)
> Where do you stand with your alternative? I am also unclear as to if your
> alternative is mutually exclusive or works with Carlos' glist_dev branch.
I based it on 20061020 and the way it is done is appreciably different
from Carlos branch. So I think it is mutually exclusive.
Note that, as I said earlier, it does replace only the 'zooming while
moving' half of Carlos' branch and not the other half, 'selection with
GList'.
> Please clarify the scope of your patch before it gets applied to CVS so
> that we don't end up with too much merge pain.
The main problem I have against the existing branch is that it
modifies directly the objects in the page before the user confirmed
the changes.
For example in a move action. The user selects one object from the
page and start moving it. There is no problem while he is translating
the object. However when he rotates or mirrors it, the changes are
applied to the object (see i_callback_edit_rotate_90_key()). But the
user can still cancel.
First consequence is that if there can not be autosaves during
actions. Maybe not a big deal since actions are not supposed to last
long but what we expect from the autosave feature is that it is
reliable and the saved data is correct at any time.
If the user cancels, the page is restored as it was before action
started thanks to the undo stuff. But this is not the job of the
undo. The undo should be limited to undoing confirmed changes
only. Remember that the action is not finished until the user confirms
the changes.
As a result of this undo, the selection is lost. Imagine you spend
time selecting several objects in various places, start a move but
cancel as you changed your mind or forgot one object... you have to
restart the selection from zero.
Conversely my patch does not modify the objects until the user apply
the changes and as such does not need to be surrounded by undo. When
you cancel you keep your selection and the autosave is guaranteed (it
can run during actions on valid data).
It also includes the support for net/bus to be correctly stretched
when moving connected objects (works with rotation and mirror). Plus
the o_drawbounding() function (now o_draw_action()) has been
refactored resulting in less glitches when drawing the action (for
example when moving from one desktop to another or covering the
drawing area with another window).
Note that these last two points can be fixed in Carlos' branch.
The new patch is already working on world coords: the conversion to
screen unit happens in the draw_xor functions. And so I think it is no
big deal to adapt your development work to this patch. It may be even
possible to apply it first.
Known problems:
- when in action the text objects may not be readable after
rotating/mirroring the objects. It's a matter of refining the
affine to enforce left-to-right character strings.
- it is possible to trigger another action from within an ongoing
action. This is a problem that also occurs with Carlos' branch
except it is silently ignored while with my patch it crashes the
program because of assertions.
If you find a different problem, please let me know.
Todo:
- it needs a bit of refactoring as there is a lot of repeated
code. Especially in starting and ending an action.
- it may be interesting to create a generic action structure/type
and to derive it for each specific actions.
- the conversion to screen coords before drawing could be integrated
directly in the affine from o_draw_action() to further simplifies
the drawing functions.
You can give this patch a try by downloading the tarballs for
libgeda and gschem respectively at:
http://www.chbcp.net/~pat/geda/libgeda-dev20061211.tar.gz
http://www.chbcp.net/~pat/geda/geda-gschem-dev20061211.tar.gz
Note: since it is based on 20061020 it is lacking the lastest changes
from HEAD (the gettext fix for example).
Regards,
Patrick
_______________________________________________
geda-dev mailing list
[email protected]
http://www.seul.org/cgi-bin/mailman/listinfo/geda-dev
