FosterFramework / Foster

A small C# game framework
MIT License
435 stars 38 forks source link

Shader Material Concept for Batcher #21

Closed NoelFB closed 11 months ago

NoelFB commented 11 months ago

This is mostly intended for the Batcher, but right now Pushing/Popping Shaders is a bit weird if you intend to modify parameters, due to the actual draw calls being deferred to later.

For example, in this scenario both rectangles are drawn with the 2nd parameter assignment of 25.0f:

custom["Wavy"].Set(10.0f);
batch.PushShader(custom);
batch.Rect(new Rect(0, 0, 32, 32), Color.Red);
batch.Pop();

// ...

custom["Wavy"].Set(25.0f);
batch.PushShader(custom);
batch.Rect(new Rect(32, 32, 32, 32), Color.Red);
batch.Pop();

// ...
batch.Render();

I don't think this is very intuitive, and have run into it multiple times in my own use case. Thus, I think the idea of a "Material" which wraps a Shader & Parameter values might make a lot of sense. When the Material is actually used it applies any modified parameters to the current Shader.

I see a few ways for how this could be implemented:

MrBrixican commented 11 months ago

Hmm, there's probably not a great solution to this. However, here's my idea:

batch.PushShader(custom)
    .With("Wavy", 10f);
batch.Rect(new Rect(0, 0, 32, 32), Color.Red);
batch.PopShader();

It uses a fluent interface/method chaining. PushShader() should return a struct that can be chained to set the uniforms. The uniform values will be stored/pooled in the Batcher itself through some means. The struct will merely be a reference to the Batcher and maybe some version id/index to ensure that the user is not storing the struct and using it after the initial call (this is just plain user error, though).

It's obviously not a perfect solution since any uniforms set on the shader will not be overwritten if a With() call is not used (unless we use automatic setting like you described in your first solution). Additionally, I'm not really sure what should happen on PopShader() (i.e. should values be reset to what they were).

The main thing I want to avoid is Shader[].Set() since ideally we don't want any graphics calls before Batcher.Render().

NoelFB commented 11 months ago

Yeah I think I like this direction, my only concern (and maybe this is a non-issue?) is programmatically assigning values. Like if you're batching stuff from data, you can't do something like:

foreach (var (key, value) in assignments)
    batch.SetShaderParameter(key, value);

Although I suppose if the struct returned with batch.PushShader is valid until the next call you could do

var it = batch.PushShader(custom);
foreach (var (key, value) in assignments)
    it.With(key, value);

Will think about this a bit! I could even just go with the first example (batch.SetShaderParameter). That might be good enough.

NoelFB commented 11 months ago

Another option is to:

(this is actually what the old foster did, if I remember correctly)

NoelFB commented 11 months ago

This is now fixed as of b8ea1b5. Opted to add a Material class that is easy to Clone, and the Batcher holds an internal pool of Materials it reuses. Did require a breaking change if you use Shaders though.