openframeworks / openFrameworks

openFrameworks is a community-developed cross platform toolkit for creative coding in C++.
http://openframeworks.cc
Other
9.97k stars 2.55k forks source link

Feature : Sugar for ofParameterGroup list initialization #4966

Closed elliotwoods closed 8 years ago

elliotwoods commented 8 years ago

hey, i'm just trying something out here

if we have a class like

class ParameterGroup : public ofParameterGroup {
public:
    typedef initializer_list<ofAbstractParameter *> InitializerList;
    ParameterGroup(const InitializerList & parameters) {
        for(auto parameter : parameters) {
            this->add(*parameter);
            delete parameter;
        }
    }

    ParameterGroup(const string & name, const InitializerList parameters)
    : ParameterGroup(parameters) {
        this->setName(name);
    }
};

Then we can write code like:

    ParameterGroup parameters = {
        new ofParameter<int>("My int", 0),
        new ofParameter<string>("My string", ""),
        new ParameterGroup("Subgroup", {
            new ofParameter<int>("Deeper int", 0),
            new ParameterGroup("Subgroup 2", {
                new ofParameter<int>("Even deeper int", 0),
            })
        })
    };

which will evaluate to a tree of parameters. And since C++11, we can just write that directly into the header file. neat huh?

this syntax isn't ideal of course. Much better would be something like

    ParameterGroup parameters = {

        ofParameter<int>("My int", 0),
        ofParameter<string>("My string", ""),

        "Subgroup" : {

            ofParameter<int>("Deeper int", 0),

            {
                ofParameter<int>("Even deeper int", 0),

            },

        },
    };

(note that initializer_list doesn't support & or && so this would mean having a temporary wrapper type constructed on the abstract reference)

Or even more sugary is also possible! Especially if there's a way to create reference instances to the individual parameters which can accessed with code completion.

I mostly thought of this after seeing the new json libraries which have great syntactic sugar, e.g.:

//jsoncons
   json book4 = json::parse(R"(
    {
        "category" : "Fiction",
        "title" : "Pulp",
        "author" : "Charles Bukowski",
        "date" : "2004-07-08",
        "price" : 22.48,
        "isbn" : "1852272007"  
    }
    )");
//jeayeson (C++14)
json_value val // json_value can be any json type; here, it's a json_map
{
  { "hello", "world" }, // nested pairs represent key/value
  {
    "arr",
    { 1.1, 2.2, 3.3 } // arrays are easy
  },
  {
    "person",
    {
      { "name", "Tom" },
      { "age", 36 },
      { "weapon", nullptr } // can also use json_null
    }
  }
};
arturoc commented 8 years ago

You can almost do something like that now:

ofParameter<float> p1{"p1", 0.f, 0.f, 1.f};
ofParameter<float> p2{"p2", 0.f, 0.f, 1.f};
ofParameter<float> p3{"p3", 0.f, 0.f, 1.f};
ofParameterGroup group1{
    "group1",
    p1,
    p2,
    p3,
};

@tgfrerer and i were discussing some days ago how to have that inlined, and i think you just need to change the current constructor with something more specialized and it'll work

tgfrerer commented 8 years ago

In VS2015 you can already do something a la:

ofParameterGroup group1{
    "group1",
    ofParameter<float> {"p1", 0.f, 0.f, 1.f},
    ofParameter<float> {"p2", 0.f, 0.f, 1.f},
    ofParameter<float> {"p3", 0.f, 0.f, 1.f}
};

Which is rather nice. Clang and gcc, however, seem to have trouble with disambiguating this. A JSON style syntax to define parameters would be very sweet, indeed.

ofZach commented 8 years ago

love the way this is heading and big thumbs up to allocation like this.

I know this is not the same, but I would love being able to allocate a parameter group via actual json (and to read out it's current state as json) now that we have ofJson :) I think this maybe hard to do because of the events / etc, but in the past I've hit situations where it made sense to get a snapshot of a parameter group at a given time and it seemed quite useful if there's a way to serialize it.

having experimented with dat.gui http://workshop.chromeexperiments.com/examples/gui/#1--Basic-Usage and https://github.com/dmvaldman/guiGlue I've gotten quite fond of setting up the gui via json now...

elliotwoods commented 8 years ago

exactly! it would be great for gui and serialization

currently i do:

struct {
    ofParameter<int> param1("Blah", 0, 0, 10);
    struct {
        ofParameter<int> param1("Nested", 0, 0, 10);
    } group1;
} myParams;

And of course that means the serialization/deserialization/guification is all manual

arturoc commented 8 years ago

yes we were thinking something along those lines for the extension of ofxGui we are working on where you can set somethings related to style... using json.

also (de)serializing an ofParameterGroup will be available once we implement (de)serizliazer for ofJson

elliotwoods commented 8 years ago

Pinging @satoruhiga also

elliotwoods commented 8 years ago

One thing i've noticed is that ofParameterGroup is generally used as a reference class not a storage class i.e. you need to do like

ofParmeterGroup params;
ofParameter<float> param1, param1;

i.e. the parameters should still be stored elsewhere by the user, and the group just stores pointers

is there a usage pattern where the parameter group itself stores the instances?

elliot

arturoc commented 8 years ago

you could store the parameters in the parameter group and access them by name if you want but the syntax is much nicer if you don't so yes most of the time ofParameterGroup is just a way of grouping the parameters by category.

If you do so in the headers and also initialize the parameters there, it makes for a nice reference of the parameters of each class:

ofParameter<float> width{"width", 10, 0, 100};
ofParameter<float> height{"height", 10, 0, 100};
ofParameterGroup size{
    "size",
   width,
   height,
};

ofParameter<float> acc{"acceleration", 0, 0, 100};
ofParameter<float> vel{"velocity", 0, 0, 100};
ofParameterGroup movement{
    "movement",
   acc,
   vel,
};
elliotwoods commented 8 years ago

got another one: (edited...)

Usage (in header)

    struct : ofParameterGroup {
        ofParameter<int> portNumber{ "Port number", 4444 };
        ofParameter<bool> connected{ "Connected", false };

        PARAM_DECLARE("Connection", portNumber, connected);
    } connection;

Implementation

#define PARAM_DECLARE(NAME, ...) bool paramDeclareConstructor \
{ [this] { this->setName(NAME), this->add(__VA_ARGS__); return true; }() };

This has both the ofParameterGroup and a struct storing the parameters in the same place.

I don't think this is going to be popular, but it's what I wanted :)!

elliotwoods commented 8 years ago

OK been testing PARAM_DECLARE for a while, and it's so so good i'd like to propose we add it to ofParameter.h Pinging @prisonerjohn for a second opinion also

arturoc commented 8 years ago

we are using it in this project i'm working with @prisonerjohn and my feeling is that wherever we use it (most of the times) is just a signal that we should be separating that functionality into another class but we haven't done it yet, for example:

class ofApp{
struct : ofParameterGroup{
    struct : ofParameterGroup{
         ofParameter<bool> depthTest{...};
         ofParameter<bool> alpha{...};

         PARAM_DECLARE("Render", depthTest, alpha);
    }render;

    struct: ofParameterGroup{
         ofParameter<int> portNumber{...}

         PARAM_DECLARE("Connection", portNumber);
    }connection;

    ofParameter<float> something;
    PARAM_DECLARE("App parameters", render, connection, something);
}parameters;

should be:

class Renderer{
    ofParameter<bool> depthTest{...};
    ofParameter<bool> alpha{...};
    ofParameterGroup parameters{
        "render",
        depthTest,
        alpha
    };
};

class Connection{
    ofParameter<int> portNumber{....};
    ofParameterGroup parameters{
        "connection",
        portNumber
    };
};

class ofApp{
     Renderer renderer;
     Connection connection;
     ofParameter<float> something;
     ofParameterGroup{
           "app parameters",
           renderer.parameters,
           connection.parameters,
           something
     }
}

there's really few cases where you really want to group the parameters of a class in categories but you can do more or less the same as:

class ofApp{
     struct RenderParameters: ofParameterGroup{
         ofParameter<bool> depthTest{...};
         ofParameter<bool> alpha{...};
         RenderParameters()
         :ofParameterGroup("Render", depthTest, alpha){}
     } render;
}

which is slightly more verbose but i don't like very much the idea of having a macro as part of the standard api.

prisonerjohn commented 8 years ago

I'm gonna have to agree with @arturoc on this one, sorry @elliotwoods :)

Although I've also adopted the PARAM_DECLARE macro in a bunch of projects, I feel it's definitely more of a personal preference and doesn't quite fit in with the rest of the openFrameworks API.

Having to re-list all parameters in the PARAM_DECLARE call is also prone to error. It's happened a few times that I'd add a parameter to the list and forget to add it to the end of the macro call, so the ofParameterGroup would not know about it. An ideal solution would be something along the lines of what @tgfrerer has above, it's unfortunate it doesn't work across all compilers.

elliotwoods commented 8 years ago

the issue with @tgfrerer's method is that you don't have static access to the parameters (can't use this->parameters.myValue, need to use something like this->parameters.getParameter("myValue"))

The issue with @arturoc's (first) method is that you can't treat the struct as an ofParameterGroup (e.g. add the struct and sub-structs to a GUI). Second method looks workable for places where PARAM_DECLARE isn't around.

Thanks for the feedback

elliotwoods commented 8 years ago

also to confirm..arturo's second method doesn't work (causes crash) because the RenderParameters constructor will be called before the constructors on its member elements, hence PARAM_DECLARE adding an unused bool member which when constructed itself causes the param grouping magic to happen