Splitting WorldRendererLwjgl

[manu3d]

I've been looking into the WorldRendererLwjgl class, which is a fairly massive, intimidating wall of code - 1221 lines at the time of this post.

While there are plenty of opportunities to make it less intimidating even leaving it as a single class, I've noticed that this implementation basically does two things:

• keeps the renderable meshes up to date
• renders the world

I've been wondering then if it would be a good idea to somehow split these two sets of functionality into two classes, one perhaps called WorldMeshesUpdater and the other WorldRendererLwjgl. This wouldn't necessarily break the WorldRenderer interface: the WorldRendererLwjgl might simply delegate its update-related tasks to WorldMeshesUpdater, at least for the time being.

Thoughts?

 

[manu3d]

Follow up on this one:

I'm currently in the process of attempting to split the implementation as suggested above. I've tentatively named the interface/class holding the data to be rendered as RenderableWorld/RenderableWorldImpl. The goal is for the WorldRenderer to have only one responsibility, to render the world, and for the RenderableWorld to have a similarly specific responsibility, to hold and maintain the renderable data up to date.

I'm noticing however that the WorldRenderer interface is perhaps burdened by methods that are not really rendering-related but more data-related and should be elsewhere. For example, the method getBlockLightValueAt(Vector3f pos) is actually simply drawing data from the WorldProvider. Given that the WorldProvider can be easily reached through the CoreRegistry, I'm not quite sure why it is useful for the WorldRenderer to be a second point of access for that data. As such I will be investigating a considerable reduction of the interface.

Finally, I'm seeing what looks like a potential performance issue in the WorldRendererLwjgl.update() method. By my understanding of the main loop, this method is called every tick. And each frame there can be multiple ticks. But what seems to be the main task of this method is to update the meshes to be rendered. So, I'm thinking that perhaps all that happens in the update() method should really happen in the render() method instead, only once per frame.

Feedback is welcome!

 

[Immortius]

I would be more interested in the removal of the music system and physics setup and general world setup from world renderer in the first instance.

Having preparation for rendering and actual rendering in the same system isn't as clearly an issue - but it is probably a good idea to split them regardless if they are reasonably loosely coupled, or if one is lwjgl specific and the other isn't.

Certainly you are correct in that there is various functionality and lookups funneled through the WorldRenderer that could be removed.

 

[manu3d]

Thank you Immortius for your feedback.

Right now I'm in a phase where the code is raising a lot of dust as I'm dealing with big items. Once the dust has cleared a little I think I should be able to address the oddity of having physics and audio dealt with in the renderer.

Concerning preparation for rendering and actual rendering, yes, I can confirm the coupling is fairly loose, with the WorldRenderer being a consumer of what the RenderableWorld (as I tentatively named it) provides, a one-way relationship. It seems to be possible to neatly separate them and conceptually it will be easier for everybody to think about them, creating a classical presentation/content division of responsibilities. I am concerned about a performance hit that the split in two separate objects might cause, but a working (and still messy) version I have seems to run at the same speed as the original code.

I will also double-check if the RenderableWorld is Lwjgl agnostic. It should be, as it is all about chunks and Terasology's internal data handling. I also see the potential for the WorldRenderer itself to be Lwjgl agnostic, as the DefaultRenderingProcess class is where the bulk of the opengl commands are issued. I just haven't looked too deeply into that yet but it is certainly something I will do within this round analyses and changes.

 

[Immortius]

An alternative possibility would be to have chunks as entities, with the mesh rendered via MeshComponents (probably would need multiple entities per chunk to support the multiple mesh per chunk). This would mean all rendering happens through the standard MeshRendering system (and SkeletonRenderer). However there is likely some advantage to the more tailored rendering for chunks.

 

[manu3d]

I'll keep that in mind. Right now I don't even know why a chunk has more than one mesh. Or what is considered a mesh, for that matter.

 

[Immortius]

A chunk has one mesh for each rendering style associated with its blocks . So a mesh for the opaque block sides, a mesh for the translucent block sides, a mesh for the transparent block sides, along those lines. Each mesh contains the geometry for those sides.

 

[manu3d]

A-ha! Thank you!!