To follow the rule of avoiding the creation of floating windows in a plug-in editor (cf. pink background), I override the following method in my custom look and feel:
However, the PopupMenu::dismissAllActiveMenus() method only looks for such floating windows and thus fails to close popup menus at various places (ComboBox::hidePopup() in my case) and can lead to crash when the editor is closed while the popup is still visible.
dismissAllActiveMenus() probably should close menus that are attached to components however, I’m not completely surprised it doesn’t and I would need to take a closer look before committing to anything.
I’m surprised closing the editor causes a crash. Chatting to the team we wondered if there could be a custom menu item? maybe it refers to data that gets deleted before it does? Without seeing more details about the crash it’s hard to say.
Could you possibly see if you can create a minimal example that demonstrates the issue? maybe try to make some minimal changes to the WidgetsDemo in the DemoRunner?
I was curious and added .withParentComponent (foo.getTopLevelComponent()) to a custom PopupMenu and when closing the app got a charming assert at ~ResizableWindow() “have you been adding your own components directly to this window…? tut tut tut Read the instructions for using a ResizableWindow!”
Does getTopLevelComponent() return a pointer to a ResizableWindow?
Using .withParentComponent (dynamic_cast<DocumentWindow*> (foo.getTopLevelComponent())->getContentComponent()) instead seems to work fine in my case. Perhaps this is relevant?
Here’s a PIP. To reproduce the issue, click the combobox to show the dropdown menu, leave it open and try to close the editor, it should crash as VST (works fine otherwise).
/*******************************************************************************
The block below describes the properties of this PIP. A PIP is a short snippet
of code that can be read by the Projucer and used to generate a JUCE project.
BEGIN_JUCE_PIP_METADATA
name: PopupMenuTest
dependencies: juce_audio_basics, juce_audio_devices, juce_audio_formats, juce_audio_plugin_client,
juce_audio_processors, juce_audio_utils, juce_core, juce_data_structures, juce_events,
juce_graphics, juce_gui_basics, juce_gui_extra
exporters: XCODE_MAC
moduleFlags: JUCE_STRICT_REFCOUNTEDPOINTER=1
type: AudioProcessor
mainClass: TestPlugin
extraPluginFormats: VST
END_JUCE_PIP_METADATA
*******************************************************************************/
#pragma once
class TestPluginEditor : public juce::AudioProcessorEditor
{
public:
TestPluginEditor (AudioProcessor& processor) : juce::AudioProcessorEditor (processor)
{
customLookAndFeel.reset (new CustomLookAndFeel());
setLookAndFeel (customLookAndFeel.get());
comboBox.addItem ("Item 1", 1);
comboBox.addItem ("Item 2", 2);
comboBox.setText ("click me then close the editor");
addAndMakeVisible (comboBox);
setSize (200, 100);
}
virtual ~TestPluginEditor()
{
comboBox.hidePopup();
setLookAndFeel (nullptr);
}
void resized() override
{
comboBox.setBounds (0, 0, 200, 20);
}
void paint (juce::Graphics& g) override
{
g.fillAll (juce::Colours::black);
}
class CustomLookAndFeel : public juce::LookAndFeel_V4
{
public:
CustomLookAndFeel() {}
juce::PopupMenu::Options getOptionsForComboBoxPopupMenu (juce::ComboBox& box, juce::Label& label) override
{
return juce::PopupMenu::Options().withTargetComponent (&box)
.withItemThatMustBeVisible (box.getSelectedId())
.withInitiallySelectedItem (box.getSelectedId())
.withMinimumWidth (box.getWidth())
.withMaximumNumColumns (1)
.withStandardItemHeight (jmax (label.getHeight(), 20))
.withParentComponent (box.getTopLevelComponent());
}
};
juce::ComboBox comboBox;
std::unique_ptr<CustomLookAndFeel> customLookAndFeel = nullptr;
};
//==============================================================================
class TestPlugin : public juce::AudioProcessor
{
public:
//==============================================================================
TestPlugin()
: AudioProcessor (BusesProperties().withInput ("Input", juce::AudioChannelSet::stereo())
.withOutput ("Output", juce::AudioChannelSet::stereo()))
{
}
~TestPlugin() override
{
}
//==============================================================================
void prepareToPlay (double, int) override
{
// Use this method as the place to do any pre-playback
// initialisation that you need..
}
void releaseResources() override
{
// When playback stops, you can use this as an opportunity to free up any
// spare memory, etc.
}
void processBlock (juce::AudioBuffer<float>& buffer, juce::MidiBuffer&) override
{
juce::ScopedNoDenormals noDenormals;
auto totalNumInputChannels = getTotalNumInputChannels();
auto totalNumOutputChannels = getTotalNumOutputChannels();
// In case we have more outputs than inputs, this code clears any output
// channels that didn't contain input data, (because these aren't
// guaranteed to be empty - they may contain garbage).
// This is here to avoid people getting screaming feedback
// when they first compile a plugin, but obviously you don't need to keep
// this code if your algorithm always overwrites all the output channels.
for (auto i = totalNumInputChannels; i < totalNumOutputChannels; ++i)
buffer.clear (i, 0, buffer.getNumSamples());
// This is the place where you'd normally do the guts of your plugin's
// audio processing...
// Make sure to reset the state if your inner loop is processing
// the samples and the outer loop is handling the channels.
// Alternatively, you can process the samples with the channels
// interleaved by keeping the same state.
for (int channel = 0; channel < totalNumInputChannels; ++channel)
{
auto* channelData = buffer.getWritePointer (channel);
// ..do something to the data...
}
}
//==============================================================================
juce::AudioProcessorEditor* createEditor() override { return new TestPluginEditor (*this); }
bool hasEditor() const override { return true; }
//==============================================================================
const juce::String getName() const override { return "EscPopupMenuTest"; }
bool acceptsMidi() const override { return false; }
bool producesMidi() const override { return false; }
double getTailLengthSeconds() const override { return 0; }
//==============================================================================
int getNumPrograms() override { return 1; }
int getCurrentProgram() override { return 0; }
void setCurrentProgram (int) override {}
const juce::String getProgramName (int) override { return {}; }
void changeProgramName (int, const juce::String&) override {}
//==============================================================================
void getStateInformation (juce::MemoryBlock& destData) override
{
// You should use this method to store your parameters in the memory block.
// You could do that either as raw data, or use the XML or ValueTree classes
// as intermediaries to make it easy to save and load complex data.
}
void setStateInformation (const void* data, int sizeInBytes) override
{
// You should use this method to restore your parameters from this memory block,
// whose contents will have been created by the getStateInformation() call.
}
//==============================================================================
bool isBusesLayoutSupported (const BusesLayout& layouts) const override
{
// This is the place where you check if the layout is supported.
// In this template code we only support mono or stereo.
if (layouts.getMainOutputChannelSet() != juce::AudioChannelSet::mono()
&& layouts.getMainOutputChannelSet() != juce::AudioChannelSet::stereo())
return false;
// This checks if the input layout matches the output layout
if (layouts.getMainOutputChannelSet() != layouts.getMainInputChannelSet())
return false;
return true;
}
private:
//==============================================================================
JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (TestPlugin)
};
@mathieudemange thanks for the PIP. At least when I run an AU from AudioPluginHost getTopLevelComponent() returns a component at the top that appears to own all it’s children and so expects to delete them all, this in itself may be worth looking into, however to fix the problem locally I used box.findParentComponentOfClass<TestPluginEditor>(), does something like this work for you? I think this is nice because it’s more explicit which component will act as the parent component of the popup menu. You could alternatively pass in a popupMenuParentComponet or similar to the loo&feel so it can always use that component for this method.
Thanks for your help. Following your suggestion, I used box.findParentComponentOfClass<AudioProcessorEditor>() which is a little more generic and fixes the problem as well.
