Deleting a list of child components

[tomharvey]

Hello,

I’m currently trying to delete a group of components set at specific positions in a parent component, then re draw a group of new components of the same type at different positions.

My approach is to first remove all the child components by iterating through the list and calling removeChildComponent() on each component.

Then i call clear on the list and while debugging i see that the list size is now 0.
m_gui_notes.clear()

I expected this to remove all existing components from the view but it doesn’t, so ive also tried the calls:

removeAllChildren()
deleteAllChildren()

On the first time entering the function initialiseNotes() it draws the notes in the correct position (GuiNotes) which for now are simply just juce::components with a set colour.

Then on the second time, there is now something to delete in m_gui_notes and it does the process i described above.

Then i would expect all the original GuiNote components to be removed from the UI. But instead they’re not and it draws the new lot of components on top of the previous components.

std::list<GuiNote*> m_gui_notes;

void
PianoRoll::initialiseNotes()
{
    for (auto& gui_note: m_gui_notes)
    {
        removeChildComponent(gui_note);
        delete gui_note;
    }
    
    removeAllChildren();
    deleteAllChildren();
    
    m_gui_notes.clear();
    std::list<GuiNote*>::iterator gui_note_iterator = m_gui_notes.begin();
    
    for (std::size_t index = 0; index < m_current_sequence.size(); index++)
    {
        m_gui_notes.push_back(new GuiNote);
        gui_note_iterator++;
        int x_pos = static_cast<int>(m_current_sequence[index].note_on)/4;
        int width = (static_cast<int>(m_current_sequence[index].note_off)/4) - x_pos;
        int y_pos = (grid_line_height*number_of_midi_note_values) -
                    (grid_line_height * m_current_sequence[index].note_value);
        (*gui_note_iterator)->setBounds(x_pos + keyboard_width, y_pos ,width, grid_line_height);
        addAndMakeVisible((*gui_note_iterator));
    }
}

I feel like i’m missing something obvious or don’t quite understand how the lifecycle of components work in JUCE.

Any help would be greatly appreciated!

[Fandusss]

Have you tried setVisible(false) on the components as you are removing them? Without look through the juce code, it could be a thing that removing a child doesn’t change its visibility for paint calls.

[tomharvey]

Thanks for the suggestion, i just tried it and i’m still experiencing the same thing. I’m not sure if its to do with how i’m creating components.

I’ve just tried a little test to initialise a member variable of the same type in the constructor. It creates a large rectangle in the UI.

m_test_gui.setBounds(0, 50, 400, 700);
addAndMakeVisible(test_gui);

Then when initialiseNotes() (the previously mentioned function) is called. Then i call:

removeChildComponent(&test_gui);

Which removes the component from the ui.

The difference is that im creating the components dynamically. Which is what i need to do as the size of the components isn’t known and will change throughout the life cycle of the program.

[tomharvey]

Ive just found this post Creating and removing components on the fly

(Maybe more appropriately named as a post) and in the answer a user suggests to use an OwnedArray.

So i have tried this and i am still having no luck:

void
PianoRoll::initialiseNotes()
{
    for (auto& gui_note: m_gui_notes)
    {
        removeChildComponent(gui_note);
        gui_note->setVisible(false);
        
    }
    m_gui_notes.clearQuick(true);
    
    for (std::size_t index = 0; index < m_current_sequence.size(); index++)
    {
        m_gui_notes.add(std::make_unique<GuiNote>());
        int x_pos = static_cast<int>(m_current_sequence[index].note_on)/4;
        int width = (static_cast<int>(m_current_sequence[index].note_off)/4) - x_pos;
        int y_pos = (grid_line_height*number_of_midi_note_values) -
                    (grid_line_height * m_current_sequence[index].note_value);
        m_gui_notes[static_cast<int>(index)]->setBounds(x_pos + keyboard_width, y_pos ,width, grid_line_height);
        addAndMakeVisible(m_gui_notes[static_cast<int>(index)]);
    }
}

[tomharvey]

Some additional information:

I have tried this from MainComponent.cpp and it works as expected.

Steps were:

Create an owned array in the header;

juce::OwnedArray<GuiNote> m_gui_notes;

Initialise it with one instance:

m_gui_notes.add(std::make_unique<GuiNote>());
m_gui_notes[0]->setBounds(0, 50 ,300, 900);
addAndMakeVisible(m_gui_notes[0]);

