DagerD/blender
1
1
Fork 0
forked from blender/blender
Code Pull Requests Activity

Implement type conversion for NodeItem #9

Merged
Bogdan Nagirniak merged 12 commits from BogdanNagirniak/blender:matx-nodeitem-type into matx-export-material 2023-09-05 12:03:24 +02:00
Conversation 0 Commits 12 Files Changed 8 +621 -461
Bogdan Nagirniak commented 2023-09-05 01:06:25 +02:00
Collaborator
Copy Link

Purpose

NodeItem requires convert() method for node type conversion. This will simplify arithmetic and other methods.

Technical steps

  1. Moved NodeItem type and conditional type in if_else from string to enum values.
  2. Implemented NodeItem::convert() method.
  3. Simplified set_input methods.
  4. Refactor: rearranged functions in NodeItem to satisfy blender code style.
  5. Fixed review request about subsurface in BSDFPrincipled node.
### Purpose `NodeItem` requires `convert()` method for node type conversion. This will simplify arithmetic and other methods. ### Technical steps 1. Moved `NodeItem` type` and conditional type` in `if_else` from `string` to `enum` values. 2. Implemented `NodeItem::convert()` method. 3. Simplified `set_input` methods. 4. Refactor: rearranged functions in `NodeItem` to satisfy blender code style. 5. Fixed review request about subsurface in `BSDFPrincipled` node.
Bogdan Nagirniak added 9 commits 2023-09-05 01:07:09 +02:00 
Removed unused code, Simplified some functions.
Bogdan Nagirniak requested review from Brian Savery (AMD) 2023-09-05 01:10:37 +02:00
Bogdan Nagirniak requested review from Vasyl Pidhirskyi 2023-09-05 01:10:38 +02:00
Georgiy Markelov was assigned by Bogdan Nagirniak 2023-09-05 07:30:03 +02:00
Bogdan Nagirniak self-assigned this 2023-09-05 07:30:04 +02:00
Bogdan Nagirniak added 1 commit 2023-09-05 08:47:04 +02:00
Georgiy Markelov requested changes 2023-09-05 10:42:46 +02:00
Georgiy Markelov left a comment
Owner
Copy Link

Comments in code. Also haven't found Working with Type::integer (in NodeItem::is_arithmetic, NodeItem::convert etc) . Shouldn't we add it?

Comments in code. Also haven't found Working with `Type::integer` (in `NodeItem::is_arithmetic`, `NodeItem::convert` etc) . Shouldn't we add it?
source/blender/nodes/shader/materialx/nodes/node_item.cc
@ -425,2 +435,2 @@
return value ? value->getTypeString() : node->getType();
}
NodeItem res = empty();
if (type() != Type::Float || other.type() != Type::Float) {
Georgiy Markelov commented 2023-09-05 10:41:06 +02:00
Owner
Copy Link

Should Integer be here?

Should `Integer` be here?
Bogdan Nagirniak commented 2023-09-05 11:46:24 +02:00
Author
Collaborator
Copy Link

Discussed: let it be here

Discussed: let it be here
BogdanNagirniak marked this conversation as resolved
source/blender/nodes/shader/materialx/nodes/node_item.h Outdated
@ -12,0 +22,4 @@
Color3,
Color4
};
enum class IfType { Less = 0, LessEq, Eq, GreaterEq, Greater, NotEq };
Georgiy Markelov commented 2023-09-05 10:42:14 +02:00
Owner
Copy Link

IfType -> ComparisonType

IfType -> ComparisonType
BogdanNagirniak marked this conversation as resolved
Georgiy Markelov requested changes 2023-09-05 10:56:05 +02:00
source/blender/nodes/shader/materialx/nodes/node_item.cc Outdated
@ -378,2 +239,3 @@
NodeItem NodeItem::convert(Type to_type) const
{
std::string mx_type = type();
Type tp = type();
Georgiy Markelov commented 2023-09-05 10:46:25 +02:00
Owner
Copy Link

tp -> from_type

tp -> from_type
BogdanNagirniak marked this conversation as resolved
source/blender/nodes/shader/materialx/nodes/node_item.cc Outdated
@ -587,3 +612,1 @@
else {
return false;
}
return tp >= Type::Float;
Georgiy Markelov commented 2023-09-05 10:52:28 +02:00
Owner
Copy Link

Type::Float -> Type::Integer

Type::Float -> Type::Integer
Bogdan Nagirniak commented 2023-09-05 11:48:02 +02:00
Author
Collaborator
Copy Link

Integer isn't used in arithmetic operations

Integer isn't used in arithmetic operations
BogdanNagirniak marked this conversation as resolved
source/blender/nodes/shader/materialx/nodes/node_item.h Outdated
@ -12,0 +12,4 @@
public:
enum class Type {
Empty = 0,
Other,
Georgiy Markelov commented 2023-09-05 10:48:58 +02:00
Owner
Copy Link

Add comment what is Other

Add comment what is `Other`
BogdanNagirniak marked this conversation as resolved
source/blender/nodes/shader/materialx/nodes/node_item.h
@ -34,3 +38,1 @@
const std::string &output_name = "");
void add_output(const std::string &name, const std::string &mx_type);
/* Operators */
Georgiy Markelov commented 2023-09-05 10:49:33 +02:00
Owner
Copy Link

Suggest add #pragma region for such places

Suggest add `#pragma region` for such places
Bogdan Nagirniak commented 2023-09-05 11:48:33 +02:00
Author
Collaborator
Copy Link

It is not commonly used in Blender code

It is not commonly used in Blender code
BogdanNagirniak marked this conversation as resolved
Bogdan Nagirniak added 1 commit 2023-09-05 11:11:40 +02:00
Bogdan Nagirniak added 1 commit 2023-09-05 11:38:42 +02:00
Georgiy Markelov approved these changes 2023-09-05 12:01:57 +02:00
Georgiy Markelov left a comment
Owner
Copy Link

Works fine.

Works fine.
Bogdan Nagirniak merged commit 127fff6108 into matx-export-material 2023-09-05 12:03:24 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
Brian Savery (AMD)
Vasyl Pidhirskyi
Georgiy Markelov
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
Georgiy Markelov
Bogdan Nagirniak
2 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: DagerD/blender#9
No description provided.
Delete Branch "BogdanNagirniak/blender:matx-nodeitem-type"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?