Basic OpenGL Renderer class

I made this Renderer class recently, to simplify the user interface of my library's API. I would like to ask advice about the move semantics (copy constructor, std::unique_ptr, ...), or any other things you can point out to improve my code.

As you can see, I take in parameters in RenderObject in the form of a pointer. This is simply because Material uses a Texture class, whose assignment operators are disabled. It only has a move constructor (C++11), but Material cannot use it.

RenderObject.h

#pragma once

#include "alpha/Mesh.h"
#include "alpha/Texture.h"
#include "alpha/Light.h"
#include "alpha/Transform.h"

class RenderObject
{
public:
    RenderObject(
        Mesh* mesh, Material* material,
        Transform transform = Transform()
        )
        :
            m_pmesh(mesh),
            m_pmaterial(material),
            m_transform(transform)
        {}
    ~RenderObject(){}

    void SetTransform(Transform transform){
        m_transform = transform;
    }
    Transform GetTransform(){
        return m_transform;
    }
    Mesh* GetMesh(){
        return m_pmesh;
    }
    void SetMesh(Mesh* mesh){
        m_pmesh = mesh;
    }
    Material* GetMaterial(){
        return m_pmaterial;
    }
    void SetMaterial(Material* material){
        m_pmaterial = material;
    }
    void SetPosition(glm::vec3 position){
        m_transform.SetPos(position);
    }
    void SetRotation(glm::vec3 rotation){
        m_transform.SetRot(rotation);
    }
    void SetScale(glm::vec3 scale){
        m_transform.SetScale(scale);
    }
    glm::vec3 GetPosition(){
        return m_transform.GetPos();
    }
    glm::vec3 GetRotation(){
        return m_transform.GetRot();
    }
    glm::vec3 GetScale(){
        return m_transform.GetScale();
    }

private:
    Transform m_transform;
    Mesh* m_pmesh = nullptr;
    Material* m_pmaterial = nullptr;
};

Material.h

```
class Material{
public:
template
Material(T&& texture, float shininess,
con

Resource management:

I general, the code seems alright to me, but one thing you continue to do, if I recall from your previous questions, is to use raw pointers and manual resource/memory management. So I'd like to urge you to look into the standard smart pointers and start using them.

The basic usage is:

-
Resource is shared between instances of different objects: Frequent case for things like textures, materials and shaders. Use a shared_ptr for them.

-
Resource is explicitly owned by an instance of an object: In a renderer, this could apply to many types of things, from light sources to object transforms. Use a unique_ptr for those.

A few other things:

-
RenderObject is just a data container with a bunch of accessors. When you get to this point, it might be worth considering just making it a plain struct with all fields public, since there is no difference regarding encapsulation when you can freely get/set all fields. In such cases, the accessors only add a level of unnecessary indirection. However, you might also consider a redesign altogether, since the heavy use of accessors indicate that all the algorithms involving a RenderObject are outside the class, which in turn indicate that it is probably strongly coupled with other areas of the code and systems.

-
I'm wondering what this is about?

~Material(){
     m_texture.Dispose(); ///not necessary... ressources are automatically disposed in texture !
 }

So, if it is not necessary, as you say, why is it there? It would be safer indeed if the destructor of Texture did the cleanup, so you should probably avoid exposing a public Dispose() method anyways, to prevent making the mistake of having a disposed/invalid object in your hands. This is specially true for library code, make it as hard as possible for the user to shoot himself in the foot (dedicated users will still manage, but don't facilitate ;)).

-
The fact that the constructor of Material is templated for a texture type puzzles me:

template
Material(T&& texture, float shininess, ...

The member m_texture is of type Texture, so unless there's some polymorphic conversion going on here, the template parameter should just be a Texture &&.

-
Don't this-> qualify member variables, ever! We have a very amusing example right here on CR of how using it can lead to some pretty hilarious bugs.

-
Avoid writing the empty constructors/destructors and let the compiler do its job providing the empty defaults. But in particular, don't do this:

~ForwardRenderer(){}///default dtor

Yep, anyone that has been programming in C++ for more than two days knows what a destructor looks like. Unless you're using the comment for trolling purposes[*], trim it down.

-
std::vector* m_pobjects in ForwardRenderer seems like it could be declared by value. Then change the constructor to move the parameter or take it by && move-reference. The less pointer chasing the better when it comes to performance critical applications like real-time rendering. Make good use your data cache.

[*]: I recall once reading a comment just above a class destructor that said something like "If you don't know what this is, then you're probably going to get fired..." -- Obviously the author was being funny, and this kind of internal jokes can help bootring the morale of a team, but otherwise, never comment explaining obvious aspects of the language.

StackExchange Code Review Q#98060, answer score: 5