MaterialX icon indicating copy to clipboard operation
MaterialX copied to clipboard

Viewer: keyboard shortcut, write output for GLSL Vulkan Shader source

Open juandejoya opened this issue 8 months ago • 5 comments

Addressing issue #2380 with the following implementations:

  • Added a Vulkan shader generator context that follows the pattern of the GLSL shader generator context. This is used to write write out the Vulkan shader source to disk in SPIR-V.
  • The Viewer uses a shader generator's unique target identifier to determine how it should write out a shader source to disk. We update VkShaderGenerator to have a unique vulkan target and added a vulkan.mtlx target definition.
  • Added a keyboard shortcut (V) to write out the GLSL Vulkan shader source and updated the help UI.

The following image shows the behavior exhibited in this PR: mtlx2380_currentbehavior

juandejoya avatar May 19 '25 08:05 juandejoya

Looking at Matrix: build failures, they seem to be related copying/moving being prohibited for the GenContext constructor:

GenContext() = delete;

Not sure if this is already expected behavior from the code (that may need to be fixed?) since the the GenContext consturctor similarly across the different shader generators. I imagine I should be using references or pointers to address the solution, but I may be missing something.

juandejoya avatar May 19 '25 09:05 juandejoya

Looking at Matrix: build failures, they seem to be related copying/moving being prohibited for the GenContext constructor:

GenContext() = delete;

Not sure if this is already expected behavior from the code (that may need to be fixed?) since the the GenContext consturctor similarly across the different shader generators. I imagine I should be using references or pointers to address the solution, but I may be missing something.

The context is meant hold / reference per target (generator) specific data so per target you can reuse but not between different targets. GenOptions are the reusable part.

kwokcb avatar May 20 '25 14:05 kwokcb

I will suggest any renaming of the target for Vulkan be put into a separate issue and change where we can perform suitable testing.

kwokcb avatar May 20 '25 14:05 kwokcb

@juandejoya Just following up on this PR, to see if you might have the bandwidth to address the build issues!

jstone-lucasfilm avatar Jul 13 '25 17:07 jstone-lucasfilm

@jstone-lucasfilm thanks for poking! I was going to reach out myself for how to proceed since it does require setting the target definition. I'll look into the build issues this week.

juandejoya avatar Jul 13 '25 18:07 juandejoya