vvvv / VL.StandardLibs

A collection of standard libraries for vvvv including VL.Stride, VL.Skia, VL.ImGui, msgpack.org[VL]
https://visualprogramming.net
GNU Lesser General Public License v3.0
36 stars 14 forks source link

IComputeNode<T> PR on Stride repo #226

Open gregsn opened 3 years ago

gregsn commented 3 years ago

our GPU<T> should be a forward of IComputeNode<T>, an interface that should get added to Stride.

two failing ideas for how to relate the new IComputeNode<T> with IComputeScalar:

So another way to do it is to inherit IComputeScalar from IComputeNode<float>.

The important part, however, is that at all the consuming locations - like MetalnessMap inside the metalness feature - would now accept a IComputeNode<float> instead of IComputeScalar.

This would enable to connect

gregsn commented 3 years ago

https://github.com/stride3d/stride/commit/a89526cc910f8338a29acc58ef52a275a3ca37d6

tebjan commented 3 years ago

There are several reasons why we don't need that and even shouldn't do that. Please wait for my input on this.

gregsn commented 3 years ago

Will do

tebjan commented 3 years ago

In short, since our ShaderFX nodes contain everything that is available as IComputeNode implementation in stride, there is no real gain in having these as additional nodes. it would rather lead to confusion and doubled nodes, which this PR did resolve: https://github.com/vvvv/VL.Stride/pull/347

so we have a generalization of the stride shader graph that is more general and more modular. we basically jumped over the stride level of abstraction. it can be seen as in inspiration for our ShaderFX nodes.

we only miss one node that is mentioned here: https://github.com/vvvv/VL.StandardLibs/issues/378 it was actually the first ShaderFX node in the early research phase, but got lost somewhere. should be easy to find.

our GPU<T> should be a forward of IComputeNode<T>

Our GPU<T> has to be a bit more strict. the forward is actually the GetVar<T> class, which reads a stream variable. this is necessary to allow multiple connections from an output. this extra restriction is not really necessary in the stride UI and it would be a bit much to force that into their repo.

but we could think about common base interfaces... we currently have a few more, e.g. ICompureValue<T> and such things.

if needed, compatibility with the stride implementation is also there via converter shaders that simply pass the value through.

gregsn commented 3 years ago

Ok thanks Tebjan. I now understand better what extra logic has been put into the ShaderFX side of things. The GetVar<T> is indeed a very important part that I overlooked here. However, the proposed changes would not mean that we'd loose any of that.

The spirit for this change was just to find an interface - one interface - that works for everybody making the whole architecture easier to grasp. So it was more about elegance, a simplification attempt, somewhat thought of a cleanup commit. So that's why I thought this basic interface must reside in Stride.

I gave this some thought and I would like to just put down some thoughts that popped up on the way:

We all know those pattern around monadic interfaces:

Regarding IComputeNode<T>: It's not a real monadic construct, since it lacks the bind, but it already has a return, so I think the comparison isn't that off.

Deep down and maybe not seen by the user, I would love if the basic architecture would be similar to the architectures above.

For me it would be important to understand where I can get my hands dirty. If I understand that there is an interface that I can implement and interact with, without breaking the rest of the system: that would be awesome and could potentially lead to more developments than when everything is tied to certain implementations(?)