-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update main.* in examples to have return values and updated syntax. #4677
base: master
Are you sure you want to change the base?
Conversation
One small question -- in Perhaps this should also be updated to the newer syntax by selecting a specific opengl version? |
Nice work @bakercp 👍 !!! |
thanks! yes any reference to the programmable renderer won't work any more and it should be called using the new syntax. also we should change every main to setup the window using the window settings, that's the most important change. the standard main should look like: #include "ofApp.h"
#include ofMain.h
//---------------------------------------------------
int main(){
ofWindowSettings settings;
settings.width = 1024;
settings.height = 768;
settings.windowMode = OF_WINDOW;
ofCreateWindow(settings);
ofRunApp(std::make_shared<ofApp>());
} |
So -- the standard main you have referenced above -- seems to be a little different than previously discussed as it doesn't have a return value and includes #include "ofAppRunner.h"
#include "ofApp.h"
int main(){
ofWindowSettings settings;
settings.width = 1024;
settings.height = 768;
settings.windowMode = OF_WINDOW;
ofCreateWindow(settings);
return ofRunApp(std::make_shared<ofApp>());
} |
yeah sorry i was missing the return, i'm not 100% sure about not including ofMain. main is usually fast to compile so including ofMain there shouldn't be such a problem and it'll allow people to use special windows, check for files existing... or any other more complex config in main. also better to include ofApp.h before ofMain.h/ofAppRunner.h that way ofApp doesn't get all of ofMain.h already included which might clash in some cases |
Also, this seems like a separate issue -- but it seems inconsistent to have a version of ofRunApp that starts the main loop and one that doesn't ...
return ofRunApp(std::make_shared<ofApp>()); vs. ...
auto window = ofCreateWindow(settings);
auto app = std::make_shared<ofApp>();
ofRunApp(window,app); // doesn't return anything
return ofRunMainLoop(); If I understand -- the reason for this is to allow multiple windows to be started ... e.g. ofRunApp(guiWindow, guiApp);
ofRunApp(mainWindow, mainApp);
return ofRunMainLoop(); but perhaps a better solution for multi-window would be to have a different function for attaching windows to apps so that the api is a little cleaner? |
yes i noticed that during a workshop somedays ago, we could just create the mainloop explicitly in that case something like:
|
+1 to a solution that removes and does this look like an appropriate default main.* file? #include "ofApp.h"
#include "ofMain.h"
int main(){
ofWindowSettings settings;
settings.width = 1024;
settings.height = 768;
settings.windowMode = OF_WINDOW;
ofCreateWindow(settings);
return ofRunApp(std::make_shared<ofApp>());
} If so, I'll make changes in the PR and also update the templates in the other PR to remove the use of the older |
yes that should be it. i have a couple of doubts about what should be the default now :) if we are going to change the syntax for multiwindow, it probably makes sense to have the same syntax for all the applications so it's easier to go from one -> several so perhaps we should use: #include "ofApp.h"
#include "ofMain.h"
int main(){
ofWindowSettings settings;
settings.width = 1024;
settings.height = 768;
settings.windowMode = OF_WINDOW;
auto window = ofCreateWindow(settings);
auto app = std::make_shared<ofApp>();
auto mainLoop = ofGetMainLoop();
mainLoop.add(window, app);
return mainLoop.run();
} ? it's way more verbose but it's easier to go from there to multiwindow. kind of the same happens with setting the GL version or other settings specific to the platform window. we now have:
perhaps we should discuss a little bit about this to try what would be a default option that makes it easier to use the most advanced uses but still is multiplatform... anyway let's try to find the best solution mostly so you don't have to change the files multiple times |
Personally, and from a teaching perspective, making things multi-platform and verbose is better. I would rather use (and teach) a more verbose, step by step approach (like what you propose) than hiding everything at the window configuration stage inside of a mysterious That said, I think we should get some other voices in on this conversation ... @danoli3 @digitalcoleman -- what do you think? |
I also prefer showing the options available. I also think a slightly less specific way to set the programmable renderer would be useful. |
I would agree that having this be more verbose will encourage more creative solutions, and additionally, it is not so verbose that it is indecipherable. |
Seems like a pretty good consensus that a more verbose, flexible main.cpp. I think in order to do this then, the next step is to update the core with the additional ofMainLoop syntax that will allow something like this: #include "ofApp.h"
#include "ofMain.h"
int main(){
ofWindowSettings settings;
settings.width = 1024;
settings.height = 768;
settings.windowMode = OF_WINDOW;
auto window = ofCreateWindow(settings);
auto app = std::make_shared<ofApp>();
auto mainLoop = ofGetMainLoop();
mainLoop->addWindow(window, app);
return mainLoop->run();
} There is already a method called |
Or, perhaps it would be better to have |
@arturoc let me know when you get the syntax you suggested updated and I can update this PR. |
@kylemcdonald @arturoc @ofTheo I'm happy to bring this up to date if it will be merged. Also happy to sync it with this one. Let me know. |
this is way more comprehensive than what i was planning on doing... :) i hope this makes it in. i have a feeling 0.10.0 is going to be around for a while. |
I'm not really sure about a couple of things regarding which function calls we want to use, in the future we should move all the examples to use ofGLSettings but because now there's ofGLSettings and ofGLESSettings we would need ifdefs in the main files so they would be multiplatform which i don't like much. Probably the solution is to join those two structs into one. We also have ofGLFWWindowSettings which i guess people can just opt in if they want more specific features Apart from that using a shared_ptr for the app was probably not a very good idea and i think we should remove it and at most use a unique pointer or just a plain allocation as we do now that will be turned into a unique ptr internally. So can you include ofMain instead of ofAppRunner so people can use different window settings without having to know the specific header and remove the shared pointer? i think we can merge this as it is and leave the settings setup for next release |
Currently main.* in most examples (with the exception of a few iOS examples) don't actually have a return statement. This hand-crafted (not grepp'ed :)) PR updates all main.cpp example files to fix this.
The updated syntax used is reflected in the new main.* templates here: #4675.