I'm working on a framework that can be used to develop 3d applications in opengl (with the intention of updating to use vulkan in the future). The basic idea is that the user initializes the App in main(), adds an initial scene, then controls the flow of the application from the scenes they compose, abstracting away the logic required for the fixed timestep loop, model loading, gpu rendering, etc. A quick outline is as follows:

> App (singleton responsible for managing window, input, and kicking off the main loop)
	> Scene (collection of stages)
		> Stages
  			> Camera (base class for PerspectiveCamera and OrthographicCamera)
			> Meshes (Stage owns unique instances of meshes and hands out pointers, ie 10 props could share the same mesh)
  			> Props (are transformable)
				> Mesh (pointer to mesh managed by stage)
					> Renderable (tracks gpu state and draws mesh) (optional as not required if headless)
			> Actors (are transformable and animatable) (not currently implemented)

I have a working example of the basic features and wanted to get some feedback before I get too carried away. My main concerns are as follows:

1. My usage of a singleton pattern to express the App object. It made sense to me at the time to implement this using a singleton since there can only ever be one instance of the App, however from the research I have been doing many people say not to use singletons and that almost everywhere they are used, a better solution could be implemented. Does anyone know of a better way for me to handle my App object, or is my current usage of the singleton pattern acceptable in this scenario?

2. In an effort to NEVER use "new" (except in the above singleton) I create and move several `unique_ptr`s. I chose to use `unique_ptr`s vs `shared_ptr`s to express that the object which owns the pointer is responsible for it's lifetime. For example, my `Stage` object contains a `meshes` member which is a vector of `unique_ptr`s to `Mesh` instances. When instantiating a `Prop`, I obtain the raw pointer from one of these `unique_ptr`s as the and save it as a member of `Prop`. My reason for doing this as opposed to using a `shared_ptr` is that the lifetime of the `Mesh` is managed by `Stage` and a `Prop` will never outlive it's `Stage`. Am I correct to implement this in the way that I have, or should I be using `shared_ptr` instead?

Also curious to know if there is anything that sticks out to anyone that is horribly wrong or could be better implemented.

The project is live on github. You should be able to download and open in visual studio without any issue as I have included all required dependencies. Please let me know if you have any issues: https://github.com/useless3d/useless3d

The name is meant to be a joke, and will be changed in the future if this project actually becomes “Useful”

whitwhoa said:
Does anyone know of a better way for me to handle my App object, or is my current usage of the singleton pattern acceptable in this scenario?

This is up to you, there is no good or bad solution. However, I prefer to let static things be static. C++ is unless C#, not fixed to a strict OOP model, so you are free to declare a namespace around a set of static global C-style functions and have the same usage as for example in a static class. This gives a very very small performance increase because you don't have to pass the this pointer to the stack, but it also prevents you from overriding your singleton, the initialization order problem and users intantiating your class even if it isn't supposed to.

whitwhoa said:
Also curious to know if there is anything that sticks out to anyone that is horribly wrong or could be better implemented.

I'm not a friend of “user aware” solutions, the reason people are writing their own stuff is that they want more. Mostly more control over what is going on in the background and you should trust people to know what they are doing (a little bit at least). A flow based render system is an interesting approach but why have I to load a scene first or manage stages with fixed meshes in them? Are meshes cached for reuse in other scenes/ stages or duplicated and loaded multiple times from disk (which is incredible slow) ?

I have not and do not intend to pull down a bunch of code to go over it in detail, and so my responses may miss something you've already addressed in code. Solely from the high-level documentation you've provided here and the questions you've asked:

1. One classic reason to not use a Singleton for your App object would be automated testing. If you want to spin up a bunch of tests in succession and ensure that the output of each test is in no way affected by the tests that ran before it… well, you can't if you're using globals. And a Singleton is really just a global in a fancy wrapper. Most engines that reach reasonable levels of complexity or success end up using automated testing somewhere. Without it, every change eventually becomes unreasonably risky.
   
2. I'm not entirely sure I understand this description, but it sounds to me like you're saying when you create a Prop, you grab a pointer to it's parent Stage and store it in a new unique_ptr on the Prop. If that is accurate, this usage of unique_ptr is incorrect, and means that every time a Prop is destroyed, the unique_ptr to the Stage containing will also be destroyed… so the Stage containing will also be destroyed, destroying every other Prop the Stage held… at least one of which has already been destroyed. You get my point. Madness ensues. If you're going to pass around references, you don't want multiple unique_ptrs. Depending on how tight your allocation and destruction methodology is intended to be, you may also not want multiple shared_ptrs either. Take a look at weak_ptr, as I think it sounds more like what I think you want.

As for sore thumbs… in my experience, all modern engines use a multithreaded renderer. As a result, the “Mesh" objects you add to a “Scene” are usually not the same objects used to submit the data for that “Mesh” to the GPU. At first I interpreted your “Renderable” to be the additional object used to store data for the render-thread view, and I objected to that object being “owned” by the Mesh. However, I then noticed that you declared that object is “optional”… so I no longer believe I understand what it is for. I think maybe you should take another pass at articulating specifically how your renderer itself will operate, and what ramifications that has on required state and associated ownerships.

Any reason all your draw() member functions can't be const?

You use a lot of creating std::uniqueptrs followed by a std::move into a container. You could just use emplace back and perfect forwarding. I don't have a problem with your use of uniqueptrs, it's pretty much ideal.

I may be old skool, but using std::optional<std::unique_ptr> as a way to hold a pointer that uses nullptr as a special value to indicate an optional value seems like code froth. I understand it's mosty a way to document the intention to use pointers the way they're usually used, but I feel that's up there with adding comments like “increment the index" to every line that increments an index.

The way you do singleton initialization is not thread-safe. That's OK as long as you can guarantee no threads before first use. If you make the instance static instead, you don't need to have that guarantee met by the users of your library because it's met by the language instead.

Then again, that use of singletons is cause for flaying and dipping in vinegar in my experience.

Shaarigan said:
A flow based render system is an interesting approach but why have I to load a scene first or manage stages with fixed meshes in them?

The goal is to create enough structure to allow a user to get started quickly, but enough freedom that they can implement their solution how they wish, similar to the various MVC frameworks used in web development. Is it your opinion that I am placing too much constraint with the scene/stage/prop/actor abstraction? I did briefly consider a node based solution similar to java's libgdx project. Maybe I should look into that again?

My thought process for using this was that a user could focus on creating scenes which contain multiple stages. Each stage containing it's own assets and camera, allowing the user to render multiple layers in either perspective or orthographic mode and within world space or view space (so for example a stage with a perspective camera which renders assets (props/actors) in world space and then a separate stage for a hud with an orthographic camera rendering in view space).

Shaarigan said:
Are meshes cached for reuse in other scenes/ stages or duplicated and loaded multiple times from disk (which is incredible slow) ?

When a stage is added to the scene, the method responsible for loading the assets from file checks the meshes container of the stage to see if the object has already been loaded for that stage. If it has, a pointer to the mesh which already exists is returned, otherwise a new mesh is created and added to the meshes container. Each prop then holds a pointer to the mesh owned by stage's meshes container and the unique transform data for that object.

Assets are only unique within individual stages. If the same asset is loaded in two stages, it will exist in memory twice. I currently fail to see a reason why a user would do that, however I'm sure there's probably a use case which currently evades me.

Thinias said:
Most engines that reach reasonable levels of complexity or success end up using automated testing somewhere.

Valid argument against my singleton approach. This gives me something else to consider when research a better way to handle this.

Thinias said:
it sounds to me like you're saying when you create a Prop, you grab a pointer to it's parent Stage and store it in a new unique_ptr on the Prop

That is not what I am saying. uniqueptrs are only used for owning objects. For example a stage owns a container of meshes so each element in that vector is a unique_ptr to a mesh. Props require a mesh as well, but do not own the mesh object (stage's meshes vector does) therefore I pass them as raw pointers.

Thinias said:
At first I interpreted your “Renderable” to be the additional object used to store data for the render-thread view, and I objected to that object being “owned” by the Mesh. However, I then noticed that you declared that object is “optional”… so I no longer believe I understand what it is for. I think maybe you should take another pass at articulating specifically how your renderer itself will operate, and what ramifications that has on required state and associated ownerships.

The optional “Renderable” member of a mesh is optional for the purpose of running in a headless state as you would still require the mesh data, but you wouldn't want a representation of it on the GPU (think authoritative server which only cares about inputs and calculating positions).

The renderer itself is currently extremely primitive. The primary loop runs in a single thread and loops through all stages of a scene, then all props of a stage (Actors as well in the future) and draws each prop object using it's mesh and transform data.

Bregma said:
Any reason all your draw() member functions can't be const?

Absolutely not! I need to do a refactor and better implement the use of const.

Bregma said:
You use a lot of creating std::uniqueptrs followed by a std::move into a container. You could just use emplace back and perfect forwarding.

That I will have to look into. At the time using move was just what I understood.

Bregma said:
I don't have a problem with your use of uniqueptrs, it's pretty much ideal.

That is a relief to hear. Thank you!

Bregma said:
The way you do singleton initialization is not thread-safe. That's OK as long as you can guarantee no threads before first use. If you make the instance static instead, you don't need to have that guarantee met by the users of your library because it's met by the language instead.

I had at first attempted to make App a namespace but ran into an issue with how it could be initialized. However as I sit here typing this out I might have just realized what I was doing wrong…so I might give that another shot as it does sit better with me than using the singleton.