Then remove it in a loop when a button is actioned on (loop not really needed)

        for (auto& gui_note: m_gui_notes)
        {
            gui_note->setVisible(false);
            removeChildComponent(gui_note);
        }
        m_gui_notes.clearQuick(true);

In the instance that i am trying to do this it is a few components down in the program and is being looked at by a ViewPort. I’ve tried multiple things like turning the viewport into a pointer and deleting and re assigning a new PianoRoll to it. But still the components remain drawn.

I’m now wondering if this has anything to do with the fact its in a viewed component. I can’t see any other difference, but there’s clearly something going on here!

[chrhaase]

Yeah, you definitely shouldn’t need to removeChildComponent nor setVisible(false) as you are deleting the object with clearQuick (true).

Might be something with the Viewport. Could you show how you’ve set that up?
I have some working code doing something like what you’re trying to. With an OwnedArray holding the child components that are added to the content component of the Viewport. Roughly, I do it this way:
In the MainComponent I have:

std::unique_ptr<Component> viewportContent;
std::unique_ptr<Viewport> viewport;
OwnedArray<TrackComponent> trackComponents;

And in the MainComponent constructor I have this:

viewportContent = std::make_unique<SomeCustomContentComponent>();
editViewport = std::make_unique<Viewport>();
editViewport->setViewedComponent (viewportContent.get(), false); // viewportContent is a unique_ptr so shouldn't be deleted by the viewport!
addAndMakeVisible (*editViewport);

Is this similar to what you have?
When there’s a change I call trackComponents.clear() and re-add and setBounds for all those - so that part should be similar. Just make sure that you add those children to the contentComponent of course.

[tomharvey]

Thanks for the reply, i wasn’t using the ViewPort or Component (in my case PianoRoll) as pointers, so i’ve updated my solution to do that.

My setup is a little bit different to yours I’ll do my best to explain…

The ‘Parent’ class in this case is called MidiSequenceView

It has two private member functions worth noting:

std::unique_ptr<PianoRoll> m_piano_roll;
std::unique_ptr<juce::Viewport> m_piano_view_port;

In the constructor i do the following:

m_piano_roll_width = PianoRoll::getPianoRollWidth(default_number_of_bars);
m_piano_roll = std::make_unique<PianoRoll>(this);
m_piano_view_port = std::make_unique<juce::Viewport>();
m_piano_view_port->setViewedComponent (m_piano_roll.get(), false); 
addAndMakeVisible (*m_piano_view_port);

It’s resized() function is called:

m_piano_roll->setBounds(0, tool_bar_height, m_piano_roll_width, arrange_window_height);
m_piano_view_port->setBounds(0, tool_bar_height, getWidth(), getHeight() - m_piano_roll->getY());

When this MidiSequenceView is opened a scrollable PianoRoll ViewPort is now visible.
Initially it has no ‘GuiNotes’ in it, those are yet to be added.

A file name is loaded in, parsed by actioning on an external toolbar.
This results in the PianoRoll:: initialiseNotes() function (described in an earlier comment) being called where an Owned array of GuiNote’s is populated.

Inside the PianoRoll all the GuiNotes are drawn at the correct position.

So instead of how you’re describing, the OwnedArray is defined in the Component rather than at the same level as the viewport.

The idea is when a new file is loaded in, it clears all the GuiNotes and draws them again. Instead it just draws them on top of each other.

To me design wise it made more sense to have the piano roll just draw the notes and the MidiSequenceView object to load the notes and perform any modifications to them.

I am open to changing up the design of this as clearly something isn’t quite right with my approach.

Hope that made sense!

[chrhaase]

@tomharvey

So instead of how you’re describing, the OwnedArray is defined in the Component rather than at the same level as the viewport.

Yeah, that makes good sense.

Hmm, sorry, I can’t really figure out what’s going wrong. The GuiNotes still being on screen even after being deleted is strange. Could it be something with how you’re painting? If you’re doing some optimisation like using Component::repaint (Rectangle< int > area) to only repaint the area of the new note. I haven’t done something like this myself, but I remember seeing examples of mistakes of this on the forum.

[tomharvey]

It turns out i forgot to reset the variable that was storing the parsed midi file. As it was a member variable passed by reference, when the parsed notes were being added it was just calling push back with no clear before hand.

Reading the ‘I can’t really figure out what’s going wrong’ really made me think differently about it, if anything it helps to just think this stuff out loud!

Sorry to waste your time there with that one, at least now there’s an example up there of how to used an owned array with components…

[chrhaase]

Haha, glad you found the bug!