Commit b45e1c68 authored by David Thompson's avatar David Thompson Committed by Kitware Robot

Merge topic 'phrase-updates'

e84783ae Test PhraseModel handling of operation results.
5965d6fa Add release note.
f01db270 Fix component-phrase-model operation handler.
74b8f91d Fix method by which descriptive phrases are updated.
Acked-by: Kitware Robot's avatarKitware Robot <kwrobot@kitware.com>
Acked-by: T.J. Corona's avatarT.J. Corona <tj.corona@kitware.com>
Merge-request: !1570
parents b0c6b911 e84783ae
......@@ -141,6 +141,14 @@ To support this pattern, SMTK's Observer pattern has been generalized to a singl
## Changes to the View System and Qt Extensions
### Descriptive Phrase System
+ The PhraseModel's updateChildren() method has been rewritten to avoid
crashes that were occurring due to insertions beyond the end of storage.
+ The ComponentPhraseModel class has been fixed properly handle operation results;
in the past, only the top-level components would be kept updated after operations
complete. Now, children are inserted as well.
### New View Type - Associations
This view has the same syntax as an Attribute View but only allows the user to change the association information of the attribute resulting in taking up less screen Real Estate
......
......@@ -226,7 +226,8 @@ Operation::Result Operation::operate()
// Execute post-operation observation
if (observePostOperation)
{
manager->observers()(shared_from_this(), EventType::DID_OPERATE, result);
auto self = shared_from_this();
manager->observers()(self, EventType::DID_OPERATE, result);
}
// Unlock the resources.
......
......@@ -296,6 +296,15 @@ bool ComponentPhraseContent::editColorValue(ContentType attr, const resource::Fl
return false;
}
smtk::resource::PersistentObjectPtr ComponentPhraseContent::relatedObject() const
{
if (m_component)
{
return m_component;
}
return this->PhraseContent::relatedObject();
}
smtk::resource::ResourcePtr ComponentPhraseContent::relatedResource() const
{
if (!m_component)
......
......@@ -45,6 +45,7 @@ public:
bool editFlagValue(ContentType attr, int val) override;
bool editColorValue(ContentType attr, const resource::FloatList& val) override;
smtk::resource::PersistentObjectPtr relatedObject() const override;
smtk::resource::ResourcePtr relatedResource() const override;
smtk::resource::ComponentPtr relatedComponent() const override;
......
......@@ -125,6 +125,9 @@ void ComponentPhraseModel::handleCreated(
// components, we should simply add anything in res that passes our test
// for matching components.
this->populateRoot();
// Finally, call our subclass method to deal with children of the root element
this->PhraseModel::handleCreated(op, res, data);
}
void ComponentPhraseModel::processResource(Resource::Ptr rsrc, bool adding)
......
......@@ -415,6 +415,7 @@ void PhraseModel::updateChildren(
}
std::map<DescriptivePhrasePtr, int> lkup;
std::map<int, DescriptivePhrasePtr> insert;
int ii = 0;
std::set<int> unused;
DescriptivePhrases& orig(src->subphrases());
......@@ -425,7 +426,8 @@ void PhraseModel::updateChildren(
}
// Find mapping and subtract out the still-in-use phrases from unused.
for (DescriptivePhrases::iterator it = next.begin(); it != next.end(); ++it)
ii = 0;
for (DescriptivePhrases::iterator it = next.begin(); it != next.end(); ++it, ++ii)
{
auto oit = lkup.find(*it);
if (oit != lkup.end())
......@@ -439,6 +441,7 @@ void PhraseModel::updateChildren(
// Note that we might have phrases that are different instances
// but identical in that they refer to the same component/resource/etc.,
// hence the second test in the conditional below:
bool preexist = false;
for (auto it2 = orig.begin(); it2 != orig.end(); ++it2)
{
if ((it->get() == it2->get()) ||
......@@ -446,8 +449,16 @@ void PhraseModel::updateChildren(
{
*it = *it2;
unused.erase(lkup[*it2]);
preexist = true;
break; // only accept the first match.
}
}
// If there is no matching entry in orig for *it, then mark its
// location for insertion.
if (!preexist)
{
insert[ii] = *it;
}
}
}
......@@ -469,69 +480,87 @@ void PhraseModel::updateChildren(
uu = ug;
}
// Update lkup to account for removed rows.
int delta = 0;
auto nxen = lkup.begin();
for (auto entry = lkup.begin(); entry != lkup.end(); ++entry)
if (insert.size() + orig.size() != next.size())
{
while (entry != lkup.end() && unused.find(entry->second) != unused.end())
{ // row is unused
++delta;
nxen = entry;
++nxen;
lkup.erase(entry);
entry = nxen;
}
if (entry != lkup.end())
{ // row used... update as needed
entry->second -= delta;
}
else
{
break;
}
smtkErrorMacro(
smtk::io::Logger::instance(), "Update to descriptive phrases has correspondence problems"
<< " (" << orig.size() << " + " << insert.size() << " != " << next.size() << ")\n");
}
std::map<int, DescriptivePhrasePtr> insert;
// Move rows that previously existed but are now ordered differently.
// int destRow = static_cast<int>(lkup.size()) - 1;
int iref = static_cast<int>(next.size()) - 1;
for (auto it = next.rbegin(); it != next.rend(); ++it, --iref)
{
auto oit = lkup.find(*it);
if (oit == lkup.end())
{ // Skip rows we need to create
insert[iref] = *it;
continue;
}
// Is this a batch of rows to move in sequence?
// FIXME: TODO: Move rows!
}
// Finally, insert new phrases.
std::vector<int> insertRange(2);
// Insert new entries, starting at the front so as not to attempt
// insertion beyond the end of the list.
DescriptivePhrases batch;
std::map<int, DescriptivePhrasePtr>::iterator ib;
for (ib = insert.begin(); ib != insert.end();)
{
batch.clear();
batch.push_back(ib->second);
auto ie = ib;
auto prev = ie;
for (++ie; ie != insert.end() && (ie->first == prev->first + 1); ++ie, ++prev)
std::vector<int> insertRange(2);
for (auto entry = insert.begin(); entry != insert.end(); /* handled inside */)
{
// Batch consecutive insertions into a single event
batch.push_back(entry->second);
int osz = static_cast<int>(orig.size());
insertRange[0] = entry->first > osz ? osz : entry->first;
insertRange[1] = entry->first;
ii = entry->first;
auto ep1 = entry;
for (++ep1, ++ii; ep1 != insert.end() && ep1->first == ii; ++ep1, ++ii)
{
batch.push_back(ie->second);
batch.push_back(ep1->second);
insertRange[1] = ii;
}
insertRange[0] = ib->first;
insertRange[1] = prev->first;
this->trigger(src, PhraseModelEvent::ABOUT_TO_INSERT, idx, idx, insertRange);
orig.insert(orig.begin() + insertRange[0], batch.begin(), batch.end());
this->trigger(src, PhraseModelEvent::INSERT_FINISHED, idx, idx, insertRange);
ib = ie;
if (ib != insert.end())
{
++ib;
batch.clear();
entry = ep1;
}
// Now reconcile entries that have been reordered in orig.
auto ni = next.begin();
auto oi = orig.begin();
for (ii = 0; ni != next.end() && oi != orig.end(); ++ii)
{
if (*ni == *oi)
{ // Entries match.
++ni;
++oi;
}
else
{ // Reorder required.
auto mv = ni;
for (; mv != next.end() && *mv != *oi; ++mv)
{
// do nothing
}
if (mv == next.end())
{
smtkErrorMacro(smtk::io::Logger::instance(), "Mismatched entries in descriptive phrase.");
break;
}
else
{
// We have a start and destination location; see if we
// should batch move.
auto bi = oi;
auto ci = mv;
for (++bi, ++ci; bi != orig.end() && ci != next.end() && *bi == *ci; ++bi, ++ci)
{
// Do nothing (advancing to find batch size)
}
std::vector<int> moveRange(3);
moveRange[0] = static_cast<int>(oi - orig.begin());
moveRange[1] = static_cast<int>(bi == orig.end() ? orig.size() : bi - orig.begin());
moveRange[2] = static_cast<int>(mv - next.begin());
this->trigger(src, PhraseModelEvent::ABOUT_TO_MOVE, idx, idx, moveRange);
// Copy batch to destination (which must be *after* source)
orig.insert(
orig.begin() + moveRange[2], orig.begin() + moveRange[0], orig.begin() + moveRange[1]);
// Erase batch in its original location (we cannot use iterators in orig
// as they may have been invalidated by insertion).
orig.erase(orig.begin() + moveRange[0], orig.begin() + moveRange[1]);
this->trigger(src, PhraseModelEvent::MOVE_FINISHED, idx, idx, moveRange);
// Update iterator to continue search for relocated entries.
oi = orig.begin() + moveRange[1];
}
}
}
}
......
......@@ -180,6 +180,15 @@ bool ResourcePhraseContent::editColorValue(ContentType attr, const resource::Flo
return false;
}
smtk::resource::PersistentObjectPtr ResourcePhraseContent::relatedObject() const
{
if (m_resource)
{
return m_resource;
}
return this->PhraseContent::relatedObject();
}
smtk::resource::ResourcePtr ResourcePhraseContent::relatedResource() const
{
return m_resource;
......
......@@ -48,6 +48,7 @@ public:
bool editFlagValue(ContentType attr, int val) override;
bool editColorValue(ContentType attr, const resource::FloatList& val) override;
smtk::resource::PersistentObjectPtr relatedObject() const override;
smtk::resource::ResourcePtr relatedResource() const override;
void setMutability(int whatsMutable);
......
......@@ -331,6 +331,7 @@ SubphraseGenerator::Path SubphraseGenerator::indexOfObjectInParent(
smtk::attribute::AttributePtr attr;
smtk::mesh::ComponentPtr mcmp;
smtk::model::EntityPtr ment;
bool added = false;
// Determine if the component is a direct-ish child of parent
if (!actualParent->relatedComponent() && actualParent->relatedResource())
{
......@@ -343,10 +344,11 @@ SubphraseGenerator::Path SubphraseGenerator::indexOfObjectInParent(
!smtk::model::Model(ment).owningModel().isValid())))
{
PreparePath(result, parentPath, IndexFromTitle(comp->name(), actualParent->subphrases()));
added = true;
}
}
else if ((ment =
std::dynamic_pointer_cast<smtk::model::Entity>(actualParent->relatedComponent())))
if (!added &&
(ment = std::dynamic_pointer_cast<smtk::model::Entity>(actualParent->relatedComponent())))
{
bool shouldAdd = false;
auto parentEntity = ment;
......@@ -459,6 +461,7 @@ SubphraseGenerator::Path SubphraseGenerator::indexOfObjectInParent(
}
if (shouldAdd)
{
added = true;
PreparePath(result, parentPath, IndexFromTitle(comp->name(), actualParent->subphrases()));
}
}
......
......@@ -22,8 +22,13 @@
#include "smtk/attribute/Attribute.h"
#include "smtk/attribute/FileItem.h"
#include "smtk/attribute/IntItem.h"
#include "smtk/attribute/ModelEntityItem.h"
#include "smtk/attribute/StringItem.h"
#include "smtk/attribute/VoidItem.h"
#include "smtk/model/SessionRef.h"
#include "smtk/model/operators/EntityGroupOperation.h"
#include "smtk/common/testing/cxx/helpers.h"
#include "smtk/model/testing/cxx/helpers.h"
......@@ -96,6 +101,7 @@ int unitComponentPhraseModel(int argc, char* argv[])
auto registry = smtk::common::Registry<smtk::session::polygon::Registrar, smtk::resource::Manager,
smtk::operation::Manager>(rsrcMgr, operMgr);
// I. Construct a ComponentPhraseModel that displays edges and faces. Load some geometry.
auto phraseModel = smtk::view::ComponentPhraseModel::create();
std::multimap<std::string, std::string> filters;
filters.insert(
......@@ -121,28 +127,81 @@ int unitComponentPhraseModel(int argc, char* argv[])
}
});
smtkTest(!!rsrc, "Unable to discern that a resource was loaded.");
auto firstSize = phraseModel->root()->subphrases().size();
test(phraseModel->root()->root() == phraseModel->root(),
"Model's root phrase was not root of tree.");
phraseModel->root()->visitChildren(printer);
// II. Now change the filters to a reduced set and verify there are fewer phrases.
filters.erase(filters.begin());
std::cout << "---\n";
phraseModel->setComponentFilters(filters);
auto reducedSize = phraseModel->root()->subphrases().size();
std::cout << "initial size " << firstSize << " reduced to " << reducedSize << "\n";
smtkTest(reducedSize < firstSize, "Expected fewer phrases without the edge filter.");
phraseModel->root()->visitChildren(printer);
if (!dataArgs.empty())
{
// III. Test that we can set an explicit "active" resource that
// serves as the only source of components.
phraseModel->setOnlyShowActiveResourceComponents(true);
std::cout << "---\n";
// III.a. A null "active" resource
smtkTest(phraseModel->root()->subphrases().empty(),
"Expected an empty list with a null active resource");
"Expected an empty list with a null active resource.");
// III.b. A non-null "active" resource
phraseModel->setActiveResource(rsrc);
std::cout << "---\n";
phraseModel->root()->visitChildren(printer);
smtkTest(!phraseModel->root()->subphrases().empty(),
"Expected a non-empty list with a non-null active resource");
"Expected a non-empty list with a non-null active resource.");
smtkTest(phraseModel->root()->subphrases().size() == reducedSize,
"Expected the same number of phrases as earlier.");
// IV. Test updates due to an operation.
// Grab the faces as reported to us:
smtk::resource::ComponentArray faces;
smtk::model::EntityPtr fmod;
phraseModel->root()->visitChildren(
[&faces, &fmod](DescriptivePhrasePtr p, const std::vector<int>&) {
auto comp = p->relatedComponent();
faces.push_back(comp);
auto ment = std::dynamic_pointer_cast<smtk::model::Entity>(comp);
fmod = ment ? ment->owningModel() : nullptr;
return 0;
});
smtkTest(!fmod || !faces.empty(), "Cannot test grouping without groupees.");
// Add a filter so groups will be listed at the top level:
filters.insert(
std::make_pair(std::string("smtk::session::polygon::Resource"), std::string("group")));
phraseModel->setComponentFilters(filters);
auto sizeBeforeAdd = phraseModel->root()->subphrases().size();
// Create a new group
auto op = operMgr->create<smtk::model::EntityGroupOperation>();
auto pm = op->parameters();
pm->findString("Operation")->setValue("Create");
pm->findString("group name")->setValue("everything");
pm->findVoid("Face")->setIsEnabled(true);
// pm->findModelEntity("cell to add")->setObjectValues(faces.begin(), faces.end());
pm->associations()->appendObjectValue(fmod);
auto res = op->operate();
std::cout << "add group success "
<< (res->findInt("outcome")->value(0) ==
static_cast<int>(smtk::operation::Operation::Outcome::SUCCEEDED)
? "T"
: "F")
<< "\n";
(void)res;
auto sizeAfterAdd = phraseModel->root()->subphrases().size();
std::cout << "---\n";
phraseModel->root()->visitChildren(printer);
smtkTest(
sizeAfterAdd == sizeBeforeAdd + 1, "Adding a group should increase number of phrases by 1.");
// Don't leak
free(dataArgs[1]);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment