OGRECave / ogre-next

aka ogre v2 - scene-oriented, flexible 3D C++ engine
https://ogrecave.github.io/ogre-next/api/latest
Other
1.08k stars 233 forks source link

Miss-spelled blendWeights in VulkanMappings #386

Closed stevenlgreen00 closed 1 year ago

stevenlgreen00 commented 1 year ago

I am exploring ogre-next vulkan shader implementation, and came across a miss-spelled label.

I do not know if this really causes an issue anywhere.

In OgreVulkanMappings.cpp:
VertexElementSemantic VulkanMappings::getHlslSemantic( const char *sem ) {
      ...
      // I think this should be "input.blendWeights"
      if( strcmp( sem, "input.blendWeigth" ) == 0 )
            return VES_BLEND_WEIGHTS;
      ...
}
darksylinc commented 1 year ago

Thanks.

Fixed.

For reference, we support two ways of compiling to SPIR-V:

  1. GLSL, via glslang (default)
  2. HLSL, via glslang (not recommended)

HLSL support was an interesting attempt but ultimately it was abandoned because glslang doesn't have good support for it. It's ok for basic stuff but it had various issues when doing cross platform (e.g. Android) support or doing more advanced operations.

I'm not even sure if HLSL via glslang is enabled by default, thus this bug impacts very few people, if any. We may even remove support for HLSL via glslang in the future due to being abandoned.

Nowadays even Khronos itself recommends to use dxc instead of glslang to support HLSL in Vulkan. We haven't done any work on integrating dxc, and we don't have it planned either, at least for now.

The main reason for our lack of interest is that dxc's SPIR-V flavour requires some Vulkan extensions that are unavailable on some Android devices, and Android support is a big appeal for Ogre-Next's Vulkan RenderSystem.

Cheers

stevenlgreen00 commented 1 year ago

Thanks for the correction:         Note that "blendWeights" is plural, your correction is missing the "s".

Also, I am new to Ogre-next and my interest is in creating a custom shader, but I am starting off simple trying to implement a rudementary set of shaders using ogre-next constructs that I can build on.

I hope to create a how-to for people trying to do the same with a simple shader and example code.

Due to prior exposure to Vulkan I had been focusing on that render system, which I understand to be GLSL. I  am using it to identify shader variable names so I could follow the flow of data from the meshes and materials to the shaders.

The HLSL mapping function  was the only I noticed under vulkan, not sure if it serves double purpose or not.

I have been looking at the HLMS samples and the render systems to start, and am starting to get a feel for how things are arranged and starting to see the relationship to the VES_ identifiers.

Hopefully I will be able to get together some code soon.

Thanks for the information on SPIR-V, I appreciate it.

Steven Green 

On Tue, 2023-05-16 at 14:26 -0700, Matias N. Goldberg wrote:

Thanks. Fixed. For reference, we support two ways of compiling to SPIR-V:

  1. GLSL, via glslang (default)
  2. HLSL, via glslang (not recommended) HLSL support was an interesting attempt but ultimately it was abandoned because glslang doesn't have good support for it. It's ok for basic stuff but it had various issues when doing cross platform (e.g. Android) support or doing more advanced operations. I'm not even sure if HLSL via glslang is enabled by default, thus this bug impacts very few people, if any. We may even remove support for HLSL via glslang due to being abandoned. Nowadays even Khronos itself recommends to use dxc instead of glslang to support HLSL in Vulkan. We haven't done any work on integrating dxc, and we don't have it planned either, at least for now. The main reason for our lack of interest is that dxc's SPIR-V flavour requires some Vulkan extensions that are unavailable on some Android devices, and Android support is a big appeal for Ogre-Next's Vulkan RenderSystem. Cheers — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>