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

Handle locale for MaterialX inputs #1178

Merged
merged 14 commits into from
May 5, 2021

Conversation

ashwinbhat
Copy link
Collaborator

@ashwinbhat ashwinbhat commented Apr 28, 2021

Some background reference around this https://stdcxx.apache.org/doc/stdlibug/24-3.html
https://docs.microsoft.com/en-us/globalization/locale/number-formatting

For MaterialX let's assume we have a xml with
<input name="diffColor" type="vector3" value="1,1.5,2.0"/>

mx:Vector3(1,1.5,2.0) should be interpreted as float[3] = [1.0f, 1.5f, 2.0f]
But based on the locale settings, 1,1.5 will be parsed as a value.

#1183

@ashwinbhat ashwinbhat added the Do Not Merge This PR should not be merged label Apr 28, 2021
@ashwinbhat ashwinbhat self-assigned this Apr 28, 2021
@ashwinbhat ashwinbhat marked this pull request as draft April 28, 2021 04:05
@ashwinbhat ashwinbhat removed the Do Not Merge This PR should not be merged label Apr 30, 2021
@ashwinbhat ashwinbhat marked this pull request as ready for review April 30, 2021 22:08
fixup! fix build break
Also add githash on generated files

e.g. given a node def e.g. ND_standard_surface_surfaceshader will
     generate a standard_surface.json and standard_surface.hpp

The hpp/json can be used for simple reflection instead of parsing mtlx libraries
Move utility python script into contrib
@ashwinbhat
Copy link
Collaborator Author

@bernardkwok I'm unsure why the build tests fail. I'm unable to repro this locally
100% tests passed, 0 tests failed out of 72


def main():
parser = argparse.ArgumentParser(
description="MaterialX nodedef to json/hpp convert.")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick. "converter" instead of "convert" ?

return None
for elem in elemlist:
if elem.isA(mx.NodeDef) and elem.getName() == nodedefname:
#print (elem)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove commented out code.

parser.add_argument(dest="inputFilename",
help="Filename of the input document.")
parser.add_argument("--node", dest="nodedef", type=str,
default="ND_standard_surface_surfaceshader",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe leave the default to be empty string instead ?

if elem.isA(mx.NodeDef):
jsonfilename = elem.getNodeString()+'.json'
hppfilename = elem.getNodeString()+'.hpp'
export_json(elem, jsonfilename)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we catch exceptions here in case the files cannot be written.

\n{nodename}\nSHA1:{filehash}\nVersion:{version}\n*/\n"\
.format(nodename=elem, filehash=INPUTFILEHASH, version=mx.getVersionString())
variable_defs = ""
for inp in elem.getActiveInputs():

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned in chat, would you want to export outputs as well ?
In which case would it be better to get "value elements" instead of just inputs. ?

@bernardkwok bernardkwok added this to In progress in Develop via automation May 4, 2021
Develop automation moved this from In progress to Reviewer approved May 5, 2021
@ashwinbhat ashwinbhat changed the title Add test case for locale testing Handle locale for MaterialX inputs May 5, 2021
@ashwinbhat ashwinbhat merged commit b302c4b into adsk_contrib/dev May 5, 2021
Develop automation moved this from Reviewer approved to Done May 5, 2021
@ashwinbhat ashwinbhat deleted the adsk_contrib/locale_utf8 branch May 5, 2021 15:07
bernardkwok pushed a commit that referenced this pull request May 31, 2021
For MaterialX let's assume we have a xml with `<input name="diffColor" type="vector3" value="1,1.5,2.0"/>`
mx:Vector3(1,1.5,2.0) should be interpreted as float[3] = [1.0f, 1.5f, 2.0f]. But based on the locale settings, 1,1.5 will be parsed as a value. By setting the locale on std::stringstream we ensure that input values are correctly read 
Some background reference around this https://stdcxx.apache.org/doc/stdlibug/24-3.html
https://docs.microsoft.com/en-us/globalization/locale/number-formatting
Partially addresses Issue #1183

Other changes:
Basic utility to generate a json and hpp export from MaterialX nodedef
e.g. given a node def e.g. ND_standard_surface_surfaceshader will
     generate a standard_surface.json and standard_surface.hpp
The hpp/json can be used for simple reflection instead of parsing mtlx libraries
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Develop
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants