Cleanup: Nodes: Simplify drawing function #112860

Lukas Tönne · 2023-09-25T16:51:48+02:00

Lukas Tönne commented

2023-09-25 16:51:48 +02:00

The node drawing function currently uses a stack-based traversal of
interface items. This is hard to read and for this purpose a recursive
function is easier to understand.

The node drawing function currently uses a stack-based traversal of interface items. This is hard to read and for this purpose a recursive function is easier to understand.

Lukas Tönne added 2 commits 2023-09-25 16:52:01 +02:00

537b398dfe Node drawing simplification: Use a recursive function for panel drawing.

68d35d5f4b Renamed function and added comment.

Lukas Tönne commented

2023-09-25 16:52:51 +02:00

This is in preparation for solving #112588 which would add some more complexity to the drawing code.

Lukas Tönne requested review from Jacques Lucke 2023-09-25 16:53:00 +02:00

Lukas Tönne added this to the Nodes & Physics project 2023-09-25 16:53:05 +02:00

Lukas Tönne changed title from ~~Nodes: Simplified drawing function.~~ to WIP: Nodes: Simplified drawing function.

2023-09-25 17:51:17 +02:00

Lukas Tönne added 2 commits 2023-09-25 18:01:30 +02:00

0f36037e85 Fix panel iteration, don't consume item before checking loop condition.

78922b7bb4 Fix panel finalization, only do this once (misplaced brackets).

Lukas Tönne changed title from ~~WIP: Nodes: Simplified drawing function.~~ to Nodes: Simplified drawing function.

2023-09-25 18:01:46 +02:00

Hans Goudey changed title from ~~Nodes: Simplified drawing function.~~ to Nodes: Simplify drawing function

2023-09-25 19:14:51 +02:00

Hans Goudey changed title from ~~Nodes: Simplify drawing function~~ to Cleanup: Nodes: Simplify drawing function

2023-09-25 19:14:59 +02:00

Hans Goudey reviewed 2023-09-25 19:17:08 +02:00

source/blender/editors/space_node/node_draw.cc Outdated

						
				@ -672,0 +641,4 @@

				  /* Recursive function that adds the expected number of items in a panel and advances the

				   * iterator. */

				  std::function<void(int, bool, const char *, bke::bNodePanelRuntime *)> add_panel_items_recursive;

Hans Goudey commented

2023-09-25 19:16:02 +02:00

Could this be a lambda rather than std::function?

const auto add_panel_items_recursive = [&](...) { }

Could this be a lambda rather than `std::function`? `const auto add_panel_items_recursive = [&](...) { }`

Iliya Katushenock commented

2023-09-25 19:19:19 +02:00

This is recursion lambda...

Hans Goudey commented

2023-09-25 19:25:18 +02:00

Huh, I see, it can't call itself. Seems like it should just be a separate function maybe?

👍 1

Lukas Tönne commented

2023-09-26 08:23:39 +02:00

Reason it's a std::function:

Needs a "forward declaration" to be able to do recursion.
Accesses a bunch of state variables that need to be carried along, either as a function argument or as functor members.

I wanted to avoid sweeping changes, the std::function was the smallest change that would work.

Reason it's a std::function: 1. Needs a "forward declaration" to be able to do recursion. 2. Accesses a bunch of state variables that need to be carried along, either as a function argument or as functor members. I wanted to avoid sweeping changes, the std::function was the smallest change that would work.

Lukas Tönne commented

2023-09-26 14:36:57 +02:00

Changed it to free functions with a state argument that contains all the iterator variables. Not necessarily less code but maybe slightly easier to understand?

👍 1

Lukas Tönne added 1 commit 2023-09-26 14:35:38 +02:00

21a5a9c872 Made free functions for updating node item visibility and location.

Jacques Lucke approved these changes 2023-09-27 11:44:33 +02:00

Jacques Lucke left a comment

LGTM, using recursion here instead of stack seems to be easier indeed.

Lukas Tönne merged commit 2f1b8f59e3 into main

2023-09-27 11:48:48 +02:00

Lukas Tönne referenced this issue from a commit

2023-09-27 11:48:48 +02:00

Cleanup: Nodes: Simplify drawing function

Lukas Tönne deleted branch recursive-panel-drawing