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

"A Beautiful Game" via the gltf_pbr node #1098

Closed

Conversation

emackey
Copy link

@emackey emackey commented Oct 7, 2022

UPDATE: This branch has been redone, and now contains the updated texture set in a common location. There is a single chess_set.glb file that works with both Standard Surface and glTF material versions, interchangeably.


The following original description is mostly obsolete now.

I'm opening this PR for comparison purposes. I don't think that folks would actually want "chess_set" and "a_beautiful_game" to co-exist in the examples folder, as users could easily confuse the material set from one with the geometry from the other, causing bad things to render.

This PR contributes a second copy of "A Beautiful Game," the chess set originally from SideFX that was open-sourced by ASWF during SIGGRAPH 2022. This new copy uses the <gltf_pbr> node, trying to replicate the glTF PBR materials from glTF's own new version, "A Beautiful Game" glTF sample model.

There is an issue here with transmission on <gltf_pbr>, which could be my error or could be some deeper problem, I'm not sure. The tops of the pawns (Pawn_Top_B and Pawn_Top_W) call for transmission but it doesn't look like they have it correctly enabled.

a_beautiful_game.glb is ~ 10.3 MB, and its material gltf_pbr_a_beautiful_game.mtlx references ~ 18.1 MB of textures.

screenshot of gltf_pbr

By comparison, chess_set.glb is ~ 34 MB, using standard_surface_chess_set.mtlx which references ~ 15.1 MB of textures.

screen shot with std surface

In the existing chess_set version from main, you can see transmission in the tops of the pawns, that is not working somehow in the gltf_pbr version.

Note the models are NOT interchangeable. The new one contributed here has much smaller geometry because I've made use of instancing. The new model contains only one copy of the pawn's mesh data, where the old one has 16 copies, one for each pawn. Likewise for other pieces, all instanced 2 times or 4 times as appropriate. This makes the mesh "playable" in that a host application could move the pieces around as individual nodes. But it changes the node structure and naming system, so the material set needs to be adapted for the new structure.

One more difference, the glTF texture set is slightly larger. I have a somewhat higher quality setting particularly for the chess board itself, and I baked the original displacement maps into the normal maps. In addition, the glTF textures contain ambient occlusion, which is wired into the gltf_pbr node but never used by MaterialXView.

Compare to this screenshot from BabylonJS, a real-time render in WebGL:

screenshot of BabylonJS

Note for example the dark "slot" in the top of the Bishop. The AO map lets Babylon know that not much environmental light gets in that slot, although a direct light source could still illuminate the inside of it if needed. It would be great for MaterialXView to be able to pick up the AO map from gltf_pbr and make use of it for non-path-traced interactive renders.

/cc @kwokcb @proog128 @jstone-lucasfilm

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 7, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: emackey / name: Ed Mackey (ccf7a05)

@jstone-lucasfilm
Copy link
Member

Thanks for providing this great reference, @emackey, and I'd be interested in incorporating some of these improvements and observations directly into the MaterialX codebase. Two initial thoughts come to mind:

  • Would it make sense to replace our chess_set.glb with the more optimized, flexible version you present here, with the corresponding changes made to the look element in our chess_set.mtlx file? It would seem ideal to make this improved GLB file the canonical version of the Open Chess Set geometry, unless there are performance tradeoffs that we should be aware of.
  • We should follow up on the missing transmission rendering for the gltf_pbr variant of the material, as I'd expect it to render with equivalent fidelity to the standard_surface version. I'm CC'ing @proog128, @pablode, and @kwokcb for their thoughts, as they have more experience in rendering with this graph in MaterialX.

@emackey
Copy link
Author

emackey commented Oct 10, 2022

It would seem ideal to make this improved GLB file the canonical version of the Open Chess Set geometry, unless there are performance tradeoffs that we should be aware of.

Yes, I think this should become the new GLB file for the Chess Set. One performance implication is that typical real-time systems make [at least] one draw call per mesh, so it's likely that this new version causes typical systems to issue more draw calls than the previous version. But, the memory savings (both file and GPU mem) and added flexibility (to move chess pieces around individually) make this a worthwhile trade, in my opinion.

We should follow up on the missing transmission rendering for gltf_pbr

Yes please. I'm not equipped to debug a MaterialX node graph myself.

@kwokcb
Copy link
Contributor

kwokcb commented Oct 18, 2022

Without looking at this too closely yet, some off the top comments:

  • I'd recommend that we keep both the current standard surface version and gltf pbr materialx materials and fix up the assignment as needed for the std surface version.
  • I'm unsure of the transmission difference. It could be addressed with @proog128's latest check-in.
  • I wouldn't think that the desktop version of MaterialXView would render any faster since we didn't add in transform instance support -- though I think we should :).
  • We'd want to run this in the MaterialXView Web for perf and visual comparison.

I am curious about the USD version of this @pablode -- I assume that it's using std surface currently so it shows up with existing render delegates? It would however be good to see how this new version renders in USD.

@pablode
Copy link
Contributor

pablode commented Oct 18, 2022

The transmission is not working because the wrong material is assigned.

I think there are two issues at play here:

  1. the 'geom' expressions in the look are wrong: they should be prefixed with '/' and contain the hierarchy (for the pawn tops: /Pawn_Body_B1/Pawn_Top_B1). See spec p. 14 "Geometry Representation".
  2. the automatic material assignment fails (even with the above change). This may be due to a corner case: in the above path, the parent (/Pawn_Body_B1) is assigned one material, and the child (/Pawn_Body_B1/Pawn_Top_B1) another one. Apparently only the parent is matched.

The materials can still be assigned manually, proving that the glTF PBR works correctly.

@emackey nice job with baking the displacement maps in the normal maps! I think it would make sense to also apply this change to the USD version once this PR is merged.

@emackey
Copy link
Author

emackey commented Oct 18, 2022

@pablode Awesome, thanks for the investigation and advice.

I've pushed an update to the geom fields, and as you predicted in point 2, the pawn top materials are working but are now incorrectly matching the pawn bodies as well. I suppose that's a bug I should file separately? Or is this covered by #1108?

@pablode
Copy link
Contributor

pablode commented Oct 18, 2022

@emackey feel free to report this as a bug - #1108 is something different. None of these issues should block this PR.

@jstone-lucasfilm
Copy link
Member

This looks like great work, thanks @emackey and @pablode!

Taking this forward within the MaterialX repository, my recommendation would be to update the canonical GLB for the Open Chess Set to match this latest version, with a corresponding update to the look element of the Standard Surface material.

When we ultimately store a glTF PBR version of the chess set in MaterialX, it should likely be an in-place shader translation of the Standard Surface version (e.g. using the T key in MaterialXView). This would avoid any duplication of textures in the repository, and would allow the glTF PBR version to stay in sync when updates to the Standard Surface version are made.

@emackey
Copy link
Author

emackey commented Oct 24, 2022

I'll take a look at updating the Standard Surface graph to match what the glTF graph is doing now. But, I do have concerns about downgrading the glTF version to be just an output target for a translation graph. In particular,

  • The glTF material contains an Ambient Occlusion channel not found in Standard Surface.
  • The glTF material contains a thickness value not supplied by Standard Surface. This is needed for volumetric rendering to be properly enabled in glTF.
  • Some artists have shown an interest in using glTF's PBR material very early in the authoring pipeline, long before the asset is finished and exported from USD to glTF format. Our material is becoming available in a growing number of applications, and has pushed implementations towards advanced features such as realtime transmission and iridescence.
  • There should be high-quality usage examples of the PBR node, directly testing and demonstrating all of its various inputs.

@jstone-lucasfilm
Copy link
Member

Good points, @emackey, and I completely agree that we don't want to suggest that any shading model is preferred over another in MaterialX. An artist should be able to choose the right shading model for their needs, and there are many situations in which the right choice is glTF PBR.

On the other hand, since the Open Chess Set was originally authored in Standard Surface, there are some advantages in maintaining this as the authoritative definition of that asset, with other versions being derived through shader translation.

To my mind, the ideal outcome would be for another high-quality asset to be contributed to MaterialX using the glTF PBR shading model, and this asset would then be canonically defined in glTF PBR.

No hurry on the update to the geometry for the Open Chess Set, but I do think that your optimizations would be broadly valuable, irrespective of which shading model the user prefers for rendering the asset.

@jstone-lucasfilm
Copy link
Member

@emackey Here's a quick sketch of the "pure optimization" version of your proposal, where we'd be replacing the canonical geometry for the Open Chess Set with your optimized, refactored version:

jstone-lucasfilm@b0ea552

My sense is that this change would be worthwhile on its own, and a single, optimized geometric definition would make it straightforward to include additional shading models in the future without duplication.

What are your thoughts? Would you be interested in writing up a pull request along these lines?

- Displacement maps from orgininal tutorial baked into normal maps.
- Higher JPG quality setting, particularly noticable on the chess board.
- Ambient Occlusion maps baked for all chess pieces.
@emackey
Copy link
Author

emackey commented Oct 31, 2022

@jstone-lucasfilm Thanks for the SS material mapping. You're welcome to merge just the new geometry, as you prototyped in that commit. I will point out though that the texture set included here is not the canonical one: The original texture set included displacement maps, and much higher quality textures than would make sense to include here. For the glTF version, I had baked the displacement into the normals, and I had re-converted some textures (particularly the large chess board) from the originals to new JPGs with higher quality settings, as I was seeing artifacts on the board. (There still are some, not as many as before).

I've pushed a new thing to consider: The new image set has been migrated to the ../Images folder, where both a glTF and a standard_surface material can reach it. This meant updating the standard_surface version to use glTF's "orm" pattern (Occlusion, Roughness, Metallic), but it's easily done with the MaterialX channels chooser attribute.

I'd certainly understand if you or other maintainers here wish to decline this kind of change, as it may be too much glTF-ness leaking into a Standard Surface example. But take new screenshots and compare with main, I would claim this branch looks a little bit better.

@jstone-lucasfilm
Copy link
Member

This changelist looks really promising, thanks @emackey, and the error messages seen above are related to the shader translation system in MaterialX (not your pull request).

My main concern with changing/upgrading the textures is that we have a number of canonical renders of the Open Chess Set in Arnold and Karma CPU/XPU (including a very prominent one on www.materialx.org), and we'd need to consider these renders out-of-date if we modified the canonical versions of the textures for this asset. Upgrading the geometry doesn't create this concern, as in theory it should have no effect on the renders if they were run again today.

Perhaps we should take this in stages, upgrading the geometry for now, and then considering how the textures ought to evolve over time. Let me know what your thoughts are.

@emackey
Copy link
Author

emackey commented Nov 13, 2022

I'm fine with that too. Your commit here captures that well I think:

jstone-lucasfilm/MaterialX@b0ea552

@jstone-lucasfilm
Copy link
Member

Since that optimization work was 100% yours, I'd be happy for you to contribute that change! Let me know, though, if you'd prefer that we make the change on our side instead.

@jstone-lucasfilm
Copy link
Member

I'll close out this pull request for now, but I believe it remains valuable as a reference for future optimizations and improvements to the Open Chess Set. Thanks, @emackey!

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

Successfully merging this pull request may close these issues.

4 participants