From cf54ebd6f3591c09a7d3c1e5fdde9d8be163f02d Mon Sep 17 00:00:00 2001 From: Mahmoud Badri Date: Fri, 7 Dec 2018 16:19:13 +0200 Subject: Fix undoing material type change doesn't update ref materials bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also removed a previous hot fix (QT3DS-2768) and some cleanups in the undo/redo system. Task-number: QT3DS-2827 Change-Id: Id1bb8e89c4121dd3cf447bf29c1a0d02d034e247 Reviewed-by: Tomi Korpipää Reviewed-by: Miikka Heikkinen --- .../Client/Code/Core/Commands/CmdDataModel.cpp | 30 ++++++---------- .../Client/Code/Core/Commands/CmdDataModel.h | 4 --- .../Client/Code/Core/Commands/CmdStack.cpp | 27 ++++++-------- src/Authoring/Client/Code/Core/Commands/CmdStack.h | 6 ++-- src/Authoring/Client/Code/Core/Doc/Doc.cpp | 13 ++++--- .../Client/Code/Core/Doc/DocumentEditor.cpp | 2 +- .../QT3DSDM/Systems/Qt3DSDMTransactions.h | 9 ----- .../Palettes/Inspector/InspectorControlModel.cpp | 1 + .../Palettes/Inspector/InspectorControlView.cpp | 42 +++++++++++----------- .../Palettes/Inspector/InspectorControlView.h | 2 ++ 10 files changed, 54 insertions(+), 82 deletions(-) diff --git a/src/Authoring/Client/Code/Core/Commands/CmdDataModel.cpp b/src/Authoring/Client/Code/Core/Commands/CmdDataModel.cpp index 1e521df8..47a44b51 100644 --- a/src/Authoring/Client/Code/Core/Commands/CmdDataModel.cpp +++ b/src/Authoring/Client/Code/Core/Commands/CmdDataModel.cpp @@ -64,9 +64,8 @@ void SApplicationState::Notify(const SApplicationState &inOther, CDoc &inDoc) // to a deleted item void SApplicationState::PreNotify(const SApplicationState &inOther, CDoc &inDoc) { - if (m_SelectedInstance != inOther.m_SelectedInstance) { + if (m_SelectedInstance != inOther.m_SelectedInstance) inDoc.DeselectAllItems(false); - } } CmdDataModel::CmdDataModel(CDoc &inDoc) @@ -96,25 +95,11 @@ bool CmdDataModel::ConsumerExists() const return m_Consumer != nullptr; } -void CmdDataModel::SetConsumer(ITransactionProducer *inProducer) -{ - if (!ConsumerExists()) - m_Consumer = std::make_shared(); - inProducer->SetConsumer(m_Consumer); -} - -void CmdDataModel::ReleaseConsumer(ITransactionProducer *inProducer, bool inRunNotifications) -{ - inProducer->SetConsumer(TTransactionConsumerPtr()); - if (inRunNotifications) - RunDoNotifications(); -} - void CmdDataModel::SetConsumer() { if (!ConsumerExists()) { - m_Doc.GetCore()->GetDispatch(); - SetConsumer(m_Doc.GetStudioSystem()); + m_Consumer = std::make_shared(); + m_Doc.GetStudioSystem()->SetConsumer(m_Consumer); m_Doc.GetAssetGraph()->SetConsumer(m_Consumer); m_BeforeDoAppState.Store(m_Doc); } @@ -123,11 +108,16 @@ void CmdDataModel::SetConsumer() void CmdDataModel::ReleaseConsumer(bool inRunNotifications) { if (ConsumerExists()) { - m_Doc.GetAssetGraph()->SetConsumer(TTransactionConsumerPtr()); + m_Doc.GetAssetGraph()->SetConsumer(nullptr); + if (HasTransactions()) m_Doc.SetModifiedFlag(true); m_AfterDoAppState.Store(m_Doc); - ReleaseConsumer(m_Doc.GetStudioSystem(), inRunNotifications); + + m_Doc.GetStudioSystem()->SetConsumer(nullptr); + + if (inRunNotifications) + RunDoNotifications(); } } diff --git a/src/Authoring/Client/Code/Core/Commands/CmdDataModel.h b/src/Authoring/Client/Code/Core/Commands/CmdDataModel.h index 7d601826..8bd8621f 100644 --- a/src/Authoring/Client/Code/Core/Commands/CmdDataModel.h +++ b/src/Authoring/Client/Code/Core/Commands/CmdDataModel.h @@ -87,10 +87,6 @@ protected: SApplicationState m_BeforeDoAppState; // The application state after this command. SApplicationState m_AfterDoAppState; - -private: - void SetConsumer(ITransactionProducer *inProducer); - void ReleaseConsumer(ITransactionProducer *inProducer, bool inRunNotifications); }; struct SScopedDataModelConsumer diff --git a/src/Authoring/Client/Code/Core/Commands/CmdStack.cpp b/src/Authoring/Client/Code/Core/Commands/CmdStack.cpp index 3530822f..76f51126 100644 --- a/src/Authoring/Client/Code/Core/Commands/CmdStack.cpp +++ b/src/Authoring/Client/Code/Core/Commands/CmdStack.cpp @@ -27,16 +27,13 @@ ** ****************************************************************************/ -#include "Qt3DSCommonPrecompile.h" #include "CmdStack.h" #include "Cmd.h" #include "CmdStackModifier.h" CCmdStack::CCmdStack() { - m_Listener = NULL; - m_MaxUndoStackSize = 100; - m_CommandStackModifier = NULL; + } CCmdStack::~CCmdStack() @@ -66,14 +63,13 @@ bool CCmdStack::ExecuteCommand(CCmd *inCommand) // Execute the command. unsigned long theUpdateMask = inCommand->Do(); - // If the listener is not null then do the notifications. - if (m_Listener != NULL) { + if (m_Listener) { m_Listener->CommandUpdate(theUpdateMask); // Set the modified flag if it needs to be set. if (inCommand->ShouldSetModifiedFlag()) { - m_Listener->SetCommandModifiedFlag(TRUE); + m_Listener->SetCommandModifiedFlag(true); } } @@ -135,11 +131,11 @@ bool CCmdStack::ExecuteCommand(CCmd *inCommand) //============================================================================= void CCmdStack::Undo() { - if (m_CommandStackModifier) { if (m_CommandStackModifier->PreUndo() == false) return; } + if (m_UndoList.size() > 0) { m_undoingOrRedoing = true; CCmd *theLastCommand = m_UndoList.back(); @@ -147,7 +143,6 @@ void CCmdStack::Undo() unsigned long theUpdateMask = theLastCommand->Undo(); - // Once a command is undone then it is considered committed. Prevents merging after this has // been redone. theLastCommand->SetCommitted(true); @@ -155,13 +150,12 @@ void CCmdStack::Undo() m_RedoList.push_back(theLastCommand); // If the listener is not null then do the notifications. - if (m_Listener != NULL) { + if (m_Listener) { m_Listener->CommandUpdate(theUpdateMask); // Set the modified flag if it needs to be set. - if (theLastCommand->ShouldSetModifiedFlag()) { - m_Listener->SetCommandModifiedFlag(TRUE); - } + if (theLastCommand->ShouldSetModifiedFlag()) + m_Listener->SetCommandModifiedFlag(true); } m_undoingOrRedoing = false; } @@ -186,13 +180,12 @@ void CCmdStack::Redo() m_UndoList.push_back(theLastCommand); // If the listener is not null then do the notifications. - if (m_Listener != NULL) { + if (m_Listener) { m_Listener->CommandUpdate(theUpdateMask); // Set the modified flag if it needs to be set. - if (theLastCommand->ShouldSetModifiedFlag()) { - m_Listener->SetCommandModifiedFlag(TRUE); - } + if (theLastCommand->ShouldSetModifiedFlag()) + m_Listener->SetCommandModifiedFlag(true); } m_undoingOrRedoing = false; } diff --git a/src/Authoring/Client/Code/Core/Commands/CmdStack.h b/src/Authoring/Client/Code/Core/Commands/CmdStack.h index 7388703d..9597f5a6 100644 --- a/src/Authoring/Client/Code/Core/Commands/CmdStack.h +++ b/src/Authoring/Client/Code/Core/Commands/CmdStack.h @@ -102,15 +102,15 @@ public: protected: void EmptyUndoStack(); - ICmdStackModifier *m_CommandStackModifier; + ICmdStackModifier *m_CommandStackModifier = nullptr; TCmdList m_UndoList; TCmdList m_RedoList; bool m_undoingOrRedoing = false; - unsigned long m_MaxUndoStackSize; + unsigned long m_MaxUndoStackSize = 100; - CModificationListener *m_Listener; + CModificationListener *m_Listener = nullptr; }; #endif // INCLUDED_CMD_STACK_H diff --git a/src/Authoring/Client/Code/Core/Doc/Doc.cpp b/src/Authoring/Client/Code/Core/Doc/Doc.cpp index 14f0a77a..15240e2a 100644 --- a/src/Authoring/Client/Code/Core/Doc/Doc.cpp +++ b/src/Authoring/Client/Code/Core/Doc/Doc.cpp @@ -427,7 +427,7 @@ Q3DStudio::IDocumentEditor &CDoc::OpenTransaction(const QString &inCmdName, cons ++m_TransactionDepth; if (m_TransactionDepth == 1) { assert(!m_OpenTransaction); - m_OpenTransaction = std::make_shared(std::ref(*this)); + m_OpenTransaction = std::make_shared(*this); m_OpenTransaction->SetName(inCmdName); m_OpenTransaction->SetConsumer(); m_Core->SetCommandStackModifier(this); @@ -442,9 +442,9 @@ Q3DStudio::IDocumentEditor &CDoc::OpenTransaction(const QString &inCmdName, cons qCInfo(qt3ds::TRACE_INFO) << inFile << "(" << inLine << "): Open Transaction: " << inCmdName; - if (!m_SceneEditor) { + if (!m_SceneEditor) m_SceneEditor = Q3DStudio::IInternalDocumentEditor::CreateEditor(*this); - } + return *m_SceneEditor; } @@ -484,9 +484,9 @@ void CDoc::IKnowWhatIAmDoingForceCloseTransaction() qCInfo(qt3ds::TRACE_INFO) << "Closing transaction"; // Ensure hasTransaction will return false right at this second. std::shared_ptr theTransaction(m_OpenTransaction); - m_OpenTransaction = std::shared_ptr(); + m_OpenTransaction.reset(); - m_Core->SetCommandStackModifier(NULL); + m_Core->SetCommandStackModifier(nullptr); // Release the consumer without running notifications because our command will run // the notifications when it first gets executed. theTransaction->ReleaseConsumer(false); @@ -1231,7 +1231,6 @@ void CDoc::OnSlideDeleted(qt3dsdm::Qt3DSDMSlideHandle inSlide) } void CDoc::OnInstanceDeleted(qt3dsdm::Qt3DSDMInstanceHandle inInstance) { - qt3dsdm::TTransactionConsumerPtr theConsumer = m_StudioSystem->GetFullSystem()->GetConsumer(); if (GetSelectedInstance() == inInstance) DeselectAllItems(); @@ -1744,7 +1743,7 @@ void CDoc::CloseDocument() // selection would be invalid from this point onwards DeselectAllItems(); - m_SceneEditor = std::shared_ptr(); + m_SceneEditor.reset(); if (m_DocumentBufferCache) { // Ensure old buffers aren't picked up for the same relative path. m_DocumentBufferCache->Clear(); diff --git a/src/Authoring/Client/Code/Core/Doc/DocumentEditor.cpp b/src/Authoring/Client/Code/Core/Doc/DocumentEditor.cpp index 3171f50c..d7cdfb73 100644 --- a/src/Authoring/Client/Code/Core/Doc/DocumentEditor.cpp +++ b/src/Authoring/Client/Code/Core/Doc/DocumentEditor.cpp @@ -5703,5 +5703,5 @@ void CUpdateableDocumentEditor::RollbackEditor() std::shared_ptr IInternalDocumentEditor::CreateEditor(CDoc &doc) { - return std::shared_ptr(new CDocEditor(doc)); + return std::make_shared(doc); } diff --git a/src/Authoring/QT3DSDM/Systems/Qt3DSDMTransactions.h b/src/Authoring/QT3DSDM/Systems/Qt3DSDMTransactions.h index f3d47d7b..f6ed3da0 100644 --- a/src/Authoring/QT3DSDM/Systems/Qt3DSDMTransactions.h +++ b/src/Authoring/QT3DSDM/Systems/Qt3DSDMTransactions.h @@ -187,15 +187,6 @@ struct CTransactionConsumer : public ITransactionConsumer } }; -struct SIgnorantTransactionConsumer : public ITransactionConsumer -{ - void OnTransaction(qt3dsdm::TTransactionPtr) override {} - // Notifications to be sent for undo/redo These are used to - // notify clients that something is different. - void OnDoNotification(std::function) override {} - void OnUndoNotification(std::function) override {} -}; - template inline void RunWithConsumer(TTransactionConsumerPtr inConsumer, TTransactionType inTransaction) { diff --git a/src/Authoring/Studio/Palettes/Inspector/InspectorControlModel.cpp b/src/Authoring/Studio/Palettes/Inspector/InspectorControlModel.cpp index 2335ba2d..0f4e2032 100644 --- a/src/Authoring/Studio/Palettes/Inspector/InspectorControlModel.cpp +++ b/src/Authoring/Studio/Palettes/Inspector/InspectorControlModel.cpp @@ -1608,6 +1608,7 @@ void InspectorControlModel::setShaderValue(long instance, int handle, const QVar const auto doc = g_StudioApp.GetCore()->GetDoc(); const auto bridge = doc->GetStudioSystem()->GetClientDataModelBridge(); + Q3DStudio::SCOPED_DOCUMENT_EDITOR(*doc, QObject::tr("Set Material Type")) ->SetMaterialType(instance, v); diff --git a/src/Authoring/Studio/Palettes/Inspector/InspectorControlView.cpp b/src/Authoring/Studio/Palettes/Inspector/InspectorControlView.cpp index e427c2de..361ad0e0 100644 --- a/src/Authoring/Studio/Palettes/Inspector/InspectorControlView.cpp +++ b/src/Authoring/Studio/Palettes/Inspector/InspectorControlView.cpp @@ -121,6 +121,24 @@ void InspectorControlView::OnNewPresentation() m_DirectoryConnection = g_StudioApp.getDirectoryWatchingSystem().AddDirectory( g_StudioApp.GetCore()->getProjectFile().getProjectPath(), std::bind(&InspectorControlView::onFilesChanged, this, std::placeholders::_1)); + + Q3DStudio::CGraph &theGraph(*g_StudioApp.GetCore()->GetDoc()->GetAssetGraph()); + m_connections.push_back(theGraph.ConnectChildAdded( + std::bind(&InspectorControlView::onChildAdded, this, std::placeholders::_2))); +} + +void InspectorControlView::onChildAdded(int inChild) +{ + const auto doc = g_StudioApp.GetCore()->GetDoc(); + const auto bridge = doc->GetStudioSystem()->GetClientDataModelBridge(); + if (bridge->IsCustomMaterialInstance(inChild)) { + QVector refMats; + doc->getSceneReferencedMaterials(doc->GetSceneInstance(), refMats); + for (auto &refMat : qAsConst(refMats)) { + if ((int)bridge->getMaterialReference(refMat) == inChild) + g_StudioApp.GetCore()->GetDispatch()->FireImmediateRefreshInstance(refMat); + } + } } void InspectorControlView::OnClosingPresentation() @@ -129,6 +147,7 @@ void InspectorControlView::OnClosingPresentation() // presentations count as subpresentations delete m_imageChooserView; m_fileList.clear(); + m_connections.clear(); } void InspectorControlView::OnTimeChanged() @@ -158,29 +177,10 @@ void InspectorControlView::onFilesChanged( g_StudioApp.GetCore()->GetDoc()->GetDocumentDirectory(), record.m_File)); - if (record.m_ModificationType == Q3DStudio::FileModificationType::Created) { + if (record.m_ModificationType == Q3DStudio::FileModificationType::Created) qt3dsdm::binary_sort_insert_unique(m_fileList, relativePath); - } else if (record.m_ModificationType - == Q3DStudio::FileModificationType::Destroyed) { + else if (record.m_ModificationType == Q3DStudio::FileModificationType::Destroyed) qt3dsdm::binary_sort_erase(m_fileList, relativePath); - } else if (record.m_ModificationType == Q3DStudio::FileModificationType::Modified) { - // TODO: Hot fix for a case when undoing a material type change does not update - // in the scene (QT3DS-2768) (any better solution?) - // This fix should be checked and removed if not needed after (QT3DS-2827) is - // fixed - const auto doc = g_StudioApp.GetCore()->GetDoc(); - const auto bridge = doc->GetStudioSystem()->GetClientDataModelBridge(); - auto mat = doc->getSceneEditor()->getMaterial(record.m_File.toQString()); - QVector refMats; - doc->getSceneReferencedMaterials(doc->GetSceneInstance(), refMats); - for (auto &refMat : qAsConst(refMats)) { - if (bridge->getMaterialReference(refMat) == mat) { - const auto dispatch = g_StudioApp.GetCore()->GetDispatch(); - dispatch->FireImmediateRefreshInstance(refMat); - break; - } - } - } } else if (isInList(fontExtensions, record.m_File.GetExtension())) { if (record.m_ModificationType == Q3DStudio::FileModificationType::Created || record.m_ModificationType == Q3DStudio::FileModificationType::Destroyed) { diff --git a/src/Authoring/Studio/Palettes/Inspector/InspectorControlView.h b/src/Authoring/Studio/Palettes/Inspector/InspectorControlView.h index 400a26b4..3d62938f 100644 --- a/src/Authoring/Studio/Palettes/Inspector/InspectorControlView.h +++ b/src/Authoring/Studio/Palettes/Inspector/InspectorControlView.h @@ -120,6 +120,7 @@ private: bool canOpenInInspector(int instance, int handle) const; void openInInspector(); void onInstancePropertyValueChanged(qt3dsdm::Qt3DSDMPropertyHandle propertyHandle); + void onChildAdded(int inChild); std::shared_ptr m_selectionChangedConnection; std::shared_ptr m_timeChanged; @@ -138,6 +139,7 @@ private: QPointer m_dataInputChooserView; std::vector m_fileList; MouseHelper m_mouseHelper; + std::vector> m_connections; int m_instance; int m_handle; -- cgit v1.2.3