Skip to content

Commit

Permalink
Draws in edit mode are now properly undo-able
Browse files Browse the repository at this point in the history
Added some functions called beginMergeMode() and endMergeMode() in
RWMolecule. beginMergeMode() turns interactive mode on, and it
calls QUndoStack::beginMacro(). This results in all changes
being merged into a single undo command once endMergeMode()
is called. Note that every beginMergeMode() must have one
endMergeMode(). When a left click or right click is pressed,
beginMergeMode() is called. When the click is released,
endMergeMode() is called. This allows whole draw actions to be
lumped into a single undo and redo command. This is necessary
since draws typically consist of several different types of
commands that cannot be merged by interactive mode (creating an
atom, creating a bond, moving an atom, changing the bond order,
changing an atom type, etc.). However, interactive mode still
merges similar commands that are in series to save space.

In addition, bond.setOrder() is only called now if the bond
order has actually changed. This allows interactive mode to
merge together the drawing properly.

Unfortunately, if there is only one undo on the undo stack, and
an undo and a redo is called, there seems to be a glitch where
all atoms are put at (0,0,0). This was not introduced by this
commit, but it needs to be fixed sometime in the future...
  • Loading branch information
psavery committed Aug 14, 2016
1 parent f292e4f commit 63e3f2a
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 12 deletions.
35 changes: 34 additions & 1 deletion avogadro/qtgui/rwmolecule.h
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,28 @@ class AVOGADROQTGUI_EXPORT RWMolecule : public QObject
* @param cartTol Cartesian tolerance for comparing atom positions.
* @return True if the algorithm succeeded, and false if it failed.
*/
bool reduceCellToAsymmetricUnit(unsigned short hallNumber, double cartTol = 1e-5);
bool reduceCellToAsymmetricUnit(unsigned short hallNumber,
double cartTol = 1e-5);

/**
* Call this function when you wish to merge all undo commands.
* It turns on interactive mode to merge similar undo commands in a series
* (in order to save space), and it uses QUndoStack's beginMacro() to merge
* dissimilar undo commands together. You must call endMergeMode() to end
* the merging section (undo and redo are unavailable while merge mode is
* on).
* @param undoName The name of the undo command
*/
void beginMergeMode(const QString& undoName = "Draw");

/**
* Call this function when you wish merge mode to end. This will turn off
* interactive mode, and it will call QUndoStack's endMacro(). All of the
* undo commands pushed between beginMergeMode() and endMergeMode() will be
* merged into one undo command. beginMergeMode() should have been called
* before this is called.
*/
void endMergeMode();

/**
* @brief Begin or end an interactive edit.
Expand Down Expand Up @@ -744,6 +765,18 @@ inline std::pair<Index, Index> RWMolecule::bondPair(Index bondId) const
return m_molecule.bondPair(bondId);
}

inline void RWMolecule::beginMergeMode(const QString& undoName)
{
m_interactive = true;
m_undoStack.beginMacro(undoName);
}

inline void RWMolecule::endMergeMode()
{
m_interactive = false;
m_undoStack.endMacro();
}

inline void RWMolecule::setInteractive(bool b)
{
m_interactive = b;
Expand Down
28 changes: 17 additions & 11 deletions avogadro/qtplugins/editor/editor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,27 +103,28 @@ QWidget *Editor::toolWidget() const
QUndoCommand *Editor::mousePressEvent(QMouseEvent *e)
{
clearKeyPressBuffer();
if (!m_renderer)
if (!m_renderer || !m_molecule)
return NULL;

updatePressedButtons(e, false);
m_clickPosition = e->pos();

if (m_molecule) {
m_molecule->setInteractive(true);
}

if (m_pressedButtons & Qt::LeftButton) {
m_clickedObject = m_renderer->hit(e->pos().x(), e->pos().y());

switch (m_clickedObject.type) {
case Rendering::InvalidType:
m_molecule->beginMergeMode(tr("Draw Atom"));
emptyLeftClick(e);
return NULL;
case Rendering::AtomType:
// We don't know yet if we are drawing a bond/atom or replacing an atom
// unfortunately...
m_molecule->beginMergeMode(tr("Draw"));
atomLeftClick(e);
return NULL;
case Rendering::BondType:
m_molecule->beginMergeMode(tr("Change Bond Type"));
bondLeftClick(e);
return NULL;
}
Expand All @@ -133,9 +134,11 @@ QUndoCommand *Editor::mousePressEvent(QMouseEvent *e)

switch (m_clickedObject.type) {
case Rendering::AtomType:
m_molecule->beginMergeMode(tr("Remove Atom"));
atomRightClick(e);
return NULL;
case Rendering::BondType:
m_molecule->beginMergeMode(tr("Remove Bond"));
bondRightClick(e);
return NULL;
default:
Expand All @@ -148,15 +151,11 @@ QUndoCommand *Editor::mousePressEvent(QMouseEvent *e)

QUndoCommand *Editor::mouseReleaseEvent(QMouseEvent *e)
{
if (!m_renderer)
if (!m_renderer || !m_molecule)
return NULL;

updatePressedButtons(e, true);

if (m_molecule) {
m_molecule->setInteractive(false);
}

if (m_clickedObject.type == Rendering::InvalidType)
return NULL;

Expand All @@ -165,6 +164,12 @@ QUndoCommand *Editor::mouseReleaseEvent(QMouseEvent *e)
case Qt::RightButton:
reset();
e->accept();
m_molecule->endMergeMode();
// Let's cover all possible changes - the undo stack won't update
// without this
m_molecule->emitChanged(Molecule::Atoms | Molecule::Bonds |
Molecule::Added | Molecule::Removed |
Molecule::Modified);
break;
default:
break;
Expand Down Expand Up @@ -606,7 +611,8 @@ void Editor::atomLeftDrag(QMouseEvent *e)
RWBond bond = m_molecule->bond(newAtom, clickedAtom);
if (bond.isValid()) {
int bondOrder = expectedBondOrder(newAtom, clickedAtom);
bond.setOrder(bondOrder);
if (bondOrder != bond.order())
bond.setOrder(bondOrder);

changes |= Molecule::Bonds | Molecule::Modified;
}
Expand Down

0 comments on commit 63e3f2a

Please sign in to comment.