blender
/
blender
137
426
Fork 613
Code Issues 6.3k Pull Requests 783 Projects 19 Wiki Activity

WIP: MaterialX: Implement Script node #113341

Draft
Georgiy Markelov wants to merge 5 commits from DagerD/blender:matx-add-script-node into main
pull from: DagerD/blender:matx-add-script-node
merge into: blender:main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
Conversation 1 Commits 5 Files Changed 1 +73 -57
1 changed files with 73 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 34c3bd420f - Show all commits

130
source/blender/nodes/shader/materialx/material.cc
View File

@ -2,6 +2,8 @@
*
* SPDX-License-Identifier: GPL-2.0-or-later */
#include <sstream>
#include <MaterialXFormat/XmlIo.h>
#include "material.h"
@ -60,23 +62,17 @@ class DefaultMaterialNodeParser : public NodeParser {
}
};
MaterialX::DocumentPtr export_to_materialx(Depsgraph *depsgraph,
Material *material,
ExportImageFunction export_image_fn)
static bool export_script_node(MaterialX::DocumentPtr doc,
Depsgraph *depsgraph,
Material *material)
{
CLOG_INFO(LOG_MATERIALX_SHADER, 0, "Material: %s", material->id.name);
MaterialX::DocumentPtr doc = MaterialX::createDocument();
bool script_found = false;
if (material->use_nodes) {
material->nodetree->ensure_topology_cache();
Main *bmain = DEG_get_bmain(depsgraph);
/* Look for first script node with assigned file.
* If it's found, we load content and ignore everything other. */
LISTBASE_FOREACH (bNode *, node, &material->nodetree->nodes) {
if (node->typeinfo->type == SH_NODE_SCRIPT) {
NodeShaderScript *script = static_cast<NodeShaderScript *>(node->storage);
if (script->mode == NODE_SCRIPT_EXTERNAL && script->filepath[0] != '\0') {
Main *bmain = DEG_get_bmain(depsgraph);
LISTBASE_FOREACH (bNode *, node, &material->nodetree->nodes) {
if (node->typeinfo->type == SH_NODE_SCRIPT) {
NodeShaderScript *script = static_cast<NodeShaderScript *>(node->storage);
if (script->mode == NODE_SCRIPT_EXTERNAL) {
if (script->filepath[0] != '\0') {
DagerD marked this conversation as resolved Outdated
Bogdan Nagirniak commented 2023-10-06 20:08:17 +02:00
Outdated
Review
Copy Link

Use explicit STREQ(script->filepath, "")

Use explicit `STREQ(script->filepath, "")`
script_found = true;
char filepath[FILE_MAX];
DagerD marked this conversation as resolved Outdated
Bogdan Nagirniak commented 2023-10-06 14:00:45 +02:00
Outdated
Review
Copy Link

move this part with if to separate function

move this part with `if` to separate function
STRNCPY(filepath, script->filepath);
@ -98,53 +94,73 @@ MaterialX::DocumentPtr export_to_materialx(Depsgraph *depsgraph,
}
break;
DagerD marked this conversation as resolved Outdated
Bogdan Nagirniak commented 2023-10-06 20:58:08 +02:00
Outdated
Review
Copy Link

I think no need to use script_found just return true here

I think no need to use `script_found` just `return true` here
}
else if (script->mode == NODE_SCRIPT_INTERNAL) {
Text *text = reinterpret_cast<Text *>(node->id);
if (text) {
script_found = true;
std::string text_content;
LISTBASE_FOREACH (TextLine *, line, &text->lines) {
text_content.append(line->line);
}
try {
MaterialX::readFromXmlString(doc, text_content);
}
catch (MaterialX::ExceptionParseError) {
CLOG_WARN(LOG_MATERIALX_SHADER,
"Material: %s, Node: %s: parsing error",
material->id.name,
node->name);
}
break;
}
else if (script->mode == NODE_SCRIPT_INTERNAL) {
Text *text = reinterpret_cast<Text *>(node->id);
if (text) {
script_found = true;
std::stringstream text_content;
LISTBASE_FOREACH (TextLine *, line, &text->lines) {
text_content << line->line;
}
DagerD marked this conversation as resolved Outdated
Bogdan Nagirniak commented 2023-10-06 14:02:02 +02:00
Outdated
Review
Copy Link

Seem std::stringstream suits better here.

Seem `std::stringstream` suits better here.
}
else {
BLI_assert_unreachable();
try {
MaterialX::readFromXmlStream(doc, text_content);
}
catch (MaterialX::ExceptionParseError) {
CLOG_WARN(LOG_MATERIALX_SHADER,
"Material: %s, Node: %s: parsing error",
material->id.name,
node->name);
}
break;
}
}
else {
BLI_assert_unreachable();
}
}
}
return script_found;
}
bNode *output_node = ntreeShaderOutputNode(material->nodetree, SHD_OUTPUT_ALL);
if (output_node && !script_found) {
NodeParserData data = {doc.get(),
depsgraph,
material,
NodeItem::Type::Material,
nullptr,
NodeItem(doc.get()),
export_image_fn};
output_node->typeinfo->materialx_fn(&data, output_node, nullptr);
}
else {
DefaultMaterialNodeParser(doc.get(),
depsgraph,
material,
nullptr,
nullptr,
NodeItem::Type::Material,
nullptr,
export_image_fn)
.compute_error();
MaterialX::DocumentPtr export_to_materialx(Depsgraph *depsgraph,
Material *material,
ExportImageFunction export_image_fn)
DagerD marked this conversation as resolved Outdated
Bogdan Nagirniak commented 2023-10-06 14:06:26 +02:00
Outdated
Review
Copy Link

Should be

if (!script_found) {
....

Otherwise else block will beexecuted

Should be ``` if (!script_found) { .... ``` Otherwise `else` block will beexecuted
{
CLOG_INFO(LOG_MATERIALX_SHADER, 0, "Material: %s", material->id.name);
MaterialX::DocumentPtr doc = MaterialX::createDocument();
bool script_found = false;
if (material->use_nodes) {
material->nodetree->ensure_topology_cache();
/* Look for first script node with assigned file.
* If it's found, we load content and ignore everything other. */
script_found = export_script_node(doc, depsgraph, material);
if (!script_found) {
DagerD marked this conversation as resolved
Bogdan Nagirniak commented 2023-10-06 20:58:45 +02:00
Review
Copy Link

if (!export_script_node(doc, depsgraph, material)) {

`if (!export_script_node(doc, depsgraph, material)) {`
bNode *output_node = ntreeShaderOutputNode(material->nodetree, SHD_OUTPUT_ALL);
if (output_node) {
NodeParserData data = {doc.get(),
depsgraph,
material,
NodeItem::Type::Material,
nullptr,
NodeItem(doc.get()),
export_image_fn};
output_node->typeinfo->materialx_fn(&data, output_node, nullptr);
}
else {
DefaultMaterialNodeParser(doc.get(),
depsgraph,
material,
nullptr,
nullptr,
NodeItem::Type::Material,
nullptr,
export_image_fn)
.compute_error();
}
}
}
else {