Page MenuHome

Fix T92080: Background of Node editors appear brighter than before
ClosedPublic

Authored by Dalai Felinto (dfelinto) on Oct 11 2021, 9:34 AM.

Details

Summary

In the original code depth=0 meant that there was no parents. But with
BLI_listbase_count we have depth 1 in those cases.

Diff Detail

Repository
rB Blender

Event Timeline

Dalai Felinto (dfelinto) requested review of this revision.Oct 11 2021, 9:34 AM
Dalai Felinto (dfelinto) created this revision.

Handle cases when the list is empty

Sergey Sharybin (sergey) requested changes to this revision.Oct 11 2021, 9:56 AM
Sergey Sharybin (sergey) added inline comments.
source/blender/editors/space_node/node_draw.cc
2187–2192

Put an explicit explanation that we want to ignore the first element of the path (as it is the top-most tree which does not need tinting), and that we don't want to go too far in the list.

The name num_parents is a bit ambiguous. I.e. how many parents you expect to be in the A -> B list? Technically there is only one parent, as the B has no children. But the code will return 2.

clamped_tree_path_length is what denotes the content of the variable the best.

2188

Avoid macro whenever possible!

This revision now requires changes to proceed.Oct 11 2021, 9:56 AM

From review: avoid macros, naming and comments

Dalai Felinto (dfelinto) marked 2 inline comments as done.Oct 11 2021, 10:00 AM

Code looks fine, and logic is sound! (some minor tweak to make it clear max_depth does not clamp the depth variable, can just rename right before commit)

In the middle of doing some other tests here, so couldn't verify by applying as a patch and so on.

source/blender/editors/space_node/node_draw.cc
2184
This revision is now accepted and ready to land.Oct 11 2021, 10:02 AM

Thanks for the review Sergey :)