BogdanNagirniak/blender
0
0
Fork 3
forked from blender/blender
Code Pull Requests Activity

BLEN-326: Pass MaterialX to Hydra via hdMaterialNetwork #5

Merged
Bogdan Nagirniak merged 4 commits from George-Shakula/blender:mtlx-translation into hydra-render 2023-03-02 12:35:51 +01:00
Conversation 6 Commits 4 Files Changed 7 +135 -2
George-Shakula commented 2023-02-22 17:16:42 +01:00
Collaborator
Copy Link

These changes require USD to be built with the usdMtlx module. Right now, pre-built USD stored on the blender's svn is built without it. @BogdanNagirniak do you see how to deal with that?

Here's how RPR delegate renders a sample scene with these changes:
image_2023-02-22_18-09-07.png

Storm does not work correctly. I'm going to push fixes for Storm in a separate PR because they are unrelated to the mtlx integration.

These changes require USD to be built with the usdMtlx module. Right now, pre-built USD stored on the blender's svn is built without it. @BogdanNagirniak do you see how to deal with that? Here's how RPR delegate renders [a sample scene](/attachments/a6a1b5f0-a50d-497e-a9e4-dc3c7d15820e) with these changes: ![image_2023-02-22_18-09-07.png](/attachments/ab4f003e-fc74-47d1-9ebb-740f29f2c09a) Storm does not work correctly. I'm going to push fixes for Storm in a separate PR because they are unrelated to the mtlx integration.
matx.blend
998 KiB
image_2023-02-22_18-09-07.png
634 KiB
George-Shakula force-pushed mtlx-translation from e74c9496fc to 504e22c3f0 2023-02-22 19:42:46 +01:00 Compare
George-Shakula force-pushed mtlx-translation from 504e22c3f0 to caf8d35795 2023-02-22 19:43:54 +01:00 Compare
Georgiy Markelov was assigned by Bogdan Nagirniak 2023-02-22 19:56:33 +01:00
Bogdan Nagirniak self-assigned this 2023-02-22 19:56:33 +01:00
Bogdan Nagirniak requested review from Georgiy Markelov 2023-02-22 19:56:39 +01:00
George-Shakula commented 2023-02-25 23:26:10 +01:00
Author
Collaborator
Copy Link

Also, I forgot to mention that the latest USD expects the 1.38.4 MaterialX library. At the same time, Blender's prebuilt libraries hosted on an SVN repository contain the 1.38.6 MaterialX library. So we need not only to build USD with usdMtlx for this PR to work but also to downgrade the MaterialX library or update USD to use the latest MaterialX.

