NVIDIAGameWorks / Falcor

Real-Time Rendering Framework
https://developer.nvidia.com/falcor
Other
2.54k stars 464 forks source link

Bug: Error mByteSize when changing the order of params definition in shader #401

Closed satoshiSchubert closed 8 months ago

satoshiSchubert commented 8 months ago

When testing the SceneDebuggerPass, I found that changing the order of variables in the struct SceneDebuggerParamsin Source\RenderPasses\SceneDebugger\SharedTypes.slang can cause rendering errors, later I discovered that some variables in SceneDebuggerParams were not being assigned correctly. Upon further examination, under certain variable arrangement orders, in ShaderVar var = mpDebugPass->getRootVar()["CB"]["gSceneDebugger"], there is a mismatch in dataSize between mByteSizeand actual size of SceneDebuggerParams(specifically, mByteSizewas 4 bytes larger than the actual size), which caused an error when calling var["params"].setBlob(mParams). Here is an example:

  1. correct case:

    
    struct SceneDebuggerParams
    {
    uint mode = (uint)SceneDebuggerMode::FaceNormal; ///< Current visualization mode. See SceneDebuggerMode.
    uint2 frameDim = { 0, 0 };
    uint frameCount = 0;
    
    uint2 selectedPixel = { 0, 0 }; ///< The currently selected pixel for readback.
    int flipSign = false;           ///< Flip sign before visualization.
    int remapRange = true;          ///< Remap valid range to [0,1] before output.
    int clamp = true;               ///< Clamp pixel values to [0,1] before output.
    
    int showVolumes = true;   ///< Show volumes.
    float densityScale = 1.f; ///< Volume density scale factor.

};

debug info:
![image](https://github.com/NVIDIAGameWorks/Falcor/assets/44137161/d7f6c1ab-16dc-418c-acc4-b740557688db)

2. error case:
```cpp
struct SceneDebuggerParams
{

    uint2 frameDim = { 0, 0 };
    uint frameCount = 0;

    uint2 selectedPixel = { 0, 0 }; ///< The currently selected pixel for readback.
    int flipSign = false;           ///< Flip sign before visualization.
    int remapRange = true;          ///< Remap valid range to [0,1] before output.
    int clamp = true;               ///< Clamp pixel values to [0,1] before output.

    int showVolumes = true;   ///< Show volumes.
    float densityScale = 1.f; ///< Volume density scale factor.
    uint mode = (uint)SceneDebuggerMode::FaceNormal; ///< Current visualization mode. See SceneDebuggerMode.
};

debug info: image and it will cause: image which resulted in assignment errors.

the followings are scripts mentioned above:

# sceneDebugger.py
import sys
sys.path.append('..')
sys.path.append('F:/WorkSpace/Falcor/scripts/')
import os
from graphs.SceneDebugger import SceneDebugger as g
from falcor import *

m.addGraph(g)
m.loadScene('Volumes.pyscene')
# Volume.pyscene
# Create volumes

sphereVolume = GridVolume('Sphere')
sphereVolume.densityGrid = Grid.createSphere(1.0, 0.01)
sphereVolume.densityScale = 2.333
sceneBuilder.addGridVolume(sphereVolume)

boxVolume = GridVolume('Box')
boxVolume.densityGrid = Grid.createBox(1.0, 1.0, 1.0, 0.01)
boxVolume.densityScale = 2.5
sceneBuilder.addGridVolume(boxVolume)

# Create camera

camera = Camera()
camera.position = float3(1, 0.4, 0.8)
camera.target = float3(0, 0, 0)
camera.up = float3(0, 1, 0)
sceneBuilder.addCamera(camera)

# Setup envmap

sceneBuilder.envMap = EnvMap("BlueSky.png")
sceneBuilder.envMap.intensity = 1

Falcor version: 7.0 Win11 VS2022 CPU: i5 12400F GPU: RTX4060 Ti Driver Version: 532.03
CUDA Version: 12.1

skallweitNV commented 8 months ago

Yeah, if you want to share struct definitions in both c++ and slang code, you have to make sure you respect the layout rules of hlsl/slang. See here: https://learn.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-packing-rules