Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A ModelBatch, a SpriteBatch, and a new Scene implementation #30

Closed
sriharshachilakapati opened this issue May 26, 2015 · 10 comments
Closed
Assignees
Milestone

Comments

@sriharshachilakapati
Copy link
Owner

As it stands right now, the scene graph is inefficient when there are many objects that needs to be rendered, and also in 3D, each model can have multiple materials, increasing the number of required state switches. And more, though models sort geometry based on materials, they are still drawn individually instead of batching all of them. A ModelBatch should be implemented to fix this issue.

ModelBatch batch = SilenceEngine.graphics.getModelBatch();

batch.begin(components);
{
    for (Entity3D entity: entities)
    {
        batch.render(entity.getModel(), entity.getTransform());
    }
}
batch.end();

The components is a list of scene components such as lights that affect the whole scene. The current Scene class can be split into Scene2D and Scene3D classes to avoid confusion between 2D and 3D entities. The SpriteBatch is similar to the ModelBatch, except that it will be used to batch sprites, where a Sprite is a container which contains an animation or a static texture per entity.

SpriteBatch batch = SilenceEngine.graphics.getSpriteBatch();

batch.begin();
{
    for (Entity2D entity : entities)
    {
        batch.render(entity.getSprite(), entity.getTransform());
    }
}
batch.end();

Both the ModelBatch and the SpriteBatch depends on the existing Batcher class, so there is no need to recreate these batches when the OpenGL context is lost due to a Display state change. As this is a major change to SilenceEngine, it will take some time to be implemented, but it will be in progress right from today.

@sriharshachilakapati sriharshachilakapati self-assigned this May 26, 2015
@sriharshachilakapati sriharshachilakapati added this to the 0.0.4a milestone May 26, 2015
@ShadowLordAlpha
Copy link
Contributor

has any more thought been given to making the 3D rendering more instance based? With large amounts of the same object just rotated differently it can save time as less data needs to be passed into the graphics card though its only really noticeable when there are a lot of the same model. This could be worked in with a list that sorts them into a map with the model as the main organizer then it could be the material (or just ignore this) then the transform as that should be different for every model skipping over the ones that are identical to one already there in case one is added twice for some reason.

(EDIT: you may have said something like that in your post -_-)

Another thing that can be done is rendering opaque things first from front to back then terrain then a skybox if there is one then transparent things from back to front so their transparent color is correct then water if there is any. Because of the way its rendered more pixels are likely to fail the depth test and as such skip the shader code where it can making the whole thing faster. Though this should only be done if the basic position can be easily gotten and ordered.

@sriharshachilakapati
Copy link
Owner Author

What matters more is the number of draw calls here. The less number of calls, the more the performance. Let's suppose, you are drawing 1000 instances of a single cube, which is 1000 * 6 (no. of faces) * 2 (no. of triangles per face) which means 12000 triangles. If we want instancing, yes, we could reduce the amount of draw calls, I agree, but it will make culling of entities that are not in viewport more difficult. However, we can implement instanced rendering as an option, let the user (developer of the game) use it if he wants.

Yet another idea, is to store all the static entities in a single/multiple VBOs, and draw them every frame. This should be done on the user side, as right now, all our shaders are designed to work with the data coming from the Batcher class. If you could join the IRC, we could discuss more on this topic.

@ShadowLordAlpha
Copy link
Contributor

(in the IRC but gonna post this here because reasons) how would it make it more difficult as for the most part the culling should be done in the code and 90% of that does not need to change enough to break it. You know the xyz and the scale (rotation doesn't matter here but it could for other things) so you get the point farthers from the center of the model when you load it in. From there you make a sphere and with that sphere you can check for culling by testing if any part of it lands in the viewport then draw it (simple version but you should get the idea)

@ShadowLordAlpha
Copy link
Contributor

Ok so continuing this from the IRC. Batch and instance are basically two different takes on the same Idea. Reduce the total number of rendering calls to speed up rendering or more basically the number of times the CPU calls and transfers data to the GPU.

Now Batching is a good idea and works very well for more static scenes where it does not have to be rebuild every frame. however due to everything being compressed into a single VAO and that VAO being recreated every frame it looses time in the CPU that could be used for rendering. Looking though the OBJModel code you also seem to recreate its VAO and VBOs when you could just simply store the VAO and VBO in the Model class as they should all have the same VAO for the most part. Also in your code to render a VAO you actually call bind() at least 6 different times in your batcher class.

If the VAO and VBOs were pre created and stored then you wouldn't need to worry about them again as you would assume that the data in the VAO and VBO is correct and because it is stored in the model class you could also assume that the user will always use the same model object when an entity uses that model as its model allowing you to use == to compare as it should be a bit faster than going though and using equal(Model) basic culling on if its in the viewport or not will also take place here using the basic method I described above all models will use the same basic shape (a sphere) so the full model doesn't even need to be know yet.

Now the material argument I remember you having before. Just use another VBO or two. Now there is an Indices buffer you can use in rendering so that you don't have to pass the same vertex data more than once (I don't remember if you use this but you should if you don't i can explain it more but this is getting long) you can use the same concept for Materials as the material data can be stored in a VBO and coordinated with a point using this buffer. This would be its own object because different models can have different materials and it would be attached on two the main VAO or passed in as its own list so that you also organize based on materials.

Also just another note that large batches of data have the ability to stall for longer times than lots of small amounts of data that add up to the same amount from what I've seen around on the internet. but this is already really long...

@sriharshachilakapati
Copy link
Owner Author

Let me give you a glimpse of what we are doing right now, and what it will be like once this is implemented, internal to these classes. Considering some psuedocode, the scene rendering right now looks like this.

foreach (component in components)
{
    component.use();

    foreach (model in models)
    {
        setMaterial(A);
        drawGeometry(A);

        setMaterial(B);
        drawGeometry(B);

        ....
    }

    component.release();
}

That is, we are making one draw call per each material in each model, and we repeat that for every component (lights). This, is the root cause of the performance issues we are having right now. We made each model contain multiple materials. What I want to change that, is to make it into this.

sort meshes from all the models here

foreach (component in components)
{
    component.use();
    Material material = meshes[0].material;

    foreach (mesh in meshes)
    {
        if (mesh.material != material)
        {
            drawCollectedGeometry();
            setMaterial(material = mesh.material);
        }

        collectGeometry(mesh);
    }

    component.release();
}

To achieve this, we have to redesign the Model class, turning a model into a simple collection of meshes, and each mesh will contain a reference to the material it uses. This allows the above rendering code to collect as many meshes as it could if they share the same material. So if you consider having 100 objects with two materials each, all hundred objects will be rendered only twice per component as opposed to rendering 200 times per component.

So I think, this will be a really huge improvement. What do you say?

@ShadowLordAlpha
Copy link
Contributor

as long as it improves performance im fine with it I'm just arguing the other mainstream way of doing it for the most part

@sriharshachilakapati
Copy link
Owner Author

I have just implemented the StaticMesh class. We can think of it as a mesh that stores a mesh directly in the GPU memory. These static meshes can be managed by the GraphicsEngine class, which will automatically dispose all the managed meshes.

StaticMesh sMesh = SilenceEngine.graphics.getStaticMesh(normalMesh);
sMesh.render(transform);

This doesn't recreate a static mesh all the time, the GraphicsEngine keeps a map of normal mesh to static mesh. Now the ModelBatch uses these static meshes to improve performance of rendering of pretty large meshes.

The meshes having less than 900 vertices are batched, and the remaining large meshes are simply turned up into static meshes and rendered. Here is a performance testing.

Mesh name Vertices Faces FPS for 400 instances Render Calls
Cube 36 12 600 2
Suzanne 46464 15488 30 800

I think this is pretty decent, as it is very rare to have high poly models in a game, and that too this can be improved with instanced rendering. There is no special commands to render using static meshes, as the ModelBatch will automatically decide which to use.

We have to do note, that when using static meshes, it is not possible to batch and reduce the render calls. The only way to achieve that is with instanced rendering, I'll look into that, but not any soon. Now to work on integrating the SpriteBatch and the ModelBatch into the scene.

@ShadowLordAlpha
Copy link
Contributor

Looks good.

When you do get around to implementing something like Instanced rendering you may wish to put a hint in the graphics engine that tells it if it should try and batch the mesh or if it should use instanced rendering with it. Both are good ways and do basically the same thing however if i have something say a tree that I plan on having like 5000 of in a scene (who knows why but it sounds like something I would do) and its low poly so something like under that 900 vertices limit you said above it would be better for the engine to use instance rendering instead of attempting to batch it. Telling the engine it should use instance rendering for some models would just be a better idea then something like counting uses of a mesh and determining it that way however doing it like that is still a valid way (just something to think about)..... I should go back to working on Yirath now.

@sriharshachilakapati
Copy link
Owner Author

That's a really good option. What I'd simply do is to overload the render method of the StaticMesh class to accept a list of transforms, and it will draw that mesh as instanced, with a transform per instance. According to some cool guys in ##OpenGL channel on the IRC, instanced rendering has no overhead to render only one instance, so we can simply use instanced rendering all the time, being single transform gives only single instance.

But there is also some cost for that, it is not with OpenGL though. That is, our shaders needs to be rewritten to make the transform matrix (mTransform) an attribute instead of a uniform to support instanced rendering, but that will also trigger a rewrite on the batcher code (it is still a minor correction). So for now, I will attempt to write the Scene3D class and remove the current Scene class, and work on instancing a bit later.

As for the option, I will be adding another addMesh overload that accepts a StaticMesh instance to the ModelBatch class. Thanks for this idea.

@sriharshachilakapati
Copy link
Owner Author

I think we are now done with what we thought of implementing initially. I'm now closing this issue, and postponing the instanced rendering for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants