Rewrite some compositing nodes to using declarations #108728

New Issue

Open

opened 2023-06-07 23:57:12 +02:00 by Iliya Katushenock · 21 comments

Iliya Katushenock commented

2023-06-07 23:57:12 +02:00

Member

Dynamic declarations for some compositor nodes.

Goals:

Stop using operators to change node sockets directly. Delegate this to the node declaration system. This have a lot of benefits:

Management of topology updates, drawing.
Part of node updatings have to be exist only as part of main node trees update class.
Handling old socket data and links from declaration code automatically.
Using a newer, maintained API.

Way to do that:

For CMP_NODE_OUTPUT_FILE node:
1.1. Added in node.storage stucture linked list to contain metadata of each output socket.
1.2. Added declare_dynamic function for generate sockets based on metadata. See blender::nodes::node_group_declare_dynamic as example implementation.
1.3. Change node property operators to change only metadata.
All bNodeSocketTemplate replace by static declarations. See ed9b21098d as example.

It is best to do this separately for each node in an individual PR.
Nodes to change:

CMP_NODE_OUTPUT_FILE.
CMP_NODE_R_LAYERS #114629.
CMP_NODE_CRYPTOMATTE ae739a6173.
- CMP_NODE_CRYPTOMATTE_LEGACY ?
CMP_NODE_SWITCH_VIEW 5dd9e57878.

Dynamic declarations for some compositor nodes. ### Goals: Stop using operators to change node sockets directly. Delegate this to the node declaration system. This have a lot of benefits: 1. Management of topology updates, drawing. 2. Part of node updatings have to be exist only as part of main node trees update class. 3. Handling old socket data and links from declaration code automatically. 4. Using a newer, maintained API. ### Way to do that: 1. For `CMP_NODE_OUTPUT_FILE` node: 1.1. Added in node.storage stucture linked list to contain metadata of each output socket. 1.2. Added `declare_dynamic` function for generate sockets based on metadata. See `blender::nodes::node_group_declare_dynamic` as example implementation. 1.3. Change node property operators to change only metadata. 2. All `bNodeSocketTemplate` replace by static declarations. See https://projects.blender.org/blender/blender/commit/ed9b21098dd27bf9364397357f89b4c2648f40c2 as example. It is best to do this separately for each node in an individual PR. Nodes to change: - [ ] `CMP_NODE_OUTPUT_FILE`. - [ ] `CMP_NODE_R_LAYERS` #114629. - [X] `CMP_NODE_CRYPTOMATTE` ae739a6173a5a61a7db8cc64d924ea6e0ebadcc7. - [ ] `CMP_NODE_CRYPTOMATTE_LEGACY` ? - [x] `CMP_NODE_SWITCH_VIEW` 5dd9e578780f7dcc17134d20cac5748faf768eb2.

Iliya Katushenock added the

labels 2023-06-07 23:57:12 +02:00

Iliya Katushenock added this to the Nodes & Physics project 2023-06-07 23:57:14 +02:00

Iliya Katushenock referenced this issue

2023-06-08 00:01:32 +02:00

Correct link is shown as invalid #108719

Iliya Katushenock referenced this issue

2023-06-08 00:02:38 +02:00

Remove unavailable sockets and links #104126

Iliya Katushenock added the

Good First Issue

label 2023-06-12 21:26:15 +02:00

Weikang-Qiu commented

2023-06-13 18:42:40 +02:00

Contributor

Can I take this issue?

Can I take this issue?

Iliya Katushenock commented

2023-06-13 18:44:53 +02:00

Author

Member

Yes, your welcome!

Yes, your welcome!

Weikang-Qiu commented

2023-06-22 21:45:16 +02:00

Contributor

I noticed that for CMP_NODE_R_LAYERS, there are also some operations for adding/removing sockets, does your 1 also applies for CMP_NODE_R_LAYERS besides CMP_NODE_OUTPUT_FILE?

I noticed that for `CMP_NODE_R_LAYERS`, there are also some operations for adding/removing sockets, does your 1 also applies for `CMP_NODE_R_LAYERS` besides `CMP_NODE_OUTPUT_FILE`?

Iliya Katushenock commented

2023-06-22 22:06:06 +02:00

Author

Member

As i know, CMP_NODE_R_LAYERS should display only sockets, declared by scene layers.
So, there is no operators (?) but only dynamic declaration should be added.
Tell me if I misunderstood something.

As i know, `CMP_NODE_R_LAYERS` should display only sockets, declared by scene layers. So, there is no operators (?) but only dynamic declaration should be added. Tell me if I misunderstood something.

Weikang-Qiu commented

2023-06-23 20:19:25 +02:00

Contributor

All bNodeSocketTemplate replace by static declarations. See ed9b21098d as example.

I thought you were saying static declarations instead o dynamic declarations for CMP_NODE_R_LAYERS.
So for your 2, we may need dynamic declarations?

Is your 1 an example and it applies to all of the 4 nodes? or 1 only applies to CMP_NODE_OUTPUT_FILE?

> All bNodeSocketTemplate replace by static declarations. See ed9b21098d as example. I thought you were saying static declarations instead o dynamic declarations for `CMP_NODE_R_LAYERS`. So for your 2, we may need dynamic declarations? Is your 1 an example and it applies to all of the 4 nodes? or 1 only applies to `CMP_NODE_OUTPUT_FILE`?

Iliya Katushenock commented

2023-06-23 20:21:33 +02:00

Author

Member

Yes, where necessary, declarations should be made dynamic.

Yes, where necessary, declarations should be made dynamic.

Weikang-Qiu commented

2023-07-09 04:25:22 +02:00

Contributor

I rewrote CMP_NODE_SWITCH_VIEW node. But I have no idea what new features are you expecting so I don't know how to test it. Do you have any ideas?

I rewrote `CMP_NODE_SWITCH_VIEW` node. But I have no idea what new features are you expecting so I don't know how to test it. Do you have any ideas?

Iliya Katushenock commented

2023-07-09 10:41:15 +02:00

Author

Member

It should not introduce new features. This is an internal refactoring that should not create behavioral changes.
There is a varniat, in theory, you should fix this bug #108719, so you can check it out.
But otherwise, probably the only option is to check the test runs (https://wiki.blender.org/wiki/Tools/Tests/Setup). It is also easy to manually check various cases of work.

It should not introduce new features. This is an internal refactoring that should not create behavioral changes. There is a varniat, in theory, you should fix this bug #108719, so you can check it out. But otherwise, probably the only option is to check the test runs (https://wiki.blender.org/wiki/Tools/Tests/Setup). It is also easy to manually check various cases of work.

~~Weikang-Qiu referenced this issue 2023-07-09 15:58:35 +02:00~~

Switch View Node does not get updated when clicking `update views` button #109866

Weikang-Qiu referenced this issue

2023-07-13 04:43:29 +02:00

Cleanup: Refactor Switch View node to use dynamic declarations #110042

Weikang-Qiu commented

2023-07-13 04:48:21 +02:00

Contributor

I started from an easier one, i.e. CMP_NODE_SWITCH_VIEW

I started from an easier one, i.e. `CMP_NODE_SWITCH_VIEW`

Iliya Katushenock referenced this issue

2023-07-14 11:16:15 +02:00

Cleanup: Refactor Switch View node to use dynamic declarations #110042

Artur-Vieira commented

2023-07-31 00:22:11 +02:00

Hi @mod_moder , could I work on this issue as a first good issue? If it's available. Thank you.

Hi @mod_moder , could I work on this issue as a first good issue? If it's available. Thank you.

Iliya Katushenock commented

2023-07-31 00:24:53 +02:00

Author

Member

@Artur-Vieira Hi, yes, welcome

@Artur-Vieira Hi, yes, welcome

Peter Tretyakov commented

2023-08-07 19:47:17 +02:00

Hi @mod_moder! I would love to take one of these nodes as a first issue. Is it available? I see that @Weikang-Qiu already made CMP_NODE_SWITCH_VIEW and @Artur-Vieira also want to contribute here so @Artur-Vieira what node you've already taken if any? Also @Weikang-Qiu do you continue working on another node from this list so I can take the other one?

Hi @mod_moder! I would love to take one of these nodes as a first issue. Is it available? I see that @Weikang-Qiu already made `CMP_NODE_SWITCH_VIEW` and @Artur-Vieira also want to contribute here so @Artur-Vieira what node you've already taken if any? Also @Weikang-Qiu do you continue working on another node from this list so I can take the other one?

Weikang-Qiu commented

2023-08-08 09:47:25 +02:00

Contributor

Hi @petertretyakov . Yeah I am planning to take another one but I haven't started yet. you can pick the one you like first.

Hi @petertretyakov . Yeah I am planning to take another one but I haven't started yet. you can pick the one you like first.

Weikang-Qiu commented

2023-08-12 08:07:36 +02:00

Contributor

Also I am wondering what node did you choose @Artur-Vieira ? Since I am beginning to refractor the next one.

Also I am wondering what node did you choose @Artur-Vieira ? Since I am beginning to refractor the next one.

Peter Tretyakov commented

2023-08-12 14:10:10 +02:00

@Weikang-Qiu which one will you take?

@Weikang-Qiu which one will you take?

Artur-Vieira commented

2023-08-12 19:04:37 +02:00

@Weikang-Qiu feel free to choose first, I am working on reviewing the code repository.

@Weikang-Qiu feel free to choose first, I am working on reviewing the code repository.

Peter Tretyakov commented

2023-08-12 19:55:14 +02:00

I'll take CMP_NODE_OUTPUT_FILE if nobody has taken it yet. @Weikang-Qiu @Artur-Vieira FYI

I'll take `CMP_NODE_OUTPUT_FILE` if nobody has taken it yet. @Weikang-Qiu @Artur-Vieira FYI

Iliya Katushenock referenced this issue

2023-08-16 13:37:28 +02:00

Regression: Blender crashes when turning on a pass that has equally named AOV pass already #111166

Iliya Katushenock commented

2023-08-16 13:42:54 +02:00

Author

Member

Rewriting of CMP_NODE_R_LAYERS node might not be a bad decision for any of you right now.

Rewriting of `CMP_NODE_R_LAYERS` node might not be a bad decision for any of you right now.

Peter Tretyakov commented

2023-08-16 14:04:45 +02:00

I spent several hours in last two days with output file node and looks like it would be better to start with another one, so I'll take render layers node and then return to output file if nobody has taken it yet. Also if somebody want to try with output file node it is free for now ))

I spent several hours in last two days with output file node and looks like it would be better to start with another one, so I'll take render layers node and then return to output file if nobody has taken it yet. Also if somebody want to try with output file node it is free for now ))

Iliya Katushenock referenced this issue

2023-08-17 14:17:36 +02:00

Crash on calling Node.inputs.new(...) #111184

Hans Goudey referenced this issue from a commit

2023-08-23 16:35:44 +02:00

Cleanup: Refactor Switch View node to use dynamic declarations

Iliya Katushenock referenced this issue

2023-11-08 15:12:13 +01:00

WIP: Refractor: Compositor: Render layer node to dynamic declaration #114629

Iliya Katushenock referenced this issue

2023-11-08 20:32:49 +01:00

WIP: Refractor: Compositor: Render layer node to dynamic declaration #114629

Philipp Oeser referenced this issue

2024-02-27 15:19:30 +01:00

output_node.inputs.new() crash blender #103824

Philipp Oeser referenced this issue

2024-02-27 15:56:59 +01:00

WIP: Fix #103824: CompositorNodeOutputFile inputs.new() crash #118807

Weikang-Qiu referenced this issue

2024-04-12 20:53:30 +02:00

File output node not showing up in search #120592

Weikang-Qiu commented

2024-04-13 20:49:46 +02:00

Contributor

Is the CMP_NODE_OUTPUT_FILE task taken by #118807?

Is the `CMP_NODE_OUTPUT_FILE` task taken by #118807?

Iliya Katushenock commented

2024-04-13 20:56:09 +02:00

Author

Member

Nope, #118807 is just fix of the bug. There is still might be other issues, and this refactor still should be done due to improvements listed in the description.

Nope, #118807 is just fix of the bug. There is still might be other issues, and this refactor still should be done due to improvements listed in the description.

Weikang-Qiu referenced this issue

2024-04-15 22:41:35 +02:00

Switch View Node is not updated after adding view #120680

Sign in to join this conversation.

No Label

Animation & Rigging

Asset Browser Project Overview

Automated Testing

Blender Asset Bundle

Dependency Graph

Development Management

EEVEE & Viewport

Images & Movies

Motion Tracking

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Render Pipeline

Sculpt, Paint & Texture

Video Sequencer

Virtual Reality

Blender 2.8 Project

Milestone 1: Basic, Local Asset Browser

Good First Issue

Animation & Rigging

Development Management

EEVEE & Viewport

Nodes & Physics

Pipeline, Assets & IO

Platforms, Builds & Tests

Render & Cycles

Sculpt, Paint & Texture

Needs Info from Developers

Needs Information from User

No Milestone

No project

No Assignees

4 Participants

Notifications

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#108728

No description provided.