Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

usdMtlx ignores an active file prefix #974

Open
willmuto-lucasfilm opened this issue Sep 19, 2019 · 5 comments
Open

usdMtlx ignores an active file prefix #974

willmuto-lucasfilm opened this issue Sep 19, 2019 · 5 comments

Comments

@willmuto-lucasfilm
Copy link

Description of Issue

usdMtlx ignores an active file prefix defined by the fileprefix attribute in a parent tag.

Steps to Reproduce

  1. Given the following MaterialX file (ExpandFilePrefix.mtlx):
<?xml version="1.0"?>
<materialx version="1.36">
    <nodedef name="ND_layerShader" type="surfaceshader" node="layerShader">
        <input name="in" type="filename"/>
    </nodedef>
    <material name="noPrefix">
        <shaderref name="layered_sr" node="layerShader">
            <bindinput name="in" type="filename" value="img1.jpg"/>
        </shaderref>
    </material>
    <material name="prefixInValue">
        <shaderref name="layered_sr" node="layerShader">
            <bindinput name="in" type="filename" value="images/img2.jpg"/>
        </shaderref>
    </material>
    <material name="prefixInAttr"  fileprefix="images">
        <shaderref name="layered_sr" node="layerShader">
            <bindinput name="in" type="filename" value="img2.jpg"/>
        </shaderref>
    </material>
</materialx>

And running the following python code:

from pxr import Usd, UsdShade, Sdf
stage = Usd.Stage.CreateInMemory()
stage = stage.Open('ExpandFilePrefix.mtlx')
material_noprefix = UsdShade.Material.Get(stage, Sdf.Path('/MaterialX/Materials/noPrefix'))
inputs = material_noprefix.GetInputs()
for i in inputs:
     print i.GetFullName(), i.Get().resolvedPath

material_prefixed1 = UsdShade.Material.Get(stage, Sdf.Path('/MaterialX/Materials/prefixInValue))
inputs = material_prefixed1.GetInputs()
for i in inputs:
     print i.GetFullName(), i.Get().resolvedPath

material_prefixed2 = UsdShade.Material.Get(stage, Sdf.Path('/MaterialX/Materials/prefixInAttr))
inputs = material_prefixed2.GetInputs()
for i in inputs:
     print i.GetFullName(), i.Get().resolvedPath

Returns the following results:

inputs:in /home/wmuto/usd_fileprefix/img1.jpg
inputs:in /home/wmuto/usd_fileprefix/images/img2.jpg
inputs:in 

This fix ensures that the fileprefix attribute is respected:

inputs:in /home/wmuto/usd_fileprefix/img1.jpg
inputs:in /home/wmuto/usd_fileprefix/images/img2.jpg
inputs:in /home/wmuto/usd_fileprefix/images/img2.jpg

I have a fix which I will be submitting a pull request for shortly!

System Information (OS, Hardware)

Linux RHEL7

Package Versions

Build Flags

@jilliene
Copy link

Filed as internal issue #USD-5571

@Derek-Nvidia
Copy link
Contributor

We've recently encountered this same issue. This ticket remains open, however the related ticket #977 was closed w/o the change being merged. Is there any particular reason why? I have a fix that I can polish up and contribute ; however I wanted to make sure I understand the complete history.

@pmolodo
Copy link
Contributor

pmolodo commented Aug 16, 2023

Just to help demonstrate that this is still an issue as of 23.08, I adapted @willmuto-lucasfilm 's original reproduction code for python3, and made it self-contained (ie, no need to download anything else):

import os
import tempfile

from pxr import Sdf, Usd, UsdShade

mtlx_contents = """\
<?xml version="1.0"?>
<materialx version="1.36">
    <nodedef name="ND_layerShader" type="surfaceshader" node="layerShader">
        <input name="in" type="filename"/>
    </nodedef>
    <material name="noPrefix">
        <shaderref name="layered_sr" node="layerShader">
            <bindinput name="in" type="filename" value="img1.jpg"/>
        </shaderref>
    </material>
    <material name="prefixInValue">
        <shaderref name="layered_sr" node="layerShader">
            <bindinput name="in" type="filename" value="images/img2.jpg"/>
        </shaderref>
    </material>
    <material name="prefixInAttr" fileprefix="images">
        <shaderref name="layered_sr" node="layerShader">
            <bindinput name="in" type="filename" value="img2.jpg"/>
        </shaderref>
    </material>
</materialx>"""


with tempfile.TemporaryDirectory() as tempdir:
    os.chdir(tempdir)

    # create the necessary files
    mtlx_path = "ExpandFilePrefix.mtlx"
    with open(mtlx_path, "w") as writer:
        writer.write(mtlx_contents)

    stage = Usd.Stage.CreateInMemory()
    stage = stage.Open(mtlx_path)

    for mat_path, expected_img in [
        ("/MaterialX/Materials/noPrefix", "img1.jpg"),
        ("/MaterialX/Materials/prefixInValue", "images/img2.jpg"),
        ("/MaterialX/Materials/prefixInAttr", "images/img2.jpg"),
    ]:
        material = UsdShade.Material.Get(stage, mat_path)
        input = material.GetInput("in")
        actual_img = input.Get().path
        print(actual_img)
        if actual_img != expected_img:
            print(f"    !!!! Expected: {expected_img} !!!!")

This outputs:

img1.jpg
images/img2.jpg
img2.jpg
    !!!! Expected: images/img2.jpg !!!!

@willmuto-lucasfilm
Copy link
Author

Hi @pmolodo @Derek-Nvidia , my apologies for the confusion here! Back when I originally submitted the PR there was some restructuring of the project and from what I remember I was going to open a new PR after the rebase which is why the related ticket was closed. I never got around to sorting this out after I wrapped my show at the time and went on holiday after, and it fell off my radar.

I can dust off the change #977 or if you have a change ready to roll @Derek-Nvidia please feel free to use that. Again, my apologies!

@Derek-Nvidia
Copy link
Contributor

Hi,
i have created an MR for this: #2660

pixar-oss added a commit that referenced this issue Nov 1, 2023
Material-X: flatten filenames upon read  #974

(Internal change: 2301054)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants