Implementing “hot reload”

[adamski]

Using the Gin FileSystemWatcher, I’ve set up to replace the view ValueTree when my layout XML file changes.

This didn’t work using a simple = assignment, I think due to the listeners breaking as it’s essentially a new ValueTree. So I tried ValueTree::copyChildrenAndProperties. This hit an assertion in the JIVE listener callbacks.

Is there a recommended way to set this up?

[adamski]

Relevant code: (view is a ValueTree member of AudioProcessor)

juce::AudioProcessorEditor* MyAudioProcessor::createEditor()
{
    view = jive::parseXML (layoutXml);

    if (auto editor = viewInterpreter.interpret (view, this))
    {
        jassert (editor != nullptr);

        if (dynamic_cast<juce::AudioProcessorEditor*> (editor.get()))
        {
            viewInterpreter.listenTo (*editor);
            return dynamic_cast<juce::AudioProcessorEditor*> (editor.release());
        }
    }

    // Fallback in case the editor wasn't constructed for some reason
    return new juce::GenericAudioProcessorEditor { *this };
}

void MyAudioProcessor::fileChanged (const juce::File& file, gin::FileSystemWatcher::FileSystemEvent fsEvent)
{
    layoutXml = layoutFolder.getChildFile ("Layout.xml").loadFileAsString();
    view.copyPropertiesAndChildrenFrom (jive::parseXML (layoutXml), nullptr);
}

Stack trace from assertion hit when reloading:

juce::VariantConverter::fromVar(const juce::var &) jive_Display.cpp:13
jive::fromVar<…>(const juce::var &) jive_VariantConvertion.h:45
jive::Property::getFrom(const juce::ValueTree &) const jive_Property.h:385
<lambda>::operator()(const juce::ValueTree &) const jive_Property.h:440
jive::Property::getFrom(const std::variant<…> &) const jive_Property.h:438
jive::Property::get() const jive_Property.h:89
jive::decorateWithHereditaryBehaviour(std::unique_ptr<…>) jive_Interpreter.cpp:136
jive::decorate(std::unique_ptr<…>, const std::vector<…> &, juce::AudioProcessor *) jive_Interpreter.cpp:202
jive::Interpreter::interpret(const juce::ValueTree &, jive::GuiItem *, juce::AudioProcessor *) const jive_Interpreter.cpp:236
jive::Interpreter::insertChild(jive::GuiItem &, int, const juce::ValueTree &) const jive_Interpreter.cpp:296
jive::Interpreter::valueTreeChildAdded(juce::ValueTree &, juce::ValueTree &) jive_Interpreter.cpp:105
<lambda>::operator()(juce::ValueTree::Listener &) const juce_ValueTree.cpp:113
juce::ListenerList::callCheckedExcluding<…>(juce::ValueTree::Listener *, const juce::ListenerList<…>::DummyBailOutChecker &, <lambda> &) juce_ListenerList.h:268
juce::ListenerList::callExcluding<…>(juce::ValueTree::Listener *, <lambda> &) juce_ListenerList.h:203
juce::ValueTree::SharedObject::callListeners<…>(juce::ValueTree::Listener *, <lambda>) const juce_ValueTree.cpp:93
juce::ValueTree::SharedObject::callListenersForAllParents<…>(juce::ValueTree::Listener *, <lambda>) const juce_ValueTree.cpp:101
juce::ValueTree::SharedObject::sendChildAddedMessage(juce::ValueTree) juce_ValueTree.cpp:113
juce::ValueTree::SharedObject::addChild(juce::ValueTree::SharedObject *, int, juce::UndoManager *) juce_ValueTree.cpp:275
juce::ValueTree::copyPropertiesAndChildrenFrom(const juce::ValueTree &, juce::UndoManager *) juce_ValueTree.cpp:700
TonebankAudioProcessor::fileChanged(const juce::File &, gin::FileSystemWatcher::FileSystemEvent) PluginProcessor.cpp:243
<lambda>::operator()(gin::FileSystemWatcher::Listener &) const juce_ListenerList.h:327
void juce::ListenerList<gin::FileSystemWatcher::Listener, juce::Array<gin::FileSystemWatcher::Listener*, juce::DummyCriticalSection, 0>>::callCheckedExcluding<void juce::ListenerList<gin::FileSystemWatcher::Listener, juce::Array<gin::FileSystemWatcher::Listener*, juce::DummyCriticalSection, 0>>::callCheckedExcluding<juce::ListenerList<gin::FileSystemWatcher::Listener, juce::Array<gin::FileSystemWatcher::Listener*, juce::DummyCriticalSection, 0>>::DummyBailOutChecker, juce::File const&, gin::FileSystemWatcher::FileSystemEvent, juce::File const&, gin::FileSystemWatcher::FileSystemEvent&>(gin::FileSystemWatcher::Listener*, juce::ListenerList<gin::FileSystemWatcher::Listener, juce::Array<gin::FileSystemWatcher::Listener*, juce::DummyCriticalSection, 0>>::DummyBailOutChecker const&, void (gin::FileSystemWatcher::Listener::*)(juce::File const&, gin::FileSystemWatcher::FileSystemEvent), juce::File const&, gin::FileSystemWatcher::FileSystemEvent&)::'lambda'(gin::FileSystemWatcher::Listener&), juce::ListenerList<gin::FileSystemWatcher::Listener, juce::Array<gin::FileSystemWatcher::Listener*, juce::DummyCriticalSection, 0>>::DummyBailOutChecker>(gin::FileSystemWatcher::Listener*, juce::ListenerList<gin::FileSystemWatcher::Listener, juce::Array<gin::FileSystemWatcher::Listener*, juce::DummyCriticalSection, 0>>::DummyBailOutChecker const&, juce::ListenerList<gin::FileSystemWatcher::Listener, juce::Array<gin::FileSystemWatcher::Listener*, juce::DummyCriticalSection, 0>>::DummyBailOutChecker&&) juce_ListenerList.h:268
juce::ListenerList::callCheckedExcluding<…>(gin::FileSystemWatcher::Listener *, const juce::ListenerList<…>::DummyBailOutChecker &, void (gin::FileSystemWatcher::Listener::*)(const juce::File &, gin::FileSystemWatcher::FileSystemEvent), const juce::File &, gin::FileSystemWatcher::FileSystemEvent &) juce_ListenerList.h:325
juce::ListenerList::call<…>(void (gin::FileSystemWatcher::Listener::*)(const juce::File &, gin::FileSystemWatcher::FileSystemEvent), const juce::File &, gin::FileSystemWatcher::FileSystemEvent &) juce_ListenerList.h:277
gin::FileSystemWatcher::fileChanged(const juce::File &, gin::FileSystemWatcher::FileSystemEvent) gin_filesystemwatcher.cpp:406
gin::FileSystemWatcher::Impl::callback(__FSEventStream const*, void*, unsigned long, void*, unsigned int const*, unsigned long long const*)::'lambda0'()::operator()() const gin_filesystemwatcher.cpp:87
AsyncCallInvoker::messageCallback() juce_MessageManager.cpp:212
juce::MessageQueue::deliverNextMessage() juce_MessageQueue_mac.h:93
juce::MessageQueue::runLoopCallback() juce_MessageQueue_mac.h:104
juce::MessageQueue::runLoopSourceCallback(void *) juce_MessageQueue_mac.h:112
juce::MessageManager::runDispatchLoop() juce_MessageManager_mac.mm:347
juce::JUCEApplicationBase::main() juce_ApplicationBase.cpp:277
juce::JUCEApplicationBase::main(int, const char **) juce_ApplicationBase.cpp:255
main juce_audio_plugin_client_Standalone.cpp:205

[ImJimmi]

It looks like the interpreter is observing the changes, but possibly something is still trying to access the old tree after it's been replaced! I'm surprised that's not covered by the tests - I'll see if I can repro it

[adamski]

What's a little confusing for me is why there is only Interpreter::valueTreeChildAdded and not the other ValueTree callbacks implemented.

The assertion comes from Interpreter::valueTreeChildAdded.

Might it be simpler to provide a function such as replaceState which recreates the entire tree?

[ImJimmi]

Ahh okay, I've now remembered why the interpreter doesn't listen for valueTreeRedirected() - because the caller takes a unique_ptr to the jive::GuiItem produced by the interpreter, the interpreter can't then alter that item when the ValueTree changes.

To respond to the value tree being redirected, we'd have to change the architecture so that the interpreter itself held onto the item it produces and the caller only gets a WeakReference or something.

For app projects this is fine, as you can just call interpret() again with the updated tree and reassign the std::unique_ptr<jive::GuiItem>... however for plugin projects that's not possible since ownership is given away in createEditor()

I think we'll need a slightly different approach where the interpreter itself takes ownership of the GuiItem it creates so that it can replace it when the value tree is redirected. We'll also then need to change how the jive::Editor component works so it doesn't give ownership of the top-level item to the plugin host... I'll have to have a think about how best to do that.

---

In the meantime, if you want to get hot reloading working, you could probably have a single <Component/> element under your <Editor/> element that you replace, instead of replacing the whole editor itself? The interpreter should then handle that as expected.

<Editor width="300" height="200">
  <Component id="replace-me">
    <Button/>
    <Slider/>
    // etc.
  </Component>
</Editor>

[ImJimmi]

I think we'll need a slightly different approach where the interpreter itself takes ownership of the GuiItem it creates so that it can replace it when the value tree is redirected. We'll also then need to change how the jive::Editor component works so it doesn't give ownership of the top-level item to the plugin host... I'll have to have a think about how best to do that.

Actually it could just be that a top-level GuiItem is a special case that acts as a wrapper around the "real" item which can be the one swapped-out by the interpreter... That would hopefully also mean no breaking changes!

[adamski]

OK sounds good. The suggestion to replace only the first inner child resulted in the same assertion being hit.

i.e.

void TonebankAudioProcessor::fileChanged (const juce::File& file, gin::FileSystemWatcher::FileSystemEvent fsEvent)
{
    layoutXml = layoutFolder.getChildFile ("Layout.xml").loadFileAsString();
    const auto newLayout = jive::parseXML (layoutXml);
    view.getChildWithProperty ("id", "main").copyPropertiesAndChildrenFrom (newLayout.getChildWithProperty ("id", "main"), nullptr);

With Layout.xml as:

<Editor width="640" height="400" align-items="centre">
    <Component id="main">
        <Component id="header" align-items="centre">
            <Text id="title">Welcome to JIVE!</Text>
            <Text>Hello</Text>
            <Text id="subtitle">The ultimate JUCE extension for building GUIs.</Text>
        </Component>

        <Component id="nav" display="grid" template-columns="1fr 1fr 1fr" template-rows="1fr">
            <Button>
                <Text>Home</Text>
            </Button>

            <Button>
                <Text>About</Text>
            </Button>

            <Button>
                <Text>Contact</Text>
            </Button>
        </Component>
    </Component>
</Editor>

[ImJimmi]

juce::VariantConverter::fromVar(const juce::var &) jive_Display.cpp:13
jive::fromVar<…>(const juce::var &) jive_VariantConvertion.h:45
jive::Property::getFrom(const juce::ValueTree &) const jive_Property.h:385
<lambda>::operator()(const juce::ValueTree &) const jive_Property.h:440
jive::Property::getFrom(const std::variant<…> &) const jive_Property.h:438
jive::Property::get() const jive_Property.h:89
jive::decorateWithHereditaryBehaviour(std::unique_ptr<…>) jive_Interpreter.cpp:136

Sorry I hadn't looked properly at this stack trace - the assertion is here in jive_Display.cpp:

https://github.com/ImJimmi/JIVE/blob/d2181fea571a3f9e51d53c4ede1b53a8a7e8c8f7/jive_layouts/utilities/jive_Display.cpp#L13

Can you let me know what value v has when this assertion is hit? Is it just that it's empty?

It should default to display="flex" if you don't explicitly specify.