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

little abstraction layer: IDX11ShaderVariableManager #319

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gregsn
Copy link
Contributor

@gregsn gregsn commented Jun 2, 2017

hi vux!

i have some few changes that would allow the community to play with shader graphs without the need of forking your repo. The main idea of the changes is just to abstract over the way how variables get routed to the shader.

abstracted over DX11ShaderVariableManager via IDX11ShaderVariableManager

  • DX11ShaderNode now can be used as a base class for a shader graph sink.
  • adjusted DX11ShaderVariableManager so that it also can serve as a base implementation of IDX11ShaderVariableManager.
  • removed PinName from IShaderPin as it felt unused (at least in this solution) please recheck

…ger.

* DX11ShaderNode now can be used as a base class for a shader graph sink.
* adjusted DX11ShaderVariableManager so that it also can serve as a base implementation of IDX11ShaderVariableManager.
* removed PinName from IShaderPin as it felt unused (at least in this solution) please recheck
@gregsn
Copy link
Contributor Author

gregsn commented Jun 3, 2017

The changes shouldn't add/remove any functionality on your side. They just help to contribute to your class family.

@mrvux
Copy link
Owner

mrvux commented Jun 6, 2017

Not sure about that one, since the main shader nodes are critical to the functioning of the whole pack, and those classes should almost be considered as internals (I only did not so so as the only need to be accessible by the factory library and I dislike internalsvisibleto).

I actually wanted to move toward splitting that project in 2 to more clearly emphasise the intent (VVVV.DX11.Lib and VVVV.DX11.Lib.Internals), but decided to eventually do it post node.

Extracting interface does modify functionality a lot, since now I have to take into account that a lot of different implementations (potentially broken) can be added into shader node.

Also, it means that suddenly the whole interface (and shader node) becomes frozen, which I believe will impact the whole system negatively (less room for improvements without breaking interface contract)

Adding the whole part of shaderVariable as protected also sounds horrible, means now the class needs to take into account that any of their member can be modified separately, and without any consistency.

About pin name, yes, that's a leftover from last changes, that can be removed (as well as in implementations)

@gregsn
Copy link
Contributor Author

gregsn commented Jun 6, 2017

ok. no problem. thanks for having a look

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

Successfully merging this pull request may close these issues.

2 participants