From 5048bf16cb62260c03c7f47720ca63de61aaa8ba Mon Sep 17 00:00:00 2001 From: Cain Kilgore Date: Mon, 12 Feb 2018 23:01:19 +0000 Subject: [PATCH 1/6] Fixed Async Prompts/Assets/Dialogs --- interface/src/scripting/WindowScriptingInterface.cpp | 11 +++++++---- libraries/ui/src/OffscreenUi.cpp | 9 ++++++--- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/interface/src/scripting/WindowScriptingInterface.cpp b/interface/src/scripting/WindowScriptingInterface.cpp index fc45555bf9..d61823e9ee 100644 --- a/interface/src/scripting/WindowScriptingInterface.cpp +++ b/interface/src/scripting/WindowScriptingInterface.cpp @@ -120,16 +120,19 @@ QScriptValue WindowScriptingInterface::confirm(const QString& message) { /// \param const QString& defaultText default text in the text box /// \return QScriptValue string text value in text box if the dialog was accepted, `null` otherwise. QScriptValue WindowScriptingInterface::prompt(const QString& message, const QString& defaultText) { - bool ok = false; - QString result = OffscreenUi::getText(nullptr, "", message, QLineEdit::Normal, defaultText, &ok); - return ok ? QScriptValue(result) : QScriptValue::NullValue; + QString result = OffscreenUi::getText(nullptr, "", message, QLineEdit::Normal, defaultText); + if (QScriptValue(result).equals("")) { + return QScriptValue::NullValue; + } + return QScriptValue(result); } /// Display a prompt with a text box /// \param const QString& message message to display /// \param const QString& defaultText default text in the text box void WindowScriptingInterface::promptAsync(const QString& message, const QString& defaultText) { - ModalDialogListener* dlg = OffscreenUi::getTextAsync(nullptr, "", message, QLineEdit::Normal, defaultText); + bool ok = false; + ModalDialogListener* dlg = OffscreenUi::getTextAsync(nullptr, "", message, QLineEdit::Normal, defaultText, &ok); connect(dlg, &ModalDialogListener::response, this, [=] (QVariant result) { disconnect(dlg, &ModalDialogListener::response, this, nullptr); emit promptTextChanged(result.toString()); diff --git a/libraries/ui/src/OffscreenUi.cpp b/libraries/ui/src/OffscreenUi.cpp index 221f5013bf..2bea2bbd91 100644 --- a/libraries/ui/src/OffscreenUi.cpp +++ b/libraries/ui/src/OffscreenUi.cpp @@ -336,10 +336,11 @@ class InputDialogListener : public ModalDialogListener { return; } connect(_dialog, SIGNAL(selected(QVariant)), this, SLOT(onSelected(const QVariant&))); + connect(_dialog, SIGNAL(canceled()), this, SLOT(onSelected())); } private slots: - void onSelected(const QVariant& result) { + void onSelected(const QVariant& result = "") { _result = result; auto offscreenUi = DependencyManager::get(); emit response(_result); @@ -700,10 +701,11 @@ class FileDialogListener : public ModalDialogListener { return; } connect(_dialog, SIGNAL(selectedFile(QVariant)), this, SLOT(onSelectedFile(QVariant))); + connect(_dialog, SIGNAL(canceled()), this, SLOT(onSelectedFile())); } private slots: - void onSelectedFile(QVariant file) { + void onSelectedFile(QVariant file = "") { _result = file.toUrl().toLocalFile(); _finished = true; auto offscreenUi = DependencyManager::get(); @@ -949,10 +951,11 @@ class AssetDialogListener : public ModalDialogListener { return; } connect(_dialog, SIGNAL(selectedAsset(QVariant)), this, SLOT(onSelectedAsset(QVariant))); + connect(_dialog, SIGNAL(canceled()), this, SLOT(onSelectedAsset())); } private slots: - void onSelectedAsset(QVariant asset) { + void onSelectedAsset(QVariant asset = "") { _result = asset; auto offscreenUi = DependencyManager::get(); emit response(_result); From 246ac25d0ab0b6fc5afdc48048e31d5d085f9d9b Mon Sep 17 00:00:00 2001 From: samcake Date: Wed, 14 Feb 2018 17:36:46 -0800 Subject: [PATCH 2/6] Introducing a way to abort a task from one of its job, using it to skip highlight tasks if nothing to highlight --- .../render-utils/src/HighlightEffect.cpp | 4 +++ libraries/task/src/task/Task.cpp | 35 +++++++++++++++++++ libraries/task/src/task/Task.h | 20 ++++++++--- 3 files changed, 55 insertions(+), 4 deletions(-) create mode 100644 libraries/task/src/task/Task.cpp diff --git a/libraries/render-utils/src/HighlightEffect.cpp b/libraries/render-utils/src/HighlightEffect.cpp index 9501a74d52..de13188733 100644 --- a/libraries/render-utils/src/HighlightEffect.cpp +++ b/libraries/render-utils/src/HighlightEffect.cpp @@ -453,6 +453,10 @@ void SelectionToHighlight::run(const render::RenderContextPointer& renderContext } } } + + if (numLayers == 0) { + renderContext->abortTask(); + } } void ExtractSelectionName::run(const render::RenderContextPointer& renderContext, const Inputs& inputs, Outputs& outputs) { diff --git a/libraries/task/src/task/Task.cpp b/libraries/task/src/task/Task.cpp new file mode 100644 index 0000000000..bb65f15b7c --- /dev/null +++ b/libraries/task/src/task/Task.cpp @@ -0,0 +1,35 @@ +// +// Task.cpp +// task/src/task +// +// Created by Sam Gateau on 2/14/2018. +// Copyright 2018 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// +#include "Task.h" + +using namespace task; + +JobContext::JobContext(const QLoggingCategory& category) : + profileCategory(category) { + assert(&category); +} + +JobContext::~JobContext() { +} + +void JobContext::resetTaskFlow() { + _doAbortTask = false; +} + +void JobContext::abortTask() { + _doAbortTask = true; +} + +bool JobContext::doAbortTask() const { + return _doAbortTask; +} + + diff --git a/libraries/task/src/task/Task.h b/libraries/task/src/task/Task.h index b75f6d7321..1f1fb79ee1 100644 --- a/libraries/task/src/task/Task.h +++ b/libraries/task/src/task/Task.h @@ -29,13 +29,21 @@ class JobNoIO {}; class JobContext { public: - JobContext(const QLoggingCategory& category) : profileCategory(category) { - assert(&category); - } - virtual ~JobContext() {} + JobContext(const QLoggingCategory& category); + virtual ~JobContext(); std::shared_ptr jobConfig { nullptr }; const QLoggingCategory& profileCategory; + + // control flow commands + void resetTaskFlow(); + void abortTask(); + + // Check command flow + bool doAbortTask() const; + +protected: + bool _doAbortTask{ false }; }; using JobContextPointer = std::shared_ptr; @@ -308,6 +316,10 @@ public: if (config->alwaysEnabled || config->enabled) { for (auto job : TaskConcept::_jobs) { job.run(jobContext); + if (jobContext->doAbortTask()) { + jobContext->resetTaskFlow(); + return; + } } } } From b21b98c81066a85be556dec87a238650729dfe00 Mon Sep 17 00:00:00 2001 From: samcake Date: Thu, 15 Feb 2018 11:24:20 -0800 Subject: [PATCH 3/6] Add a way to early abort a task from a Job, apply that to the highlight effect to shave unecessary work --- .../render-utils/src/HighlightEffect.cpp | 2 +- libraries/task/src/task/Task.cpp | 6 ++-- libraries/task/src/task/Task.h | 28 +++++++++++++------ 3 files changed, 23 insertions(+), 13 deletions(-) diff --git a/libraries/render-utils/src/HighlightEffect.cpp b/libraries/render-utils/src/HighlightEffect.cpp index de13188733..0bf8e7fa71 100644 --- a/libraries/render-utils/src/HighlightEffect.cpp +++ b/libraries/render-utils/src/HighlightEffect.cpp @@ -455,7 +455,7 @@ void SelectionToHighlight::run(const render::RenderContextPointer& renderContext } if (numLayers == 0) { - renderContext->abortTask(); + renderContext->taskFlow.abortTask(); } } diff --git a/libraries/task/src/task/Task.cpp b/libraries/task/src/task/Task.cpp index bb65f15b7c..621d77d7bf 100644 --- a/libraries/task/src/task/Task.cpp +++ b/libraries/task/src/task/Task.cpp @@ -20,15 +20,15 @@ JobContext::JobContext(const QLoggingCategory& category) : JobContext::~JobContext() { } -void JobContext::resetTaskFlow() { +void TaskFlow::reset() { _doAbortTask = false; } -void JobContext::abortTask() { +void TaskFlow::abortTask() { _doAbortTask = true; } -bool JobContext::doAbortTask() const { +bool TaskFlow::doAbortTask() const { return _doAbortTask; } diff --git a/libraries/task/src/task/Task.h b/libraries/task/src/task/Task.h index 1f1fb79ee1..34cdbb5439 100644 --- a/libraries/task/src/task/Task.h +++ b/libraries/task/src/task/Task.h @@ -27,6 +27,21 @@ template class JobT; template class TaskT; class JobNoIO {}; +class TaskFlow { +public: + TaskFlow() = default; + ~TaskFlow() = default; + + // called after each job + void reset(); + + void abortTask(); + bool doAbortTask() const; + +protected: + bool _doAbortTask{ false }; +}; + class JobContext { public: JobContext(const QLoggingCategory& category); @@ -35,15 +50,10 @@ public: std::shared_ptr jobConfig { nullptr }; const QLoggingCategory& profileCategory; - // control flow commands - void resetTaskFlow(); - void abortTask(); - - // Check command flow - bool doAbortTask() const; + // Task flow control + TaskFlow taskFlow{}; protected: - bool _doAbortTask{ false }; }; using JobContextPointer = std::shared_ptr; @@ -316,8 +326,8 @@ public: if (config->alwaysEnabled || config->enabled) { for (auto job : TaskConcept::_jobs) { job.run(jobContext); - if (jobContext->doAbortTask()) { - jobContext->resetTaskFlow(); + if (jobContext->taskFlow.doAbortTask()) { + jobContext->taskFlow.reset(); return; } } From c70b655fbc5a170e180e9d99f800fb39d73c6af2 Mon Sep 17 00:00:00 2001 From: samcake Date: Thu, 15 Feb 2018 11:55:16 -0800 Subject: [PATCH 4/6] A bit more explanations to the Task Flow class --- libraries/task/src/task/Task.h | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/libraries/task/src/task/Task.h b/libraries/task/src/task/Task.h index 34cdbb5439..08df55dddd 100644 --- a/libraries/task/src/task/Task.h +++ b/libraries/task/src/task/Task.h @@ -27,21 +27,32 @@ template class JobT; template class TaskT; class JobNoIO {}; +// Task Flow control class is a simple per value object used to communicate flow control commands trhough the graph of tasks. +// From within the Job::Run function, you can access it from the JobCOntext and issue commands which will be picked up by the Task calling for the Job run. +// This is first introduced to provide a way to abort all the work from within a task job. see the "abortTask" call class TaskFlow { public: + // A job that wants to abort the rest of the other jobs execution in a task would issue that call "abortTask" and let the task early exit for this run. + // All the varyings produced by the aborted branch of jobs are left unmodified. + void abortTask(); + + // called by the task::run to perform flow control + // This should be considered private but still need to be accessible from the Task class TaskFlow() = default; ~TaskFlow() = default; - - // called after each job void reset(); - - void abortTask(); bool doAbortTask() const; protected: bool _doAbortTask{ false }; }; +// JobContext class is the base calss for the context object which is passed through all the Job::run calls thoughout the graph of jobs +// It is used to communicate to the job::run its context and various state information the job relies on. +// It specifically provide access to: +// - The taskFlow object allowing for messaging control flow commands from within a Job::run +// - The current Config object attached to the Job::run currently called. +// The JobContext can be derived to add more global state to it that Jobs can access class JobContext { public: JobContext(const QLoggingCategory& category); From e4db09fef8845c10b8e9872d00c5d8e56e67e75e Mon Sep 17 00:00:00 2001 From: Dante Ruiz Date: Thu, 15 Feb 2018 15:02:45 -0800 Subject: [PATCH 5/6] don't update avatar entities if the avatars ID is AVATAR_SELF_ID --- libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp index 86635cd3bf..1b1511e2c6 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp @@ -219,7 +219,7 @@ void Avatar::updateAvatarEntities() { return; } - if (getID() == QUuid()) { + if (getID() == QUuid() || getID() == AVATAR_SELF_ID) { return; // wait until MyAvatar gets an ID before doing this. } From 81662b5edc3180e1ae2221c27374bd16437aaae1 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 15 Feb 2018 19:14:14 -0800 Subject: [PATCH 6/6] don't overwrite general description object with filtered one --- domain-server/src/DomainServerSettingsManager.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/domain-server/src/DomainServerSettingsManager.cpp b/domain-server/src/DomainServerSettingsManager.cpp index 8febbd5769..3fa36ceb90 100644 --- a/domain-server/src/DomainServerSettingsManager.cpp +++ b/domain-server/src/DomainServerSettingsManager.cpp @@ -1196,8 +1196,8 @@ bool DomainServerSettingsManager::handleAuthenticatedHTTPRequest(HTTPConnection } bool DomainServerSettingsManager::restoreSettingsFromObject(QJsonObject settingsToRestore, SettingsType settingsType) { - QJsonArray& filteredDescriptionArray = settingsType == DomainSettings - ? _domainSettingsDescription : _contentSettingsDescription; + QJsonArray* filteredDescriptionArray = settingsType == DomainSettings + ? &_domainSettingsDescription : &_contentSettingsDescription; // grab a copy of the current config before restore, so that we can back out if something bad happens during QVariantMap preRestoreConfig = _configMap.getConfig(); @@ -1206,7 +1206,7 @@ bool DomainServerSettingsManager::restoreSettingsFromObject(QJsonObject settings // enumerate through the settings in the description // if we have one in the restore then use it, otherwise clear it from current settings - foreach(const QJsonValue& descriptionGroupValue, filteredDescriptionArray) { + foreach(const QJsonValue& descriptionGroupValue, *filteredDescriptionArray) { QJsonObject descriptionGroupObject = descriptionGroupValue.toObject(); QString groupKey = descriptionGroupObject[DESCRIPTION_NAME_KEY].toString(); QJsonArray descriptionGroupSettings = descriptionGroupObject[DESCRIPTION_SETTINGS_KEY].toArray(); @@ -1328,15 +1328,15 @@ QJsonObject DomainServerSettingsManager::settingsResponseObjectForType(const QSt const QString AFFECTED_TYPES_JSON_KEY = "assignment-types"; // only enumerate the requested settings type (domain setting or content setting) - QJsonArray& filteredDescriptionArray = _descriptionArray; + QJsonArray* filteredDescriptionArray = &_descriptionArray; if (includeDomainSettings && !includeContentSettings) { - filteredDescriptionArray = _domainSettingsDescription; + filteredDescriptionArray = &_domainSettingsDescription; } else if (includeContentSettings && !includeDomainSettings) { - filteredDescriptionArray = _contentSettingsDescription; + filteredDescriptionArray = &_contentSettingsDescription; } // enumerate the groups in the potentially filtered object to find which settings to pass - foreach(const QJsonValue& groupValue, filteredDescriptionArray) { + foreach(const QJsonValue& groupValue, *filteredDescriptionArray) { QJsonObject groupObject = groupValue.toObject(); QString groupKey = groupObject[DESCRIPTION_NAME_KEY].toString(); QJsonArray groupSettingsArray = groupObject[DESCRIPTION_SETTINGS_KEY].toArray();