5

I often hesitate to use friend classes in C++. Since it often does not feel right, or just seems to be an indicator of bad design. Though, in this case I think a friend class might improve the design. Let me explain:

Let us say I have a very simple mesh class:

class Mesh {
    friend class Renderer;
public:
    using Vertices = std::vector<Vertex>;
    Vertices::iterator begin() {
        return vertices_.begin();
    } 
    Vertices::iterator end() {
        return vertices_.end();
    } 
    void invalidate(){
        valid_ = false;
    }

private:
    Vertices vertices_
    bool valid_;        
}

The renderer looks something like this:

class Renderer {
public:
    void render(Mesh & mesh){
        if (!mesh.valid_){
            //The mesh has changed, update internal data.. GL buffers etc.
            mesh.valid_ = true; // Mesh becomes valid again.
        }
        //render...
    }
}

So, this is of course a very simplified example. My question is, if this is a good design in this case? Clients can only call invalidate() on meshes. And only the renderer is allowed to "validate" a mesh again. Without a friend class, A public setter would have to be used in the Mesh class. Hence providing a "leaky" interface. Allowing clients to "validate" meshes, which I do not want.

Edit to clarify

A valid mesh in this sense means that its corresponsing data in other subsystems is the same. The renderer has its own mesh data in its own form (a GL buffer). Therefore, only the renderer should be able to validate, since it knows about its own mesh data. Clients can edit meshes, and invalidate them only.

Morgan Bengtsson
  • 353
  • 4
  • 8
  • 1
    Why not have a Mesh::Prepare function that validates itself internally? –  Jan 04 '15 at 21:51

4 Answers4

4

The obvious approach to me would be to invert the interaction between Renderer and Mesh, making the render() method a member of Mesh instead, which would take a renderer reference. A Mesh is a higher level concept that the renderer doesn't have to know about, while the mesh obviously has to know about the renderer to be able to draw itself, so it should be a one-way dependency.

YMMV, but I think the renderer should only deal with basic geometry primitives or perhaps even simpler, just vertices and buffers (plus the other low level stuff like shaders and render states, of course).

Example:

class Mesh {
public:
    void render(Renderer & renderer) {
        if (!valid_) {
            renderer.updateVertexBuffer(vboHandle_, vertices_);
            valid_ = true;
        }
        renderer.drawVertexBuffer(vboHandle_);
    }
private:
    // data ...
};
Community
  • 1
glampert
  • 3,067
  • 1
  • 18
  • 32
  • I think the other way around. Making the mesh dependent on the Renderer, imposes unnecessary coupling all the way down to OpenGL in this case even. If the mesh does not know about vbo handles, specific vbo update/draw methods. Then the Renderering becomes API independent. I could make a new DX renderer for example. – Morgan Bengtsson Jan 07 '15 at 09:19
  • 1
    @MorganBengtsson, you don't have to sprinkle GL calls in the mesh code, that's why the renderer class exists, to wrap that low-level library in a more friendly and portable interface. And VBOs are a pretty standard concept, so you should probably have a class for them as well... – glampert Jan 07 '15 at 13:34
3

This is one reason why the friend keyword exists.

Both Mesh and Renderer are part of the same system: You can't really use Mesh without Renderer and vice-versa.

You should probably still use a getter mesh.IsValid() and setter mesh.MarkValid() in case you need to add functionality later, rather than access valid_ directly.

It would be nice if C++ had access groups so we could mark only some members public to only a certain group and retain some access protection. In theory its possible to get close to this with multiple inheritance but it becomes a bigger maintenance mess than using self discipline.

Cheers,

Stephane Hockenhull
  • 11,962
  • 1
  • 25
  • 44
1

I do not think that allowing clients to validate a mesh is a problem, this allows for a clean api and maintainable code. It makes it extensible for example if later you want to add some code into the validation step etc. (However the last part could also be handled with private functions and a friend class)

Remark that if clients cannot validate / prepare a mesh then what is the point to know if it is in a valid state from a client perspective ? Should this be kept in the api ?

That said, there are alternatives to enforce the validation (or visibility scope) here:

  • create a member function that uploads the data to the gpu when in dirty state;

  • use some sort of opaque RendererData object that only the renderer can use (not exposed in the public api);

  • store the dirty flag in the renderer (I often use handles for resources which would make this approach efficient with a constant time access, O(1) vs. O(log(n)) (see Game Programming Gems vol.1) ).

Thelvyn
  • 379
  • 3
  • 10
0

I'm not sure if this is what Stephane is talking about when mentioning multiple inheritance, but you could theoretically create a base class that is a friend to the mesh class and implement functions that access mesh internals there, called, say, MeshPrivateAccess. You can then derive classes like the renderer from that (eg. class Renderer : protected MeshPrivateAccess) so that accidental access is prevented, but other classes can access the data without needing to update the Mesh class to include friend declarations (impossible if Mesh was designed to be part of a library for example).

It's a bit cleaner that way, but I wouldn't recommend this solution here since it doesn't solve the fundamental design problem.

If you really want to store per-mesh data inside of a renderer, I'd probably store the valid_ variable inside of the renderer as well. Conceptually, that variable has got nothing to do with the mesh data structure itself, as it is related to a cache that exists somewhere in the renderer, and with the current design, changing the implementation of the rendering process or caching mechanism would require changes to the Mesh class. Storing any isChanged variable inside the renderer instead solves that.

That leaves the question of how to best update it.

I'd probably use something like the observer pattern or delegates to notify the renderer when a mesh has been modified. It can then update its isChanged status for the mesh as a response, or do anything else you may want to do in the future.

That mechanism could for example be used to also notify the renderer when the mesh is deleted so it will be able to free the associated caches and so on. And you could re-use that infrastructure if you want other actions to happen that are implemented in other parts of your code, like, say, auto-saving a mesh whenever it is changed, or having a physics simulator mark its cached bounding volume calculation as invalid, initiate the redraw of a view when you are running in an editor mode, and so on. It would also allow it to scale if one day you needed to run multiple renderers concurrently (say, for running on multiple displays that use different graphics API contexts and thus can't share resources). All that without ever having to touch the Mesh class.

Depending on what type of process changes the mesh, you might also consider having that code issue the change notification instead of having the mesh do it, but most likely the Mesh itself would be the best solution.

Peter
  • 1