Sorting objects to be rendered by their state

I'm writing a renderer to learn my way around OpenGL and right now I'm trying to understand how to sort the objects to be rendered. As I understand there are a few rules to this:

• Sort to minimize state changes (like glUseProgram)

• Sort solid objects from transparent objects

• Sort the solid ones front to back, to decrease overdraw

• Sort transparent ones back to front to make sure the ones in front don't hide the ones in the back.

• Sort by mesh to use instancing (?)

• Sort by materials (which is sorting by state I suppose)

What I have so far works and I get over 100 FPS but I'm not sure my approach is correct. Also, my processor and video card are pretty good, so I don't know how telling the FPS really is.

First I have a few classes that compose the state and material.

struct opengl_state
{
    GLclampf alpha_ref;
    GLenum alpha_func;
    GLenum blend_sfactor;
    GLenum blend_dfactor;
    GLenum cull_mode;
    GLenum depth_func;
    bool alpha;
    bool blend;
    bool cull;
    bool depth;
    opengl_state() = default;
    static opengl_state initial_state();
    bool operator textures;
    std::unordered_map vec2;
    //Other types (like vec3/mat4/etc are omitted)
    render_pass(luck::program* program) : program(program) {}
    bool operator passes;
    material(render_pass pass1)
    {
        passes.push_back(pass1);
    }
};

They can be sorted, which is something that can be used in the rendering step.

Then I have some structs to help me organize the scene, I can't simply sort the meshs because they can have more than one pass, so I split a single mesh into several passes. And then sort these passes.

```
struct _Uniforms
{
luck::entity entity;
std::unordered_map textures;
std::unordered_map vec2;
//Other types (like vec3/mat4/etc are omitted)
};

struct _Meshs
{
luck::mesh* mesh;
std::vector uniforms;
};

struct _Programs
{
luck::program* program;
std::vector meshs;
};

struct _States
{
luck::openg

Tips on coding style:

Names starting with an underscore _ are reserved for use by the C++ implementation and should definitely be avoided. I believe that you have defined the types _Uniforms, _Meshs, _Programs and _States with an underscore in the name to make them internal/private to your library. If this is the case, then remove the starting _ and place them inside a namespace named something like internal, priv or detail. Preferably, place all code from your library inside a namespace an place private library detail inside a child namespace of your library's main namespace.

You use some very long lines. Long lines are a common source of bugs. Take this one for example:

glVertexAttribPointer(glGetAttribLocation(program_id, "texcoord"), 2, GL_FLOAT, GL_FALSE, sizeof(mesh_data_resource::vertex), (void*)offsetof(mesh_data_resource::vertex, u));

It would be far more readable to break it:

glVertexAttribPointer(
    /* index      = */ glGetAttribLocation(program_id, "texcoord"), 
    /* size       = */ 2,  
    /* type       = */ GL_FLOAT, 
    /* normalized = */ GL_FALSE, 
    /* stride     = */ sizeof(mesh_data_resource::vertex), 
    /* pointer    = */ (void*)offsetof(mesh_data_resource::vertex, u));

Notice that I've also added a comment to name the function parameters. This can be very helpful to people unfamiliar with some of the libraries used in the project, in this case, OpenGL.

The method renderable_system::render() is massive! You can break it into quite a few more specific subroutines. This will make it a LOT more readable and less intimidating. To name a few, you could separate it into the tasks: compute_transforms, sort_objects, build_state_list, do_render. You can probably break it even further, these are just a few of the helper functions you can introduce.

I like to align similar assignment statements, such as initialization of several variables. On opengl_state::initial_state(), I suggest that you reformat to:

opengl_state opengl_state::initial_state()
{
    opengl_state state {};
    std::memset(&state, 0, sizeof(opengl_state));

    state.alpha_ref     = GL_ALWAYS;
    state.alpha_func    = 0;
    state.blend_sfactor = GL_ONE;
    state.blend_dfactor = GL_ZERO;
    state.cull_mode     = GL_BACK;
    state.depth_func    = GL_LESS;
    state.alpha         = false;
    state.blend         = false;
    state.cull          = true;
    state.depth         = true;

    return state;
}

inline is not necessary when the method is defined directly inside the body of the class declaration:

inline bool transparent()
{
    return alpha || blend;
}

From opengl_state can be just:

bool transparent()
{
    return alpha || blend;
}

Also, is_transparent() would be a better, more descriptive name, for the method above.

At the end of renderable_system::render(), there is a superfluous return;. Your should remove it.

A few tips on performance and OpenGL:

You are calling glGetAttribLocation() and glGetUniformLocation() inside renderable_system::render(). This is a waste since the values never change and the strings passed are constants. You should query the locations on startup and cache the values inside named variables. E.g:

GLint viewLocation = glGetUniformLocation(program_id, "view");
// and store 'viewLocation' somewhere. Probably inside a ShaderProgram class...

You can speed rendering quite a bit by using VAOs. It is very likely that your video card supports VAOs, since you are already using shaders.

While you are at it, switch to core profile OpenGL and ditch those calls to glPushAttrib.

As to sorting, I suggest that you completely separate solid from translucent objects and render them on separate passes. Sort the translucent ones back-to-front as you do now.

I'm not sure if there is going to be any gain by sorting the solid ones in front-to-back order. The level of overdraw is usually not that big on most scenes. You will have to profile to find that out.

When defining your sorting predicates, refer to this "hot-cold" list of OpenGL states.

StackExchange Code Review Q#61229, answer score: 7