diff --git a/src/MainComponent.cpp b/src/MainComponent.cpp index d0d2600c..9280b7ac 100644 --- a/src/MainComponent.cpp +++ b/src/MainComponent.cpp @@ -64,7 +64,8 @@ void MainComponent::paintOverChildren(Graphics& g) // Add extra highlights to the cutout path for (auto& rect : tutorialExtraHighlights) { - highlightPath.addRoundedRectangle(rect.toFloat(), 5.0f); + if (! rect.isEmpty()) + highlightPath.addRoundedRectangle(rect.toFloat(), 5.0f); } backgroundPath.setUsingNonZeroWinding(false); @@ -73,11 +74,16 @@ void MainComponent::paintOverChildren(Graphics& g) g.fillPath(backgroundPath); g.setColour(Colours::white); - g.drawRoundedRectangle(tutorialHighlightRect.toFloat(), 5.0f, 2.0f); + + // An empty rectangle is a region the current step has nothing to + // point at; outlining it would leave a stray mark in the corner + if (! tutorialHighlightRect.isEmpty()) + g.drawRoundedRectangle(tutorialHighlightRect.toFloat(), 5.0f, 2.0f); for (auto& rect : tutorialExtraHighlights) { - g.drawRoundedRectangle(rect.toFloat(), 5.0f, 2.0f); + if (! rect.isEmpty()) + g.drawRoundedRectangle(rect.toFloat(), 5.0f, 2.0f); } } } @@ -430,32 +436,63 @@ void MainComponent::setTutorialExtraHighlights(std::vector> bound void MainComponent::ensureTutorialModelLoaded() { + // Loading is asynchronous, so without this guard every repeated call that + // arrives before the first load finishes - clicking Next again, say - would + // open yet another tab or start yet another load. + if (tutorialModelLoadInFlight) + return; + auto* tab = getCurrentModelTab(); if (tab == nullptr) { + // createNewTab() selects the tab it creates, which is what the tutorial + // steps compute their highlights against; leave it selected. tab = modelTabs.createNewTab(); - modelTabs.setCurrentTabIndex(0); - - if (welcomeWindow != nullptr) - tab->addChangeListener(welcomeWindow.get()); + tutorialCreatedTab = tab; + } - if (tab != nullptr) - tab->loadDefaultModel(); + if (tab->isModelLoaded()) return; - } - if (! tab->isModelLoaded()) - tab->loadDefaultModel(); + tutorialModelLoadInFlight = true; + + Component::SafePointer safeThis(this); + tab->onNextModelLoadComplete( + [safeThis](ModelTab*, bool) + { + if (safeThis != nullptr) + safeThis->tutorialModelLoadInFlight = false; + }); + + tab->loadDefaultModel(); } void MainComponent::resetTutorialAutoLoadedModel() { - if (auto* tab = getCurrentModelTab()) - { - if (tab->isModelLoaded() && tab->getLoadedPath() == TutorialConstants::fallbackModelPath) - tab->resetState(); - } + // Close the tab the tutorial opened on the user's behalf. Resetting it in + // place would leave a blank tab behind, since a model tab has no model + // selection of its own - models are chosen on the Home tab. + if (auto* tab = tutorialCreatedTab.getComponent()) + modelTabs.closeTab(tab); + + tutorialCreatedTab = nullptr; +} + +void MainComponent::ensureMediaClipboardVisible() +{ + if (! showMediaClipboard) + viewMediaClipboardCallback(); +} + +Rectangle MainComponent::getTabBarBounds() +{ + auto& tabBar = modelTabs.getTabbedButtonBar(); + + if (tabBar.getNumTabs() == 0) + return {}; + + return getLocalArea(&tabBar, tabBar.getLocalBounds()); } Rectangle MainComponent::getModelSelectBounds() @@ -469,9 +506,14 @@ Rectangle MainComponent::getModelSelectBounds() if (auto* tab = getCurrentModelTab()) { auto bounds = tab->getModelSelectBounds(); - return getLocalArea(tab, bounds); + + if (! bounds.isEmpty()) + return getLocalArea(tab, bounds); } - return {}; + + // Models are selected on the Home tab, so while a model tab is showing, + // point at the tab bar that leads back to it. + return getTabBarBounds(); } Rectangle MainComponent::getControlsBounds() @@ -680,5 +722,10 @@ void MainComponent::changeListenerCallback(ChangeBroadcaster* source) if (source == &modelTabs) { updateWindowConstraints(); + + // Model tabs are created and closed while the tutorial is open, so it + // follows the container rather than subscribing to individual tabs. + if (welcomeWindow != nullptr) + welcomeWindow->notifyModelStateChanged(); } } diff --git a/src/MainComponent.h b/src/MainComponent.h index 4d777918..6a495529 100644 --- a/src/MainComponent.h +++ b/src/MainComponent.h @@ -79,13 +79,16 @@ class MainComponent : public Component, void setTutorialHighlight(Rectangle bounds); void setTutorialExtraHighlights(std::vector> bounds); void ensureTutorialModelLoaded(); + bool isTutorialModelLoadInFlight() const { return tutorialModelLoadInFlight; } void resetTutorialAutoLoadedModel(); + void ensureMediaClipboardVisible(); + + ModelTab* getCurrentModelTab() const; + ModelTab* getFirstModelTab() const; - ModelTab* getCurrentModelTab() const; - ModelTab* getFirstModelTab() const; - // Bounds accessors for tutorial steps (public for WelcomeWindow) + Rectangle getTabBarBounds(); Rectangle getModelSelectBounds(); Rectangle getControlsBounds(); Rectangle getInputTrackBounds(); @@ -129,7 +132,7 @@ class MainComponent : public Component, // Miscellaneous //void focusCallback(); - void changeListenerCallback(ChangeBroadcaster* source) override; + void changeListenerCallback(ChangeBroadcaster* source) override; /* Interface */ @@ -158,12 +161,18 @@ class MainComponent : public Component, StatusAreaWidget statusAreaWidget; MediaClipboardWidget mediaClipboardWidget; - bool isTutorialActive = false; - Rectangle tutorialHighlightRect; - std::vector> tutorialExtraHighlights; - std::unique_ptr welcomeWindow; - - SharedResourcePointer sharedTokens; + bool isTutorialActive = false; + Rectangle tutorialHighlightRect; + std::vector> tutorialExtraHighlights; + std::unique_ptr welcomeWindow; + + // Set while the tutorial's fallback model is loading, so that repeated + // requests to load it do not stack up. The tab the tutorial opened on the + // user's behalf is remembered so that it can be closed again at the end. + bool tutorialModelLoadInFlight = false; + Component::SafePointer tutorialCreatedTab; + + SharedResourcePointer sharedTokens; SharedResourcePointer statusMessage; JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR(MainComponent) diff --git a/src/Model.h b/src/Model.h index eb205086..33167972 100644 --- a/src/Model.h +++ b/src/Model.h @@ -142,6 +142,10 @@ class Model return result; } + // Requests made while querying belong to this model as well, even though + // the client only replaces the current one once loading has succeeded + tempClient->setRequestRegistry(&requestRegistry); + setStatus(ModelStatus::QUERYING_CONTROLS); // Initialize empty dictionary to hold query response @@ -347,6 +351,13 @@ class Model return result; } + // Aborts any request this model currently has in flight, locally. Unlike + // cancel(), this needs neither the server nor a reachable network, and it + // unblocks the worker thread that is waiting on the connection. Once aborted + // the model will not open any further connections, so this is only for + // models that are being discarded (see ModelTab::abandon). + void abortActiveRequests() { requestRegistry.abortActiveRequests(); } + OpResult cancel() { setStatus(ModelStatus::CANCELING); @@ -617,6 +628,9 @@ class Model ModelStatus status; + // Declared before the client so that it is destroyed after it + RequestRegistry requestRegistry; + std::unique_ptr client; String loadedPath; diff --git a/src/ModelTab.h b/src/ModelTab.h index a8155bdf..1e6dfb8a 100644 --- a/src/ModelTab.h +++ b/src/ModelTab.h @@ -72,7 +72,15 @@ class ModelTab : public Component, private ChangeListener, public ChangeBroadcas // Bounds accessors for tutorial steps Rectangle getModelSelectBounds() const { - return modelSelectionWidget.getBounds().expanded(2, 2); + auto bounds = modelSelectionWidget.getBounds(); + + // The model browser lives on the Home tab, so this widget is laid out + // with an empty size here. Report nothing rather than a stray rectangle + // in the top left corner. + if (bounds.getWidth() > 0 && bounds.getHeight() > 0) + return bounds.expanded(2, 2); + + return {}; } Rectangle getControlsBounds() const @@ -117,6 +125,32 @@ class ModelTab : public Component, private ChangeListener, public ChangeBroadcas bool isModelLoaded() { return model->isLoaded(); } + // True while a model load or process request is still queued or running on + // one of this tab's thread pools. Used to defer destruction of the tab: if + // its ThreadPool is destroyed while a worker is blocked in a network call, + // JUCE force-kills the worker thread, which can corrupt the shared + // networking subsystem and hang all future requests. + bool hasPendingRequests() const + { + return loadingThreadPool.getNumJobs() > 0 || processingThreadPool.getNumJobs() > 0; + } + + // Called once this tab has left the UI but cannot be destroyed yet because a + // request is still in flight. Aborts the connection locally so that the + // request settles within moments instead of whenever the server or the + // request timeout gets around to it - which may be long after the app has + // started shutting down, at which point delivering the result crashes. Any + // result that does still arrive is dropped, since the user closed this tab. + void abandon() + { + abandoned = true; + + // Invalidate any in-flight jobs + ++currentProcessID; + + model->abortActiveRequests(); + } + void resized() override { FlexBox tabArea; @@ -426,6 +460,12 @@ class ModelTab : public Component, private ChangeListener, public ChangeBroadcas MessageManager::callAsync( [this, result] { + if (abandoned) + { + // Tab was closed while this load was in flight + return; + } + if (result.wasOk()) { modelSelectionWidget.setSuccessfulState(); @@ -539,6 +579,12 @@ class ModelTab : public Component, private ChangeListener, public ChangeBroadcas MessageManager::callAsync( [this, result, outputFilesPtr, labelsPtr] { + if (abandoned) + { + // Tab was closed while this process was in flight + return; + } + std::function onExit = [this] { // Re-enable processing immediately @@ -627,5 +673,6 @@ class ModelTab : public Component, private ChangeListener, public ChangeBroadcas ThreadPool processingThreadPool { 10 }; std::atomic currentProcessID { 0 }; + std::atomic abandoned { false }; std::function initialLoadCallback; }; diff --git a/src/ModelTabContainer.h b/src/ModelTabContainer.h index 7c9320b7..4d94ab4b 100644 --- a/src/ModelTabContainer.h +++ b/src/ModelTabContainer.h @@ -119,6 +119,26 @@ class ModelTabContainer : public TabbedComponent, ~ModelTabContainer() override { getTabbedButtonBar().setLookAndFeel(nullptr); + + // App shutdown: a tab that is still open (never closed) may have an + // in-flight request. Letting modelTabs auto-destruct would delete such + // a tab and destroy its ThreadPool while a worker is blocked in a + // network syscall, force-killing it (see ModelTab::hasPendingRequests). + // Waiting for the request instead would stall the quit for as long as + // its timeout, so abandon it - which aborts the connection, so that the + // OS networking task resolves now rather than minutes from now, once + // too much of the app has gone away to deliver the result safely - and + // release ownership without deleting, leaving the threads and memory + // for the exiting process to reclaim. Idle tabs are left in the array + // and destroyed normally. + for (int i = modelTabs.size(); --i >= 0;) + { + if (ModelTab* tab = modelTabs.getUnchecked(i); tab->hasPendingRequests()) + { + tab->abandon(); + modelTabs.remove(i, false); + } + } } ModelTab* createNewTab(const String& modelPath = {}, const String& modelName = {}) @@ -127,6 +147,10 @@ class ModelTabContainer : public TabbedComponent, auto* tab = new ModelTab(); + // Owned from creation, so that a tab is never left unowned while a + // request is in flight (see closeModelTab and the destructor) + modelTabs.add(tab); + auto tabName = modelName; if (tabName.isEmpty() && modelPath.isNotEmpty()) @@ -148,6 +172,10 @@ class ModelTabContainer : public TabbedComponent, return dynamic_cast(getCurrentContentComponent()); } + // Closes a model tab as if its close button had been clicked. Does nothing + // if the tab is no longer in the tab bar. + void closeTab(ModelTab* tab) { closeModelTab(tab); } + ModelTab* getFirstModelTab() const { for (int i = 0; i < getNumTabs(); ++i) @@ -164,10 +192,14 @@ class ModelTabContainer : public TabbedComponent, { tab->addChangeListener(this); + // The tab is already owned by modelTabs, so it is added with + // deleteComponentWhenNotNeeded = false: closing it must not force JUCE + // to destroy it synchronously in removeTab(); see closeModelTab() for + // why destruction may need to be deferred. addTab(tabName, tabBackgroundColour, tab, - true); + false); addCloseButtonToModelTab(tab); @@ -200,11 +232,34 @@ class ModelTabContainer : public TabbedComponent, : (currentIndex > i ? currentIndex - 1 : currentIndex); + // Remove the tab from the UI immediately so it looks closed to + // the user. Because the tab was added with + // deleteComponentWhenNotNeeded = false, removeTab() does not + // destroy it; we own it via modelTabs. removeTab(i); if (getNumTabs() > 0) setCurrentTabIndex(jlimit(0, getNumTabs() - 1, targetIndex)); + tabToClose->removeChangeListener(this); + + if (tabToClose->hasPendingRequests()) + { + // A network request is still in flight. Destroying the tab + // (and its ThreadPool) now would force-kill a worker thread + // blocked in a network syscall. Abandoning it aborts the + // connection so the worker returns within moments; hand + // ownership to the reaper, which deletes it once it does. + tabToClose->abandon(); + + modelTabs.removeObject(tabToClose, false); + tabReaper.add(tabToClose); + } + else + { + modelTabs.removeObject(tabToClose, true); + } + sendChangeMessage(); return; } @@ -216,7 +271,12 @@ class ModelTabContainer : public TabbedComponent, auto* homeTab = new HomeTab(); homeTab->onModelLoadRequested = [this, homeTab](String modelPath, String modelName) { + // Owned from creation as well: this tab is not in the UI yet, but it + // has a load request in flight, so quitting the app now must find it + // in modelTabs and abandon it rather than leave it running. auto* pendingTab = new ModelTab(); + modelTabs.add(pendingTab); + pendingTab->onNextModelLoadComplete( [this, homeTab, modelName](ModelTab* tab, bool wasSuccessful) { @@ -227,6 +287,9 @@ class ModelTabContainer : public TabbedComponent, } else { + // Deleted asynchronously because this runs from inside + // the tab's own load completion handler + modelTabs.removeObject(tab, false); MessageManager::callAsync([tab] { delete tab; }); } @@ -252,9 +315,66 @@ class ModelTabContainer : public TabbedComponent, } } + // Owns model tabs whose UI has been closed while a request was still in + // flight. Polls each tab until its thread pools are idle, then deletes it on + // the message thread so that no worker thread is force-killed mid-request. + class DeferredTabReaper : private Timer + { + public: + ~DeferredTabReaper() override + { + stopTimer(); + + // The reaper is only destroyed when the container is, i.e. at app + // shutdown. Any tab still pending here has an in-flight request; + // deleting it would destroy a ThreadPool whose worker is blocked in + // a network syscall, force-killing it. Their connections were + // already aborted when they were added, so release ownership + // without deleting and let the exiting process reclaim the threads + // and memory. + pendingTabs.clear(false); + } + + void add(ModelTab* tab) + { + pendingTabs.add(tab); + + if (! isTimerRunning()) + startTimer(pollIntervalMs); + } + + private: + void timerCallback() override + { + for (int i = pendingTabs.size(); --i >= 0;) + { + ModelTab* tab = pendingTabs.getUnchecked(i); + + if (! tab->hasPendingRequests()) + { + // Release ownership and delete via callAsync so that any + // completion callbacks the finished job already queued run + // before the tab is destroyed. + pendingTabs.removeObject(tab, false); + MessageManager::callAsync([tab] { delete tab; }); + } + } + + if (pendingTabs.isEmpty()) + stopTimer(); + } + + static constexpr int pollIntervalMs = 250; + + OwnedArray pendingTabs; + }; + const Colour tabBackgroundColour { getUIColourIfAvailable(LookAndFeel_V4::ColourScheme::UIColour::windowBackground) }; ModelTabsLookAndFeel tabsLookAndFeel; + + OwnedArray modelTabs; + DeferredTabReaper tabReaper; }; diff --git a/src/clients/Client.h b/src/clients/Client.h index 7f134307..05f475cf 100644 --- a/src/clients/Client.h +++ b/src/clients/Client.h @@ -176,16 +176,169 @@ inline OpResult getRequiredArrayProperty(DynamicObject::Ptr& parentDict, return OpResult::ok(); } +/* + Keeps track of the network streams that are currently in flight for a single + model, so that they can be aborted locally. + + This is not the same thing as Client::cancel(), which asks the server to stop + a job by sending it a brand new request: that leaves the original connection + open and needs the network to be reachable. Aborting here closes the + connection on this side, so a worker thread blocked on it returns promptly and + the underlying OS networking task cannot deliver its completion long after the + objects that started the request are gone. +*/ +class RequestRegistry +{ +public: + void abortActiveRequests() + { + const ScopedLock lock(streamsLock); + + aborted = true; + + for (WebInputStream* stream : streams) + { + stream->cancel(); + } + } + + bool hasBeenAborted() const + { + const ScopedLock lock(streamsLock); + + return aborted; + } + + void addStream(WebInputStream* stream) + { + const ScopedLock lock(streamsLock); + + streams.add(stream); + } + + void removeStream(WebInputStream* stream) + { + const ScopedLock lock(streamsLock); + + streams.removeFirstMatchingValue(stream); + } + +private: + CriticalSection streamsLock; + + Array streams; + + bool aborted = false; +}; + +/* + A WebInputStream that stays registered with a RequestRegistry for as long as + it exists, so the registry can abort it while a worker thread is blocked + connecting to it or reading from it. Unregistering in the destructor (under + the registry lock) guarantees the registry never holds a dangling stream. +*/ +class RegisteredWebInputStream : public WebInputStream +{ +public: + RegisteredWebInputStream(RequestRegistry& registryToUse, + const URL& url, + bool addParametersToRequestBody) + : WebInputStream(url, addParametersToRequestBody), registry(registryToUse) + { + registry.addStream(this); + } + + ~RegisteredWebInputStream() override { registry.removeStream(this); } + +private: + RequestRegistry& registry; +}; + class Client { public: Client() = default; virtual ~Client() = default; + void setRequestRegistry(RequestRegistry* registryToUse) { requestRegistry = registryToUse; } + virtual String inferHostSlashModel(String modelPath) = 0; virtual String inferEndpointPath(String modelPath) = 0; virtual String inferDocumentationPath(String modelPath) = 0; + /* + Equivalent to URL::createInputStream(), except that the stream is + registered with requestRegistry for its whole lifetime so that it can be + aborted locally (see RequestRegistry). Registering before connecting is + what makes the connect phase abortable too - that phase blocks for up to + the connection timeout, which is two minutes for process requests. + + Returns nullptr if the connection could not be established, including + when the request was aborted. Clients without a registry (e.g. the + short-lived one used for token validation) behave exactly as before. + */ + std::unique_ptr createRequestStream(const URL& endpoint, + const URL::InputStreamOptions& options) const + { + if (requestRegistry == nullptr || endpoint.isLocalFile()) + { + return endpoint.createInputStream(options); + } + + if (requestRegistry->hasBeenAborted()) + { + // Requests were aborted, so do not open another connection + return nullptr; + } + + auto stream = std::make_unique( + *requestRegistry, + endpoint, + options.getParameterHandling() == URL::ParameterHandling::inPostData); + + const String extraHeaders = options.getExtraHeaders(); + + if (extraHeaders.isNotEmpty()) + { + stream->withExtraHeaders(extraHeaders); + } + + const int connectionTimeoutMs = options.getConnectionTimeoutMs(); + + if (connectionTimeoutMs != 0) + { + stream->withConnectionTimeout(connectionTimeoutMs); + } + + const String requestCmd = options.getHttpRequestCmd(); + + if (requestCmd.isNotEmpty()) + { + stream->withCustomRequestCommand(requestCmd); + } + + stream->withNumRedirectsToFollow(options.getNumRedirectsToFollow()); + + const bool connected = stream->connect(nullptr); + + if (int* statusCode = options.getStatusCode()) + { + *statusCode = stream->getStatusCode(); + } + + if (StringPairArray* responseHeaders = options.getResponseHeaders()) + { + *responseHeaders = stream->getResponseHeaders(); + } + + if (! connected || stream->isError()) + { + return nullptr; + } + + return stream; + } + OpResult queryToken(const String& tokenToQuery, String& response, const int timeoutMs = 10000) { String tokenValidationPath = tokenValidationURL.toString(true); @@ -206,7 +359,7 @@ class Client .withConnectionTimeoutMs(timeoutMs) .withStatusCode(&statusCode); - std::unique_ptr stream(tokenValidationURL.createInputStream(options)); + std::unique_ptr stream(createRequestStream(tokenValidationURL, options)); if (stream == nullptr) { @@ -322,4 +475,8 @@ class Client } SharedResourcePointer sharedTokens; + + // Owned by the Model this client belongs to; null for clients that are not + // tied to a model, in which case requests are simply not abortable. + RequestRegistry* requestRegistry = nullptr; }; diff --git a/src/clients/GradioClient.h b/src/clients/GradioClient.h index b1374fdf..084f8355 100644 --- a/src/clients/GradioClient.h +++ b/src/clients/GradioClient.h @@ -571,7 +571,7 @@ class GradioClient : public Client .withNumRedirectsToFollow(5) .withHttpRequestCmd("POST"); - std::unique_ptr stream(endpoint.createInputStream(options)); + std::unique_ptr stream(createRequestStream(endpoint, options)); if (stream == nullptr) { @@ -622,7 +622,7 @@ class GradioClient : public Client .withStatusCode(&statusCode) .withNumRedirectsToFollow(5); - stream = endpoint.createInputStream(options); + stream = createRequestStream(endpoint, options); if (stream == nullptr) { @@ -743,6 +743,12 @@ class GradioClient : public Client OpResult result = makeGETRequestStream(endpoint, stream, errorPath, timeoutMs); + if (result.failed()) + { + // No stream was opened, e.g. because the request was aborted + return result; + } + // Remove file at target path if one already exists fileToDownload.deleteFile(); diff --git a/src/clients/providers/stability/StabilityClient.h b/src/clients/providers/stability/StabilityClient.h index bba18d37..fa2cd92f 100644 --- a/src/clients/providers/stability/StabilityClient.h +++ b/src/clients/providers/stability/StabilityClient.h @@ -396,7 +396,7 @@ class StabilityClient : public Client .withNumRedirectsToFollow(5) .withHttpRequestCmd("POST"); - std::unique_ptr stream(endpoint.createInputStream(options)); + std::unique_ptr stream(createRequestStream(endpoint, options)); if (stream == nullptr) { diff --git a/src/windows/WelcomeWindow.h b/src/windows/WelcomeWindow.h index 7450f6d5..5b9026e2 100644 --- a/src/windows/WelcomeWindow.h +++ b/src/windows/WelcomeWindow.h @@ -25,9 +25,12 @@ struct TutorialStep String description; std::function(MainComponent*)> getHighlightBounds; std::function>(MainComponent*)> getExtraHighlights = nullptr; + // Steps that highlight the media clipboard need it on screen: it is hidden + // by default, and a step with no highlight just dims the whole window. + bool requiresMediaClipboard = false; }; -class WelcomeWindow : public DocumentWindow, public ChangeListener +class WelcomeWindow : public DocumentWindow { public: WelcomeWindow(MainComponent* mainComp) @@ -53,11 +56,7 @@ class WelcomeWindow : public DocumentWindow, public ChangeListener positionOnMainComponentDisplay(); if (mainComponent) - { - if (auto* tab = mainComponent->getFirstModelTab()) - tab->addChangeListener(this); mainComponent->setTutorialActive(true); - } rebuildSteps(); updateStep(); @@ -66,47 +65,20 @@ class WelcomeWindow : public DocumentWindow, public ChangeListener ~WelcomeWindow() override { if (mainComponent) - { - if (auto* tab = mainComponent->getFirstModelTab()) - tab->removeChangeListener(this); mainComponent->setTutorialActive(false); - } } - void changeListenerCallback(ChangeBroadcaster*) override + // Called by MainComponent whenever a model tab is opened, closed, or + // finishes loading a model. + void notifyModelStateChanged() { - // Model loaded/changed - if (pendingTutorialFallbackLoad) - { - if (mainComponent != nullptr) - { - auto* tab = mainComponent->getFirstModelTab(); - auto model = tab != nullptr ? tab->getModel() : nullptr; - auto loadedPath = model ? model->getLoadedPath() : String(); - - autoLoadedByTutorialFallback = (loadedPath == TutorialConstants::fallbackModelPath); - } - pendingTutorialFallbackLoad = false; - } - else if (autoLoadedByTutorialFallback && mainComponent != nullptr) - { - auto* tab = mainComponent->getFirstModelTab(); - auto model = tab != nullptr ? tab->getModel() : nullptr; - auto loadedPath = model ? model->getLoadedPath() : String(); - if (loadedPath != TutorialConstants::fallbackModelPath) - autoLoadedByTutorialFallback = false; - } - rebuildSteps(); - if (content->currentStep > 0) + if (content->currentStep > 0 && steps.size() > 2 && isModelLoadedInCurrentTab()) { - // Auto-jump to Step 3 ("Quick Start") if user loads a new model + // Auto-jump to Step 3 ("Quick Start") once a model is loaded // (Index 2 corresponds to "Quick Start") - if (steps.size() > 2) - { - content->currentStep = 2; - } + content->currentStep = 2; } updateStep(); @@ -114,6 +86,20 @@ class WelcomeWindow : public DocumentWindow, public ChangeListener void refreshHighlightForCurrentStep() { updateStep(); } + // The tutorial describes and highlights the tab that is on screen, so every + // step reads the model from the current tab rather than the first one. + std::shared_ptr getCurrentModel() const + { + auto* tab = mainComponent != nullptr ? mainComponent->getCurrentModelTab() : nullptr; + return tab != nullptr ? tab->getModel() : nullptr; + } + + bool isModelLoadedInCurrentTab() const + { + auto model = getCurrentModel(); + return model != nullptr && model->isLoaded(); + } + void rebuildSteps() { steps.clear(); @@ -136,10 +122,8 @@ class WelcomeWindow : public DocumentWindow, public ChangeListener String modelName = "current model"; - if (mainComponent) { - auto* tab = mainComponent->getFirstModelTab(); - auto model = tab != nullptr ? tab->getModel() : nullptr; + auto model = getCurrentModel(); if (model && model->isLoaded()) { modelName = model->getMetadata().name; @@ -183,8 +167,7 @@ class WelcomeWindow : public DocumentWindow, public ChangeListener // 4. Configure Parameters (Dynamic) if (mainComponent) { - auto* tab = mainComponent->getFirstModelTab(); - auto model = tab != nullptr ? tab->getModel() : nullptr; + auto model = getCurrentModel(); if (model && model->isLoaded()) { String controlsStepTitle = "Configure Parameters (Optional)"; @@ -275,7 +258,8 @@ class WelcomeWindow : public DocumentWindow, public ChangeListener std::vector> v; v.push_back(c->getClipboardControlsBounds()); return v; - } }); + }, + true }); // 8. Interface Summary steps.push_back( @@ -291,7 +275,8 @@ class WelcomeWindow : public DocumentWindow, public ChangeListener v.push_back(c->getInputTrackBounds()); v.push_back(c->getClipboardBounds()); return v; - } }); + }, + true }); steps.push_back( { "All Set!", @@ -663,6 +648,13 @@ class WelcomeWindow : public DocumentWindow, public ChangeListener if (steps.empty()) return; + content->currentStep = jlimit(0, (int) steps.size() - 1, content->currentStep); + + // Showing the clipboard resizes the main window, which re-enters this + // method, so it happens before anything holds a reference into steps + if (mainComponent != nullptr && steps[size_t(content->currentStep)].requiresMediaClipboard) + mainComponent->ensureMediaClipboardVisible(); + const auto& step = steps[size_t(content->currentStep)]; content->titleLabel.setText(step.title, dontSendNotification); @@ -718,16 +710,22 @@ class WelcomeWindow : public DocumentWindow, public ChangeListener void nextStep() { - if (content->currentStep == 1 && mainComponent != nullptr) + if (content->currentStep == 1 && mainComponent != nullptr && ! isModelLoadedInCurrentTab()) { - auto* tab = mainComponent->getFirstModelTab(); - auto model = tab != nullptr ? tab->getModel() : nullptr; - if (! model || ! model->isLoaded()) + // A load is already running; the step advances by itself once the + // model arrives (see notifyModelStateChanged) + if (mainComponent->isTutorialModelLoadInFlight()) + return; + + if (! attemptedFallbackLoad) { - pendingTutorialFallbackLoad = true; + attemptedFallbackLoad = true; mainComponent->ensureTutorialModelLoaded(); return; } + + // The fallback load did not succeed. Carry on rather than leaving + // the user stuck on this step with a button that does nothing. } if (content->currentStep < (int) steps.size() - 1) @@ -758,14 +756,15 @@ class WelcomeWindow : public DocumentWindow, public ChangeListener ! content->dontShowAgainToggle.getToggleState() ? "1" : "0"; Settings::setValue("view.showWelcomePopup", showWelcomePopupUponNextStartup, true); - if (autoLoadedByTutorialFallback && mainComponent != nullptr) + if (mainComponent != nullptr) mainComponent->resetTutorialAutoLoadedModel(); closeButtonPressed(); } - bool pendingTutorialFallbackLoad = false; - bool autoLoadedByTutorialFallback = false; + // Whether the tutorial has already asked for the fallback model on the + // user's behalf, so that a failed load does not trap them on that step + bool attemptedFallbackLoad = false; std::vector steps;