Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
home:wolfi323:branches:KDE:Frameworks5
avogadrolibs
876.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File 876.patch of Package avogadrolibs
From b088d239d51903b9c67395294bb5d7ec07b0d61f Mon Sep 17 00:00:00 2001 From: Geoff Hutchison <geoff.hutchison@gmail.com> Date: Thu, 28 Apr 2022 16:51:14 -0400 Subject: [PATCH 1/2] WIP to find the memory bug involved in #824 Right now, this reverts #806 but there's still some sort of memory corruption when assigning one molecule to another Also, it seems like File -> Import Crystal adds atoms that don't have layer information Signed-off-by: Geoff Hutchison <geoff.hutchison@gmail.com> --- avogadro/core/layermanager.cpp | 7 ++++++ avogadro/core/layermanager.h | 2 +- avogadro/core/molecule.cpp | 22 +++++-------------- avogadro/qtgui/pluginlayermanager.cpp | 8 +++---- avogadro/qtgui/pluginlayermanager.h | 2 +- avogadro/qtgui/rwlayermanager.cpp | 3 ++- avogadro/qtgui/rwmolecule.cpp | 5 +++++ .../insertfragment/insertfragment.cpp | 4 ++++ 8 files changed, 30 insertions(+), 23 deletions(-) diff --git a/avogadro/core/layermanager.cpp b/avogadro/core/layermanager.cpp index 0fd914b99..148d9f63e 100644 --- a/avogadro/core/layermanager.cpp +++ b/avogadro/core/layermanager.cpp @@ -26,6 +26,7 @@ Layer& LayerManager::getMoleculeLayer() Layer& LayerManager::getMoleculeLayer(const Molecule* mol) { + assert(mol != nullptr); auto it = m_molToInfo.find(mol); if (it == m_molToInfo.end()) { m_molToInfo[mol] = make_shared<MoleculeInfo>(mol); @@ -41,6 +42,7 @@ shared_ptr<MoleculeInfo> LayerManager::getMoleculeInfo() shared_ptr<MoleculeInfo> LayerManager::getMoleculeInfo(const Molecule* mol) { + assert(mol != nullptr); auto it = m_molToInfo.find(mol); if (it == m_molToInfo.end()) { m_molToInfo[mol] = make_shared<MoleculeInfo>(mol); @@ -51,6 +53,9 @@ shared_ptr<MoleculeInfo> LayerManager::getMoleculeInfo(const Molecule* mol) Layer& LayerManager::getMoleculeLayer(const Molecule* original, const Molecule* copy) { + assert(original != nullptr); + assert(copy != nullptr); + auto it = m_molToInfo.find(original); if (it == m_molToInfo.end()) { auto molecule = make_shared<MoleculeInfo>(original); @@ -65,6 +70,8 @@ Layer& LayerManager::getMoleculeLayer(const Molecule* original, void LayerManager::deleteMolecule(const Molecule* mol) { + assert(mol != nullptr); + auto aux = m_molToInfo.find(mol); if (aux != m_molToInfo.end()) { auto id = aux->second->mol; diff --git a/avogadro/core/layermanager.h b/avogadro/core/layermanager.h index 96f648166..1618f4bda 100644 --- a/avogadro/core/layermanager.h +++ b/avogadro/core/layermanager.h @@ -51,7 +51,7 @@ struct LayerData /** * @class MoleculeInfo layermanager.h <avogadro/core/layermanager.h> * @brief All layer dependent data. Original molecule @p mol, is layer hidden - * @p visible, accepts eddits @p locked, and key-value data like @p enable, + * @p visible, accepts edits @p locked, and key-value data like @p enable, * and custom data @p settings. */ struct MoleculeInfo diff --git a/avogadro/core/molecule.cpp b/avogadro/core/molecule.cpp index a3b824f55..a52df7adf 100644 --- a/avogadro/core/molecule.cpp +++ b/avogadro/core/molecule.cpp @@ -45,12 +45,8 @@ Molecule::Molecule(const Molecule& other) m_residues(other.m_residues), m_graph(other.m_graph), m_bondOrders(other.m_bondOrders), m_atomicNumbers(other.m_atomicNumbers), - m_layers(LayerManager::getMoleculeLayer(this)) + m_layers(LayerManager::getMoleculeLayer(&other, this)) { - // Copy the layers, if they exist - if (other.m_layers.maxLayer() > 0) - m_layers = LayerManager::getMoleculeLayer(&other, this); - // Copy over any meshes for (Index i = 0; i < other.meshCount(); ++i) { Mesh* m = addMesh(); @@ -83,12 +79,8 @@ Molecule::Molecule(Molecule&& other) noexcept m_bondPairs(std::move(other.m_bondPairs)), m_bondOrders(std::move(other.m_bondOrders)), m_atomicNumbers(std::move(other.m_atomicNumbers)), - m_layers(LayerManager::getMoleculeLayer(this)) + m_layers(LayerManager::getMoleculeLayer(&other, this)) { - // Copy the layers, if they exist - if (other.m_layers.maxLayer() > 0) - m_layers = LayerManager::getMoleculeLayer(&other, this); - m_basisSet = other.m_basisSet; other.m_basisSet = nullptr; @@ -138,11 +130,10 @@ Molecule& Molecule::operator=(const Molecule& other) m_basisSet = other.m_basisSet ? other.m_basisSet->clone() : nullptr; delete m_unitCell; m_unitCell = other.m_unitCell ? new UnitCell(*other.m_unitCell) : nullptr; - } - // Copy the layers, if they exist - if (other.m_layers.maxLayer() > 0) + // Copy the layers, if they exist m_layers = LayerManager::getMoleculeLayer(&other, this); + } return *this; } @@ -184,8 +175,7 @@ Molecule& Molecule::operator=(Molecule&& other) noexcept other.m_unitCell = nullptr; // Copy the layers, if they exist - if (other.m_layers.maxLayer() > 0) - m_layers = LayerManager::getMoleculeLayer(&other, this); + m_layers = LayerManager::getMoleculeLayer(&other, this); } return *this; @@ -193,7 +183,7 @@ Molecule& Molecule::operator=(Molecule&& other) noexcept Molecule::~Molecule() { - LayerManager::deleteMolecule(this); + //LayerManager::deleteMolecule(this); delete m_basisSet; delete m_unitCell; clearMeshes(); diff --git a/avogadro/qtgui/pluginlayermanager.cpp b/avogadro/qtgui/pluginlayermanager.cpp index 5562984f5..1f236a254 100644 --- a/avogadro/qtgui/pluginlayermanager.cpp +++ b/avogadro/qtgui/pluginlayermanager.cpp @@ -40,7 +40,7 @@ PluginLayerManager::~PluginLayerManager() bool PluginLayerManager::isEnabled() const { - if (m_activeMolecule == nullptr || + if (m_activeMolecule == nullptr || m_molToInfo[m_activeMolecule] == nullptr || m_molToInfo[m_activeMolecule]->enable.find(m_name) == m_molToInfo[m_activeMolecule]->enable.end()) { return false; @@ -55,7 +55,7 @@ bool PluginLayerManager::isEnabled() const bool PluginLayerManager::isActiveLayerEnabled() const { - if (m_activeMolecule == nullptr || + if (m_activeMolecule == nullptr || m_molToInfo[m_activeMolecule] == nullptr || m_molToInfo[m_activeMolecule]->enable.find(m_name) == m_molToInfo[m_activeMolecule]->enable.end()) { return false; @@ -70,7 +70,7 @@ bool PluginLayerManager::isActiveLayerEnabled() const void PluginLayerManager::setEnabled(bool enable) { - if (m_activeMolecule == nullptr) { + if (m_activeMolecule == nullptr || m_molToInfo[m_activeMolecule] == nullptr) { return; } auto& molecule = m_molToInfo[m_activeMolecule]; @@ -88,7 +88,7 @@ void PluginLayerManager::setEnabled(bool enable) bool PluginLayerManager::atomEnabled(Index atom) const { - if (m_activeMolecule == nullptr || + if (m_activeMolecule == nullptr || m_molToInfo[m_activeMolecule] == nullptr || m_molToInfo[m_activeMolecule]->enable.find(m_name) == m_molToInfo[m_activeMolecule]->enable.end()) { return false; diff --git a/avogadro/qtgui/pluginlayermanager.h b/avogadro/qtgui/pluginlayermanager.h index 376f9229c..09830fbe4 100644 --- a/avogadro/qtgui/pluginlayermanager.h +++ b/avogadro/qtgui/pluginlayermanager.h @@ -18,7 +18,7 @@ namespace QtGui { * @class PluginLayerManager pluginlayermanager.h * <avogadro/qtgui/pluginlayermanager.h> * @brief The PluginLayerManager class is a set of common layer dependent - * operators usefull for Layer dependent QtPlugins. + * operators useful for Layer dependent QtPlugins. */ class AVOGADROQTGUI_EXPORT PluginLayerManager : protected Core::LayerManager { diff --git a/avogadro/qtgui/rwlayermanager.cpp b/avogadro/qtgui/rwlayermanager.cpp index 5c0caaa16..c112a1aa5 100644 --- a/avogadro/qtgui/rwlayermanager.cpp +++ b/avogadro/qtgui/rwlayermanager.cpp @@ -242,9 +242,10 @@ void RWLayerManager::addMolecule(const Core::Molecule* mol) Array<std::pair<size_t, string>> RWLayerManager::activeMoleculeNames() const { - if (m_activeMolecule == nullptr) { + if (m_activeMolecule == nullptr || m_molToInfo[m_activeMolecule] == nullptr) { return Array<std::pair<size_t, string>>(); } + auto& molecule = m_molToInfo[m_activeMolecule]; size_t qttyLayer = molecule->layer.layerCount(); vector<set<string>> active(qttyLayer, set<string>()); diff --git a/avogadro/qtgui/rwmolecule.cpp b/avogadro/qtgui/rwmolecule.cpp index 73262ca6f..214918dca 100644 --- a/avogadro/qtgui/rwmolecule.cpp +++ b/avogadro/qtgui/rwmolecule.cpp @@ -26,6 +26,8 @@ #include <avogadro/core/spacegroups.h> #include <avogadro/qtgui/hydrogentools.h> +#include <QtCore/QDebug> + namespace Avogadro { namespace QtGui { @@ -413,6 +415,9 @@ void RWMolecule::modifyMolecule(const Molecule& newMolecule, ModifyMoleculeCommand* comm = new ModifyMoleculeCommand(*this, m_molecule, newMolecule); + qDebug() << " checking layers " << m_molecule.layer().maxLayer() << m_molecule.layer(0); + qDebug() << " and in newMol " << newMolecule.layer().maxLayer() << newMolecule.layer(0); + comm->setText(undoText); m_undoStack.push(comm); diff --git a/avogadro/qtplugins/insertfragment/insertfragment.cpp b/avogadro/qtplugins/insertfragment/insertfragment.cpp index 2129c8f1d..bb943a437 100644 --- a/avogadro/qtplugins/insertfragment/insertfragment.cpp +++ b/avogadro/qtplugins/insertfragment/insertfragment.cpp @@ -124,6 +124,10 @@ void InsertFragment::performInsert(const QString& fileName, bool crystal) if (crystal) { Molecule::MoleculeChanges changes = (Molecule::Atoms | Molecule::Bonds | Molecule::Added | Molecule::Removed); + + // check layers in newMol + qDebug() << " added " << newMol.layer().maxLayer() << newMol.layer(0); + m_molecule->undoMolecule()->modifyMolecule(newMol, changes, tr("Import Crystal")); emit requestActiveTool("Navigator"); From 165305f7edcad0e7b838380e41becb28513ed611 Mon Sep 17 00:00:00 2001 From: Geoff Hutchison <geoff.hutchison@gmail.com> Date: Thu, 28 Apr 2022 19:45:53 -0400 Subject: [PATCH 2/2] Make sure when copy / assign molecules that all atoms are added to the active layer. An ideal thing might be to create a new temporary layer (e.g., a copy layer) but this is a good step Signed-off-by: Geoff Hutchison <geoff.hutchison@gmail.com> --- avogadro/core/molecule.cpp | 86 ++++++++++++------- avogadro/qtgui/rwmolecule.cpp | 5 -- .../insertfragment/insertfragment.cpp | 16 +--- .../qtplugins/insertfragment/insertfragment.h | 13 +-- 4 files changed, 59 insertions(+), 61 deletions(-) diff --git a/avogadro/core/molecule.cpp b/avogadro/core/molecule.cpp index a52df7adf..cf054bc03 100644 --- a/avogadro/core/molecule.cpp +++ b/avogadro/core/molecule.cpp @@ -43,8 +43,7 @@ Molecule::Molecule(const Molecule& other) m_residues(other.m_residues), m_graph(other.m_graph), m_graphDirty(other.m_graphDirty), m_bondPairs(other.m_bondPairs), m_bondOrders(other.m_bondOrders), m_atomicNumbers(other.m_atomicNumbers), - m_atomicNumbers(other.m_atomicNumbers), - m_layers(LayerManager::getMoleculeLayer(&other, this)) + m_layers(LayerManager::getMoleculeLayer(this)) { // Copy over any meshes for (Index i = 0; i < other.meshCount(); ++i) { @@ -58,6 +57,15 @@ Molecule::Molecule(const Molecule& other) Cube* c = addCube(); *c = *other.cube(i); } + + // Copy layers, if they exist + if (other.m_layers.maxLayer() > 0) + m_layers = LayerManager::getMoleculeLayer(&other, this); + else { + // make sure all the atoms are in the active layer + for (Index i = 0; i < atomCount(); ++i) + m_layers.addAtomToActiveLayer(i); + } } Molecule::Molecule(Molecule&& other) noexcept @@ -79,13 +87,22 @@ Molecule::Molecule(Molecule&& other) noexcept m_residues(std::move(other.m_residues)), m_graph(std::move(other.m_graph)), m_bondOrders(std::move(other.m_bondOrders)), m_atomicNumbers(std::move(other.m_atomicNumbers)), - m_layers(LayerManager::getMoleculeLayer(&other, this)) + m_layers(LayerManager::getMoleculeLayer(this)) { m_basisSet = other.m_basisSet; other.m_basisSet = nullptr; m_unitCell = other.m_unitCell; other.m_unitCell = nullptr; + + // Copy the layers, only if they exist + if (other.m_layers.maxLayer() > 0) + m_layers = LayerManager::getMoleculeLayer(&other, this); + else { + // make sure all the atoms are in the active layer + for (Index i = 0; i < atomCount(); ++i) + m_layers.addAtomToActiveLayer(i); + } } Molecule& Molecule::operator=(const Molecule& other) @@ -131,8 +148,14 @@ Molecule& Molecule::operator=(const Molecule& other) delete m_unitCell; m_unitCell = other.m_unitCell ? new UnitCell(*other.m_unitCell) : nullptr; - // Copy the layers, if they exist - m_layers = LayerManager::getMoleculeLayer(&other, this); + // Copy the layers, only if they exist + if (other.m_layers.maxLayer() > 0) + m_layers = LayerManager::getMoleculeLayer(&other, this); + else { + // make sure all the atoms are in the active layer + for (Index i = 0; i < atomCount(); ++i) + m_layers.addAtomToActiveLayer(i); + } } return *this; @@ -175,7 +198,13 @@ Molecule& Molecule::operator=(Molecule&& other) noexcept other.m_unitCell = nullptr; // Copy the layers, if they exist - m_layers = LayerManager::getMoleculeLayer(&other, this); + if (other.m_layers.maxLayer() > 0) + m_layers = LayerManager::getMoleculeLayer(&other, this); + else { + // make sure all the atoms are in the active layer + for (Index i = 0; i < atomCount(); ++i) + m_layers.addAtomToActiveLayer(i); + } } return *this; @@ -183,7 +212,7 @@ Molecule& Molecule::operator=(Molecule&& other) noexcept Molecule::~Molecule() { - //LayerManager::deleteMolecule(this); + // LayerManager::deleteMolecule(this); delete m_basisSet; delete m_unitCell; clearMeshes(); @@ -472,12 +501,12 @@ Molecule::BondType Molecule::bond(Index atomId1, Index atomId2) const assert(atomId1 < atomCount()); assert(atomId2 < atomCount()); - const std::vector<Index> &edgeIndices = m_graph.edges(atomId1); + const std::vector<Index>& edgeIndices = m_graph.edges(atomId1); for (Index i = 0; i < edgeIndices.size(); i++) { Index index = edgeIndices[i]; - const std::pair<Index, Index> &pair = m_graph.endpoints(index); + const std::pair<Index, Index>& pair = m_graph.endpoints(index); if (pair.first == atomId2 || pair.second == atomId2) - return BondType(const_cast<Molecule *>(this), index); + return BondType(const_cast<Molecule*>(this), index); } return BondType(); } @@ -490,25 +519,25 @@ Array<Molecule::BondType> Molecule::bonds(const AtomType& a) return bonds(a.index()); } -Array<const Molecule::BondType *> Molecule::bonds(Index a) const +Array<const Molecule::BondType*> Molecule::bonds(Index a) const { - Array<const BondType *> atomBonds; + Array<const BondType*> atomBonds; if (a < atomCount()) { - const std::vector<Index> &edgeIndices = m_graph.edges(a); + const std::vector<Index>& edgeIndices = m_graph.edges(a); for (Index i = 0; i < edgeIndices.size(); ++i) { Index index = edgeIndices[i]; - if (m_graph.endpoints(index).first == a - || m_graph.endpoints(index).second == a) - { + if (m_graph.endpoints(index).first == a || + m_graph.endpoints(index).second == a) { // work around to consult bonds without breaking constantness - atomBonds.push_back(new BondType(const_cast<Molecule *>(this), index)); + atomBonds.push_back(new BondType(const_cast<Molecule*>(this), index)); } } } - std::sort(atomBonds.begin(), atomBonds.end(), [](const BondType *&a, const BondType *&b) { - return a->index() < b->index(); - }); + std::sort(atomBonds.begin(), atomBonds.end(), + [](const BondType*& a, const BondType*& b) { + return a->index() < b->index(); + }); return atomBonds; } @@ -516,7 +545,7 @@ Array<Molecule::BondType> Molecule::bonds(Index a) { Array<BondType> atomBonds; if (a < atomCount()) { - const std::vector<Index> &edgeIndices = m_graph.edges(a); + const std::vector<Index>& edgeIndices = m_graph.edges(a); for (Index i = 0; i < edgeIndices.size(); ++i) { Index index = edgeIndices[i]; auto bond = bondPair(index); @@ -525,9 +554,8 @@ Array<Molecule::BondType> Molecule::bonds(Index a) } } - std::sort(atomBonds.begin(), atomBonds.end(), [](BondType &a, BondType &b) { - return a.index() < b.index(); - }); + std::sort(atomBonds.begin(), atomBonds.end(), + [](BondType& a, BondType& b) { return a.index() < b.index(); }); return atomBonds; } @@ -982,8 +1010,8 @@ bool Molecule::removeBonds(Index atom) if (atom >= atomCount()) return false; - while(true) { - const std::vector<size_t> &bondList = m_graph.edges(atom); + while (true) { + const std::vector<size_t>& bondList = m_graph.edges(atom); if (!bondList.size()) break; size_t bond = bondList[0]; @@ -995,7 +1023,7 @@ bool Molecule::removeBonds(Index atom) Array<std::pair<Index, Index>> Molecule::getAtomBonds(Index index) const { Array<std::pair<Index, Index>> result; - const std::vector<Index> &edgeIndices = m_graph.edges(index); + const std::vector<Index>& edgeIndices = m_graph.edges(index); for (Index i = 0; i < edgeIndices.size(); i++) { result.push_back(m_graph.endpoints(edgeIndices[i])); } @@ -1005,7 +1033,7 @@ Array<std::pair<Index, Index>> Molecule::getAtomBonds(Index index) const Array<unsigned char> Molecule::getAtomOrders(Index index) const { Array<unsigned char> result; - const std::vector<Index> &edgeIndices = m_graph.edges(index); + const std::vector<Index>& edgeIndices = m_graph.edges(index); for (Index i = 0; i < edgeIndices.size(); i++) { result.push_back(m_bondOrders[edgeIndices[i]]); } diff --git a/avogadro/qtgui/rwmolecule.cpp b/avogadro/qtgui/rwmolecule.cpp index 214918dca..73262ca6f 100644 --- a/avogadro/qtgui/rwmolecule.cpp +++ b/avogadro/qtgui/rwmolecule.cpp @@ -26,8 +26,6 @@ #include <avogadro/core/spacegroups.h> #include <avogadro/qtgui/hydrogentools.h> -#include <QtCore/QDebug> - namespace Avogadro { namespace QtGui { @@ -415,9 +413,6 @@ void RWMolecule::modifyMolecule(const Molecule& newMolecule, ModifyMoleculeCommand* comm = new ModifyMoleculeCommand(*this, m_molecule, newMolecule); - qDebug() << " checking layers " << m_molecule.layer().maxLayer() << m_molecule.layer(0); - qDebug() << " and in newMol " << newMolecule.layer().maxLayer() << newMolecule.layer(0); - comm->setText(undoText); m_undoStack.push(comm); diff --git a/avogadro/qtplugins/insertfragment/insertfragment.cpp b/avogadro/qtplugins/insertfragment/insertfragment.cpp index bb943a437..4123fc76b 100644 --- a/avogadro/qtplugins/insertfragment/insertfragment.cpp +++ b/avogadro/qtplugins/insertfragment/insertfragment.cpp @@ -1,17 +1,6 @@ /****************************************************************************** - This source file is part of the Avogadro project. - - Copyright 2020 Geoffrey Hutchison - - This source code is released under the New BSD License, (the "License"). - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - + This source code is released under the 3-Clause BSD License, (see "LICENSE"). ******************************************************************************/ #include "insertfragment.h" @@ -125,9 +114,6 @@ void InsertFragment::performInsert(const QString& fileName, bool crystal) Molecule::MoleculeChanges changes = (Molecule::Atoms | Molecule::Bonds | Molecule::Added | Molecule::Removed); - // check layers in newMol - qDebug() << " added " << newMol.layer().maxLayer() << newMol.layer(0); - m_molecule->undoMolecule()->modifyMolecule(newMol, changes, tr("Import Crystal")); emit requestActiveTool("Navigator"); diff --git a/avogadro/qtplugins/insertfragment/insertfragment.h b/avogadro/qtplugins/insertfragment/insertfragment.h index 70ee1a291..39f2b2fda 100644 --- a/avogadro/qtplugins/insertfragment/insertfragment.h +++ b/avogadro/qtplugins/insertfragment/insertfragment.h @@ -1,17 +1,6 @@ /****************************************************************************** - This source file is part of the Avogadro project. - - Copyright 2020 Geoffrey R. Hutchison - - This source code is released under the New BSD License, (the "License"). - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - + This source code is released under the 3-Clause BSD License, (see "LICENSE"). ******************************************************************************/ #ifndef AVOGADRO_QTPLUGINS_INSERTFRAGMENT_H
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor