Hi list, I'm sure many of you saw this:
http://sourceforge.net/tracker/?func=detail&aid=3459981&group_id=165310&atid=835077 This bug report summarizes a lot of the feedback I get from my colleagues after they use avogadro for a while -- they like the program, see potential, but it's just not stable enough for everyday use. I've noticed this myself -- in fact, I always get a tad nervous and mentally review recent changes before hitting "Undo" in case I did anything slightly out of the ordinary that may trigger a crash. I think that "Undo" is one area of avogadro that really needs some attention. I'm as guilty as anyone; many of my early patches did not implement undo at all, and even recent patches that had undo (e.g. the crystallography extension) did so in a way that cluttered the internal data structures and left some things inconsistent. I've been thinking about how we can improve this, and my current favorite solution would be a "MoleculeDocument" class. The key concepts that go into this class: * MoleculeDocument holds a pointer to the current molecule * MoleculeDocument has API functions that modify the molecule (addAtom, removeBond, etc) * Each function that modifies the molecule creates an UndoCommand to carry out the specified changes * Changes can be concatenated Concatenation can be carried out automatically behind the scenes using QUndoStack::(begin|end)Macro() and/or QUndoCommand::mergeWith(): http://developer.qt.nokia.com/doc/qt-4.8/qundostack.html#id-288389e4-1582-4772-bda4-381569e3ada4 If all modification to the molecule is performed through the MoleculeDocument and the class is heavily unit-tested and stable, then undo becomes much simpler and more reliable. As an example, here are some things I'd like to be able to do from an extension/tool/etc: Add a methane molecule at the origin. This code would produce at least 9 individual undo commands, 5 AddAtomCommands and 4 AddBondCommands. Depending on how the calls are made, there may be SetBondOrderCommands, TranslateAtomCommands, etc may be produced as well. They will all be concatenated into a single UndoCommand with the name "Add methane". MoleculeDocument *doc = getCurrentMoleculeDocument(); doc->beginModify("Add methane"); doc->addAtom(6, 0.0, 0.0, 0.0); // Carbon at origin doc->adjustHydrogens(); doc->endModify(); We could do something similar for crystals: MoleculeDocument *mdoc = getCurrentMoleculeDocument(); CrystalDocument *doc = qobject_cast<CrystalDocument*>(mdoc); if (!doc) return; // Not a crystal doc->beginModify("Construct crystal"); doc->setCellParams(a, b, c, alpha, beta, gamma); doc->addAtom(...); // etc doc->setSpacegroup("Pnma"); doc->fillUnitCell(); doc->reduceToPrimitiveCell(); doc->endModify() Again, each function in the Document class will produce a number of UndoCommands that are concatenated into a single command, "Construct crystal". This would be a lot of work and it might prove inefficient for some corner cases (like extremely large-scale changes), but I think that the reliability payoff far exceeds this, and we could always allow for custom undo commands to be used for those "special" cases that are problematic. Any thoughts or opinions? This would be a large scale change, and I'm not sure what version something like this should be slotted for. But as evidenced by the bug report, we are turning away a lot of potential users with a negative experience and this is a major area that needs improvement. Dave ------------------------------------------------------------------------------ Learn Windows Azure Live! Tuesday, Dec 13, 2011 Microsoft is holding a special Learn Windows Azure training event for developers. It will provide a great way to learn Windows Azure and what it provides. You can attend the event by watching it streamed LIVE online. Learn more at http://p.sf.net/sfu/ms-windowsazure _______________________________________________ Avogadro-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/avogadro-devel