Also, I forgot to mention that the latest USD [expects the 1.38.4 MaterialX library](https://github.com/PixarAnimationStudios/USD/blob/v23.02/build_scripts/build_usd.py#L1601). At the same time, Blender's prebuilt libraries hosted on an SVN repository contain the 1.38.6 MaterialX library. So we need not only to build USD with usdMtlx for this PR to work but also to downgrade the MaterialX library or [update USD to use the latest MaterialX](https://github.com/pablode/USD/tree/mtlx-1.38.6).
George-Shakula force-pushed mtlx-translation from caf8d35795 to 634cf7f9e3 2023-02-26 12:16:38 +01:00 Compare
George-Shakula commented 2023-02-26 12:17:33 +01:00
Author
Collaborator
Copy Link

Code style has been fixed

Code style has been fixed
George-Shakula force-pushed mtlx-translation from 634cf7f9e3 to 2f5f8e5e83 2023-02-26 13:49:51 +01:00 Compare
Georgiy Markelov commented 2023-02-27 15:50:26 +01:00
Collaborator
Copy Link

Hi.

I will try to build USD with usdMtlx module and test how it works.

Hi. I will try to build USD with usdMtlx module and test how it works.
George-Shakula commented 2023-02-27 16:46:11 +01:00
Author
Collaborator
Copy Link

@DagerD in order for it to work, USD must be built with usd.diff from this PR applied. It requires 1.38.6 MaterialX (the one Blender already targets).

@DagerD in order for it to work, USD must be built with usd.diff from this [PR](https://github.com/bnagirniak/RPRHydraRenderBlenderAddon/pull/7) applied. It requires 1.38.6 MaterialX (the one Blender already targets).
Bogdan Nagirniak reviewed 2023-02-27 20:09:16 +01:00
source/blender/render/hydra/sceneDelegate/material.h
@ -26,3 +26,3 @@
void mark_prim_dirty(DirtyBits dirty_bits) override;
pxr::VtValue material_resource();
pxr::VtValue material_resource(pxr::HdSceneDelegate *scene_delegate);
Bogdan Nagirniak commented 2023-02-27 20:01:19 +01:00
Owner
Copy Link

scene_delegate is already available in MaterialData from IdData

`scene_delegate` is already available in MaterialData from IdData
George-Shakula marked this conversation as resolved
source/blender/render/hydra/sceneDelegate/mtlxHydraAdapter.h
@ -0,0 +14,4 @@
struct HdMaterialNetworkMap;
void HdMtlxConvertToMaterialNetworkMap(std::string const &mtlxPath,
Bogdan Nagirniak commented 2023-02-27 20:08:53 +01:00
Owner
Copy Link

Can we move this function into MaterialData class as method?

Can we move this function into MaterialData class as method?
George-Shakula commented 2023-02-27 20:13:49 +01:00
Author
Collaborator
Copy Link

Yes, we can. One of the reasons it is in a separate file is that Brian wanted to have this feature separated from anything else for convenient sharing with others. By keeping it in its own files we ensure this is easily relocatable and not obscured by anything else.

Yes, we can. One of the reasons it is in a separate file is that Brian wanted to have this feature separated from anything else for convenient sharing with others. By keeping it in its own files we ensure this is easily relocatable and not obscured by anything else.
Bogdan Nagirniak commented 2023-02-28 14:14:24 +01:00
Owner
Copy Link

I see, ok

I see, ok
BogdanNagirniak marked this conversation as resolved
Georgiy Markelov commented 2023-02-28 13:54:27 +01:00
Collaborator
Copy Link

Material works for HdRPR. Please check warnings (both for HdRPR and HdStorm).
image

Material works for HdRPR. Please check warnings (both for HdRPR and HdStorm). ![image](/attachments/11872637-e9fb-40ec-a41c-8c5d145e25fc)
image.png
171 KiB
Bogdan Nagirniak requested changes 2023-03-01 13:02:22 +01:00
source/blender/render/hydra/CMakeLists.txt
@ -86,4 +86,17 @@ set(LIB
blender_add_lib(bf_render_hydra "${SRC}" "${INC}" "${INC_SYS}" "${LIB}")
if(TARGET MaterialXCore AND TARGET MaterialXFormat)
Bogdan Nagirniak commented 2023-03-01 12:58:52 +01:00
Owner
Copy Link

We expect that MaterialX is enabled for hydra_render project. Can be removed.

We expect that MaterialX is enabled for hydra_render project. Can be removed.
George-Shakula marked this conversation as resolved
source/blender/render/hydra/sceneDelegate/mtlxHydraAdapter.cc
@ -0,0 +64,4 @@
std::string stageId = TfStringPrintf(
"%s%s%s.usda", basePath.c_str(), ARCH_PATH_SEP, mtlxName.c_str());
auto stage = UsdStage::CreateInMemory(stageId, context);
if (_Read(stage, [&mtlxPath]() { return UsdMtlxReadDocument(mtlxPath); })) {
Bogdan Nagirniak commented 2023-02-28 14:30:16 +01:00
Owner
Copy Link

This part with _Read() and lambda function is hard to maintain in future + used in one place, can you move it content of _Read here.

This part with `_Read()` and lambda function is hard to maintain in future + used in one place, can you move it content of `_Read` here.
George-Shakula marked this conversation as resolved
source/blender/render/hydra/sceneDelegate/mtlxHydraAdapter.cc
@ -0,0 +69,4 @@
if (UsdPrimSiblingRange children = materials.GetChildren()) {
if (auto material = UsdShadeMaterial(
*children.begin())) { // TODO: specify which material to use from the mtlx
if (UsdShadeShader mtlxSurface = material.ComputeSurfaceSource(renderContexts)) {
Bogdan Nagirniak commented 2023-02-28 14:32:38 +01:00
Owner
Copy Link

Change construction if (...) if (...) .... to if (!...) return; if (!...) return as more understandable and easier to maintain.

Change construction `if (...) if (...) .... ` to `if (!...) return; if (!...) return` as more understandable and easier to maintain.
BogdanNagirniak marked this conversation as resolved
George-Shakula added 3 commits 2023-03-01 21:39:05 +01:00
Bogdan Nagirniak approved these changes 2023-03-02 12:31:14 +01:00
Bogdan Nagirniak merged commit 85864b39a4 into hydra-render 2023-03-02 12:35:51 +01:00
George-Shakula commented 2023-03-02 22:46:50 +01:00
Author
Collaborator
Copy Link

Material works for HdRPR. Please check warnings (both for HdRPR and HdStorm).
image

As soon as I set PXR_MTLX_STDLIB_SEARCH_PATHS environment variable, these warnings wanish. This leads to a conclusion that either this env var was not set properly while testing the PR or there is some other problem. I would start from embedding this env var into blender.

USD code expects this environment variable to be set if the MaterialX/libraries folder used at the time of compilation is moved to different folder. This is always the case for us because our build is obviously can be installed anywhere.

> Material works for HdRPR. Please check warnings (both for HdRPR and HdStorm). > ![image](/attachments/11872637-e9fb-40ec-a41c-8c5d145e25fc) As soon as I set `PXR_MTLX_STDLIB_SEARCH_PATHS` environment variable, these warnings wanish. This leads to a conclusion that either this env var was not set properly while testing the PR or there is some other problem. I would start from embedding this env var into blender. USD code expects this environment variable to be set if the MaterialX/libraries folder used at the time of compilation is moved to different folder. This is always the case for us because our build is obviously can be installed anywhere.
Sign in to join this conversation.
Reviewers
No reviewers
Georgiy Markelov
Bogdan Nagirniak
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
Bogdan Nagirniak
3 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: BogdanNagirniak/blender#5
No description provided.
Delete Branch "George-Shakula/blender:mtlx-translation"

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?