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

Batch, Batch, Batch #282

Closed
kvark opened this issue Aug 25, 2014 · 10 comments · Fixed by #285
Closed

Batch, Batch, Batch #282

kvark opened this issue Aug 25, 2014 · 10 comments · Fixed by #285

Comments

@kvark
Copy link
Member

kvark commented Aug 25, 2014

Reasoning

Shader program has a central role in the draw call, because it dictates what data needs to be provided and how. When we have a program and a mesh, for each program-requested attribute, we look for it in the mesh, and bind. It is inefficient, and the results of it should be cached. Same goes for the shader parameters, but we already have their match results cached. It is contained in a hidden link structure that is generated by #[shader_param], and stored in UserProgram (the L generic type). Next, the mesh binding link can look the same, and we need it. The mesh link is between user's mesh and the program, and the shader parameter link is between user's parameter struct and the program. Once we put mesh, mesh link, parameters, parameter link, and the program together in a struct, we almost have a batch. It is natural to add mesh slice to it and a state. Thus, batch concept is an evolution of our existing framework.

I asked for the feedback because there is a lot of implementation details that are not obvious. For example, ProgramInfo has owned vectors, and it is a part of ProgramHandle now. Including it in the batch would restrict user's ability to clone it where needed (in order, to say, store it in a render queue of sorts). Instead, we could have a context with all the meshes and programs that user has. But his has several issues on its own: ergonomics may suffer, complexity increases, safety can't be guaranteed.

If we had associated constants, we would be able to use Vec-kind types on a fixed memory, thus retaining the POD type. But even then, we'd be forced into limiting the number of uniforms/buffers/attributes/etc, which is also something that would be nice to avoid...

Impact on the API

  • (for HeavyBatch only) the draw state and the slice will be created automatically and exposed to the user as public members
  • accepting a batch would allow much simpler interfaces for future variations of draw calls (namely, instancing and transform feedback).
  • all errors are detected on batch creation, which makes the drawing code error-less

The new draw call may look like that:

renderer.draw(&batch, &frame);

Concept details

Needs:

  • light-weight, POD
  • self-contained, safe

Variants:

  1. Include ProgramInfo (pod?)
    1. Make it POD (how?)
    2. Clone instead (no pod)
  2. Share ProgramInfo (no safe?)
    • context is required
    • also share the mesh

Prototype code

/// Heavy Batch - self-contained
pub struct HeavyBatch<L, T> {
    mesh: mesh::Mesh,
    mesh_link: (),  //TODO
    pub slice: mesh::Slice,
    program: device::ProgramHandle,
    param: T,
    param_link: L,
    pub state: state::DrawState,
}

/// Light Batch - copyable and smaller
pub struct LightBatch<L, T> {
    mesh_id: uint,
    mesh_link: (),  //TODO
    pub slice: mesh::Slice,
    program_id: uint,
    param: T,
    param_link: L,
    state_id: uint,
}

/// Factory of light batches
pub struct Context {
    meshes: Vec<mesh::Mesh>,
    programs: Vec<device::ProgramHandle>,
    states: Vec<state::DrawState>,
}

Here is a more complete implementation by @sectopod for your reference.

@kvark
Copy link
Member Author

kvark commented Aug 25, 2014

Summoning @cmr and @csherratt.

@ghost
Copy link

ghost commented Aug 25, 2014

These would have utility for doing things like drawcall sorting. at least in the case of a LightBatch most of the key pieces of data that you would want to sort on (that gfx cares about, a user would probably want to add things like distance to camera ect) ((we might need to have someway to expose ordering w/o exposing data)).

I like the looks of the LightBatch, not sure if this is an either/or question.

I have some questions about lifetimes. Since a light batch is all based on indexes, its always possible for the data they are indexing to move under them. For example, I create 10 batches, delete there mesh they reference and then feed the batches as draw calls. The batches are all now broken...

@kvark
Copy link
Member Author

kvark commented Aug 25, 2014

@csherratt

These would have utility for doing things like drawcall sorting

Yes, it would definitely simplify higher-level code that manages render queues and culls/sorts batches (presumably, in a separate library).

I like the looks of the LightBatch, not sure if this is an either/or question.

Yes, part of the question is whether we want to have HeavyBatch, or LightBatch + Context, or both by having a Batch trait implemented for them. I'm leaning towards the latter myself, so that we'll have something while still figuring out how to do it right.

Since a light batch is all based on indexes, its always possible for the data they are indexing to move under them.

Yes, this is definitely something to worry about... Not only the context can be modified, but the user can also try using a batch with a wrong context. Of course, we can catch most of these errors at runtime by tracking generations and having some unique data shared between a context and batches that rely on it.

Basically, heavy batches are self-contained (thus, safe), but they are next to impossible to have light-weight (POD). And if we have render queues, these will need to copy the batches, so POD is really needed. Light batches, on the other hand, are POD, but they rely on user Context at all times...

@kvark
Copy link
Member Author

kvark commented Aug 25, 2014

Also please note that the batch concept replaces the current Program type. Thus, #[shader_param] will typedef a batch, and if we want to have both types, we'll need to decide which is the standard one (to be typedef-ed).

@brendanzab
Copy link
Contributor

Ah, that is useful info to know.

@kvark
Copy link
Member Author

kvark commented Aug 25, 2014

John Carmack:

Focused, hard work is the real key to success. Keep your eyes on the goal, and just keep taking the next step towards completing it. If you aren't sure which way to do something, do it both ways and see which works better.

Well I guess that answers my question, for the most part ;)

@brendanzab
Copy link
Contributor

Perhaps we could just try using Vecs initially, then iterate on that.

@brendanzab
Copy link
Contributor

I would also like to see a concrete example of what the API would look like in use. @sectopod is working on that?

@sectopod
Copy link
Contributor

yo! Kael reports on duty! I wanted to get a feel on how you develop things here and design the architecture, so volunteered to implement batches. By using @kvark 's prototype code, I managed to get a basic implementation of LightBatch in place, including the Renderer::draw_batch function. Couldn't go any further because it looks like the macros need to be changed... and the exact batch type is unknown for the user (IIRC). Will try to get more tonight.

@brendanzab
Copy link
Contributor

Maybe create a new PR with a comment "Closes #282" and "[WIP]" in the title.

@sectopod sectopod mentioned this issue Aug 26, 2014
adamnemecek pushed a commit to adamnemecek/gfx that referenced this issue Apr 1, 2021
282: Fix dynamic stencil values r=grovesNL a=kvark

Fixes gfx-rs#279

Co-authored-by: Dzmitry Malyshau <kvarkus@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants