Vectors of ofxPanel

beginners
#1

I have used vectors of ofxPanel objects in the past and am reviving and old project. I cannot get it to work any more so I am guessing something has changed with OF, I am up to date with the current master branch of OF from git.

Here is some test code:

in ofApp.h
std::vector<ofxPanel> samplePanels;

and in ofApp.cpp

//--------------------------------------------------------------
void ofApp::setup(){
    for (int i = 0; i < 6; i++) {
        ofxPanel tempPanel;
        samplePanels.push_back(tempPanel);
        samplePanels[i].setup("Panel " + ofToString(i + 1), "settings" + ofToString(i) + ".xml");
    }
}

This does not compile and results in:

Call to implicitly-deleted copy constructor of 'std::__1::unique_ptr<ofxBaseGui, std::__1::default_delete<ofxBaseGui> >'

How can I properly use a vector of ofxPanels?

#2

Ok, I managed to work this out using this code:
in ofApp.h
std::vector<ofxPanel*> samplePanels;

and in ofApp.cpp

void ofApp::setup(){
    for (int i = 0; i < 6; i++) {
        ofxPanel * tempPanel = new ofxPanel();
        samplePanels.push_back(tempPanel);
        samplePanels[i].setup("Panel " + ofToString(i + 1), "settings" + ofToString(i) + ".xml");
    }
}
#3

Hey, glad you got it to compile! Just a few thoughts on this as I had something similar recently.

I’m not sure if ofxPanel has pointers in it, but sometimes pointers and vectors have special considerations. From your compile error, I’d say that ofxPanel might have a unique_ptr in it, so it can’t be constructed and then copied into a vector with push_back. But its my understanding that this is exactly what happens when you push_back a vector; a copy of the object is added to the vector, and not the object itself.

So, there are a couple of ways to get around the copy that is made with push_back. You could make an ofxPanel object (which we’ll assume for now has a unique_ptr in it), and then std::move it into the vector, like this:

std::vector<ofxPanel> samplePanels;
ofxPanel tempPanel;
// maybe .setup() and some other stuff in tempPanel, then
panels.push_back(std::move(panel));

I’ve also seen code where the object is “constructed in place” in the vector, using emplace_back, which calls the constructor to construct the object inside the vector, rather than copying a constructed object into the vector. I’ve tried emplace_back a few times and it seemed to work well, but its been a while.

Then finally, using vectors of pointers can help in lots of situations. Its particularly helpful with oF classes like ofNode, where the class variable ofNode::parent is a raw pointer. This raw pointer can get “lost” when an ofNode is copied into a vector with push_back, or when the vector relocates itself in memory as it grows. But using a smart pointer for the object, instead of the object itself, gets around this issue. You can use either shared or unique pointers. So, in your case with ofxPanel:

std::vector<std::unique_ptr<ofxPanel>> samplePanels;
// make a panel
ofxPanel tempPanel;
// do some stuff to it if you want
// then, make a smart pointer to it on the heap, instantiated with tempPanel
std::unique_ptr<ofxPanel> ptrPanel = std::make_unique<ofxPanel>(tempPanel);
// and you can use auto if you want
auto ptrPanel = std::make_unique<ofxPanel>(tempPanel);
// and I'm not sure but you might have to std::move(tempPanel) into the new pointer
auto ptrPanel  = std::make_unique<ofxPanel>(std::move(tempPanel));
// then push_back samplePanels; use std::move for unique_ptr since they can't be copied
panels.push_back(std::move(ptrPanel));

If you did the above with a shared_ptr, then you could just push_back the vector with the shared pointer because copies are allowed.

The awesome thing about smart pointers is that they take care of their own destruction when they go out of scope, or when all of the copies have been deleted. So you don’t have to remember to delete them like raw pointers. A unique pointer will only allow 1 instance of itself ever, and you have to std::move() it around if it changes ownership. Shared pointers can have many copies of the pointer available, and it will keep track of how many objects are using the pointer at any given time.

2 Likes
#4

hey fred,

i always get a bad feeling in the stomach when using raw pointers, because i’m afraid to introduce memory leaks. anytime you use “new” you should do “delete” as well to free the allocated memory.

if you run this just once in setup, you should be fine of course. but it could be good practice to use a smart pointer here, just to make sure it’s cleaned up.
sth like

ofApp.h

vector<shared_ptr<ofxPanel>> samplePanels;

ofApp.cpp

void ofApp::setup(){
    for (int i = 0; i < 6; i++) {
        auto tempPanel = make_shared<ofxPanel>();
        samplePanels.push_back(tempPanel);
        samplePanels[i].setup("Panel " + ofToString(i + 1), "settings" + ofToString(i) + ".xml");
    }
}

arturo wrote an article on it here:
https://openframeworks.cc/ofBook/chapters/memory.html

ha, TimChi, i see you were faster :slight_smile: i tend to almost always use shared_ptr because it’s less code in many cases.

2 Likes
#5

@atanyimebutnow & @TimChi
Thanks for both of your suggestions. Yes ofxPanel seems to have pointers in it. I was kind of OK with the memory issue as the panels are made once in setup and used for the duration of the program. However, sometimes it is good to be smart. Both solutions work perfectly, I went with shared pointers as it looks a little neater.

I do need to read up on the differences between the two solutions proposed.

Thanks both of you.

#6

I see the same error message in one specific project macOS 10.15 + OF 0.11.2 but it has no ofxPanel vectors.
I have other classes that declare a ofxPanel but those classes are not part of any vector<>.

I do use Singleton and refer to classes in there that do have ofxPanels.

Do you think that could be the cause for the same error you see?

class Singleton {
public: 
    static Singleton* Instance(); // pointer to itself
	
    heartShaper shaper_object; //has ofxPanel
    audioDesk audioDesk_object;//has ofxPanel
    
protected: 
    Singleton(); // protected constuctor
private:
    static Singleton* _instance;
};
#7

I’m going to (tentatively) vote yes on this. I’m thinking the compiler is saying that there isn’t a copy constructor for the object (a class that contains something derived from ofxBaseGui in this case) because a unique pointer is involved in ofxBaseGui, and there can only be 1 copy of it. I’m thinking you’ll have to figure out where the compiler is trying to make copies, and see if you can use std::move() instead (or otherwise transfer ownership, or construct it in place, etc).

I’m also kinda wondering if you could write your own copy constructor for the class. You may have to rewrite other constructors though, according to the “rule of 5”: The rule of three/five/zero - cppreference.com.

Or (thinking out loud), maybe another unique pointer would work? Maybe try using a unique pointer for each Singleton instance, and then std::move() those around as needed?

I wish I could be more helpful.

#8

@TimChi thanks for your quick reply.
I have never used move() before, so I will have to dive deeper in to c++ to try your suggestions.
It’s strange that this works fine with OF 0.11.0 but not OF 0.11.2.

Thanks again.

#9

Hum, yeah that is weird about 0.11.0 vs. 0.11.2. I had a look at the change log here:
https://github.com/openframeworks/openFrameworks/blob/0.11.2/CHANGELOG.mdm , and came across this in the ofxGui section of ADDONS:

  • ofxGui: Fix possible memory leak, because keyword “new” was being used but not deleted. Now using only unique_ptr (#6570) commit

So there’s another unique pointer in ofxGui somewhere that wasn’t there before! Or at the very least a C-style pointer was replaced with a smart pointer.

I still like the idea of using a unique pointer for each instance of a class of (or uses) Singleton. It may not work, but it intuitively it seems like its worth a try. I’m thinking you could make an object, maybe set some stuff up in it if you want, and then use std::make_unique(std::move(object)) to create the unique pointer and make it point to the object you created.

#10

Thanks again for your advice.

Seems like Singleton is not the cause of my problems.
I made this simple example and it compiles fine.guiExample_singelton.zip (102.8 KB)

#11

Found it.
I was using my own gui tooltip recipe which caused all this headache with OF 0.11.2

1 Like
#12

Hey awesome! I’m curious about the details if you have time to post!

So for example is a copy being made here (even though its an assignment)?

auto control = _gui.getControl(i);

And would the fix be something like this (use _gui directly):

string paramString = ofToString(_gui.getControl(i)->getParameter());

And maybe there are other copies are also made here (?):

string paramString = ofToString(control->getParameter());
// and
auto g_control = aGroup.getControl(n);

So glad you found it! Other who have used of the ofxGui classes may have similar issues going forward.

#13

Hi, I am the one that introduced this “problem”.

This was done because, as the commit says, to avoid having to deal with raw pointers, and its deletion, which was actually not happening, creating a potential memory leak.

The fact that a class has an unique_ptr in it will make it inherently non-copyable.
As already mentioned, the most common way to solve the issue about putting these kind of objects in a vector is to use emplace and move but I dont like this method cause you can not use std::vector::resize for example.

I prefer to use std::vector<unique_ptr<SomeClass> > or std::vector<shared_ptr<SomeClass> > depending on the use I will give to these objects, although I tend to default to unique.

There are some other classes in of that can give you similar problems, like any class that has an ofEventListener or ofEventListeners instance in it, which is solved in the same way. (No, there is no typo, ofEventListener and ofEventListeners are different classes, where the latter is a collection of the former.

The following is how to properly use a collection of ofxPanel

#pragma once

#include "ofMain.h"
#include "ofxGui.h"

class ofApp : public ofBaseApp{

	public:
		void setup();
		
		void draw();
	vector<unique_ptr<ofxPanel>> uniquePanels;
	
	ofParameter<float> f0 = {"f0", 0, 0, 1};
	ofParameter<float> f1 = {"f1", 0.3, 0, 1};
	ofParameter<float> f2 = {"f2", 0.6, 0, 1};
	ofParameter<float> f3 = {"f3", 1, 0, 1};
	
};


#include "ofApp.h"

//--------------------------------------------------------------
void ofApp::setup(){

	ofRectangle prevShape(20,20,0,0); // just to keep track of the shape of the previous gui, so we can layout all these nicely
	for(int i = 0; i < 4; i++){
		
		uniquePanels.push_back (make_unique<ofxPanel>());
		
		uniquePanels[i]->setup("panel" + ofToString(i), "", prevShape.x, prevShape.getMaxY());
		uniquePanels[i]->add(f0);
		uniquePanels[i]->add(f1);
		uniquePanels[i]->add(f2);
		uniquePanels[i]->add(f3);
		
		prevShape = uniquePanels[i]->getShape();
		
	}
}
//--------------------------------------------------------------
void ofApp::draw(){
	for(int i = 0; i < uniquePanels.size(); i++){
		uniquePanels[i]->draw();
	}
}
3 Likes
#14

There are not many details.
I simply did not use my own class

but rather used ofxGuiTooltip.zip

#15

After a quick study, this line from stephanschulz’s function ofApp::drawGuiInfo(…) calls the copy constructor:

// in the if block:
ofxGuiGroup aGroup = _gui.getGroup(control->getName());

The other local variables (created with auto) compiled OK.

#16

Hey Roy thanks for posting about this! I think it will really help those with compiling issues for unique pointers.

I tested 11 or so different ways of getting an ofxPanel or a unique pointer of one into a vector, in both 0.11.0 and 0.11.2. Most all of them no longer compiled in 0.11.2, where they did in 0.11.0.

The only ones that worked for me was the one you suggested, plus a couple of similar:

// in ofApp.h
std::vector<std::unique_ptr<ofxPanel>> uniquePanels;

// in ofApp.cpp
// this compiled
uniquePanels.push_back (std::make_unique<ofxPanel>());
// this compiled too
uniquePanels.emplace_back(std::make_unique<ofxPanel>());
// and so did this
auto pGui = std::make_unique<ofxPanel>();
uniquePanels.push_back(std::move(pGui));

It seems the key is to use a “totally empty” ofxPanel in a unique pointer, get it into a container, and then set it up once its there. I couldn’t even make a unique pointer with a non-empty ofxPanel, much less get it into a container. And I couldn’t get an ofxPanel (no unique pointer) into a vector either.

#17

it is strange though that some compiled on 0.11.2 while not on 0.11.0.
The following should also work.

		auto p = make_unique<ofxPanel>();
		
		p->setup("panel" + ofToString(i), "", prevShape.x, prevShape.getMaxY());
		p->add(f0);
		p->add(f1);
		p->add(f2);
		p->add(f3);
		
		
		uniquePanels.emplace_back (std::move(p));

although I prefer the method where it is created and immediately moved into the vector.

#18

Yup that compiles too! I was having compiler issues with things like:

// compiled in 0.11.0, but not in 0.11.2
std::vector<ofxPanel> panels;
ofxPanel panel;
panel.add(f0); // borrowing same f0 from above code
panels.push_back(panel);
panels.push_back(std::move(panel));

// with a unique pointer
std::vector<unique_ptr<ofxPanel>>. uniquePanels;
ofxPanel aPanel;
aPanel.add(f0);
auto pGui = std::make_unique<ofxPanel>(aPanel); // compiled in 0.11.0 but not in 0.11.2
auto pGui = std::make_unique<ofxPanel>(std::move(aPanel)); // also won't compile in 0.11.2
uniquePanels.push_back(std::move(pGui)); // compiles 0.11.0

Edit: I was surprised that I couldn’t make a unique pointer and make it point to an existing ofxPanel with std::move(). But maybe this is because there isn’t a move constructor defined (anymore). I like less constructors though, rather than more. I’m not sure there is a benefit to delving deep into the “rule of five”, as oF seems to have a reliable and consistent approach to using containers with classes that have unique pointers.

#19

Hi, I think that doing such is somehow violating the C++ rules for the unique_ptr and maybe that is why it wont work. The unique_ptr must own the pointer, otherwise it does not make sense. which means that passing it through a std::move operation will not assure you that the pointer is owned by the unique_ptr.

1 Like