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

Node Groups #391

Open
wants to merge 64 commits into
base: Development
Choose a base branch
from
Open

Node Groups #391

wants to merge 64 commits into from

Conversation

pragma37
Copy link
Member

@pragma37 pragma37 commented Sep 11, 2022

Initial Node Groups implementation.

Each shader type now has a corresponding group type (ie. Mesh and Mesh Group).
These groups can be used with the new Node Group node (Add Node > Other > Node Group).

Group graphs generate a function in a separate file that can be imported by the node trees that use them.
All node group parameters are private and their values are stored in the tree itself (this greatly simplifies the implementation).

@pragma37 pragma37 changed the title Node groups Node Groups Sep 11, 2022
@pragma37 pragma37 changed the base branch from node-ux to Development September 15, 2022 14:04
@Kolupsy
Copy link
Collaborator

Kolupsy commented Sep 16, 2022

I did not have time to look into the implementation of groups but one thing that can be made possible using groups is a loop node. UI-wise they are always tricky but with groups you would be able to load in a group as your loop target and set an iterations number. Passing the iteration count to the group would basically open up the entire workflow using loops that was only available through code before.

I was wondering if the design of the groups in malt would allow for something like this.
if it generates a callable function then maybe the wrapper loop node could output a loop block as its source code containing that function or something

@Kolupsy
Copy link
Collaborator

Kolupsy commented Sep 16, 2022

Also we should allow creating new group node trees from the Group Node UI as well as the ability to select certain nodes and compress them down to a node group

@pragma37
Copy link
Member Author

if it generates a callable function then maybe the wrapper loop node could output a loop block as its source code containing that function or something

Yep, that should be doable.
If you look into the MaltGroupNode class, it doesn't even override the get_source_code function, it compiles into a regular function call.
For a loop node, you just would need a group with an iteration count parameter.

Also we should allow creating new group node trees from the Group Node UI as well as the ability to select certain nodes and compress them down to a node group

Agree, but honestly, I'm somewhat tired of working on so much node/UI stuff lately.
I implemented the core group functionality because it requires a full understanding of the whole compilation process, but I would like to focus on rendering again.

@Kolupsy
Copy link
Collaborator

Kolupsy commented Sep 16, 2022

Agree, but honestly, I'm somewhat tired of working on so much node/UI stuff lately.

I can take care of that if you want. Cant do it right this second but I hope I find some time this week. (so much stuff going on now I will need to make myself a todo list :D)

For a loop node, you just would need a group with an iteration count parameter.

I think that should be enforceable. bpy.props.PointerPropery allows you to poll objects that you want to reference. Needs to be clear to the user as well but from a code-security standpoint this can be done easily.

I would like to focus on rendering again.

I feel you! Do you already have in mind what you want to tackle?

@pragma37
Copy link
Member Author

Do you already have in mind what you want to tackle?

A while ago, I implemented support for freestyle edges and geometry based line detection (like Freestyle/LineArt) for contours and creases.
It would be nice to finish and merge that.
I also want to improve shadow map filtering and add support for light probes and screen tracing.

There's plenty of other stuff I'd like to experiment with, but those are the main ones I think.

@dibli-goost
Copy link

dibli-goost commented Sep 26, 2022

Been testing this out for a good bit, converting old shaders to node groups for efficiency's sake. I'll probably make a longer discussion thread later about some stuff I'd like to see, but figured I should bring this up asap due to how negatively it can affect the workflow.

You can't have input properties and output properties with the same name, you always get a redefinition error when doing so. Which it makes sense, I'm guessing this is just how GLSL works? But some sort of fail safe is needed I think, people should be able to call their properties what they want. Perhaps on the frontend, Malt displays the properties as you type them, but in the code it adds Input or Output to the beginning of the property, so that there can't be conflicts there?

Or maybe at the very least Malt could entirely prevent you from adding duplicate property names? I was really scratching my head for awhile last night trying to figure out what I was doing wrong with the Node Groups but yeah it was just duplicate properties 😄

@pragma37
Copy link
Member Author

Fixed! Thanks for the testing.

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.

None yet

3 participants