Search Unity

  1. Unity support for visionOS is now available. Learn more in our blog post.
    Dismiss Notice

Bug Weird "Only instancing constant buffers can have struct variables" error

Discussion in 'General Graphics' started by Saniell, Oct 18, 2023.

  1. Saniell

    Saniell

    Joined:
    Oct 24, 2015
    Posts:
    181
    When using Dx11 unity for some reason modified shader compiler so that any non-instancing cbuffer will produce a compilation error, at least on 2021.3 LTS.

    As it turns out "instancing cbuffer" is one that has name beginning with 'UnityInstancing_'. So workaround is to just add that prefix to cbuffer and it will actually "just work" with Shader.SetGlobalConstantBuffer and friends it seems (at least seems to work so far?)

    Now, I understand this is done for compatibility reasons (probably? as always?), but using cbuffers is important for performance and is advised on nvidia's blog for one (https://developer.nvidia.com/content/how-about-constant-buffers)

    I would appreciate if you removed this restriction for platforms that don't care for it, or turned it into a warning.
     
  2. aleksandrk

    aleksandrk

    Unity Technologies

    Joined:
    Jul 3, 2017
    Posts:
    2,948
    The restriction is only on structs as part of constant buffers. You absolutely can use cbuffers, just not with structs inside.
    Please don't try to work around this by adding a "UnityInstancing_" prefix to the name. It changes the behaviour.
     
  3. Saniell

    Saniell

    Joined:
    Oct 24, 2015
    Posts:
    181
    This makes no sense to have this restrictions in newer unity versions though, since we can set constant buffers directly. Before you would have problems because there was no Shader.SetStruct but it isn't needed and I have hard time understanding how do I not use structs when I need to put array of things into cbuffer.
    Like Light in NVIDIA's example, in my case its even worse and I don't see expressing this properly without structs

    Code (CSharp):
    1. struct TerrainMaterial
    2. {
    3.     float4 scaleTiling;
    4.        
    5.     int albedoTexture;
    6.     int normalTexture;
    7.     int maskTexture;
    8.     int _pad;
    9.        
    10.     float normalStrength;
    11.     float metallic;
    12.     float smoothness;
    13.     int _pad2;
    14. };
    15.  
    16. struct TerrainMaterialTable
    17. {
    18.     int   mainIndex;
    19.     int   triplanarIndex;
    20.     int   blendIndex;
    21.     float blend;
    22. };
    23.  
    24. cbuffer UnityInstancing_TerrainMaterialInfoList_CBuffer
    25. {
    26.     uniform TerrainMaterialTable PT_MaterialTable[32];
    27.     uniform TerrainMaterial      PT_MaterialDesc [24];
    28. };
    Might want to remove level of indirection but still better to have this as structs
     
  4. aleksandrk

    aleksandrk

    Unity Technologies

    Joined:
    Jul 3, 2017
    Posts:
    2,948
    Maybe. We'd need to double-check if that's really the case.
     
  5. Saniell

    Saniell

    Joined:
    Oct 24, 2015
    Posts:
    181
    I appreciate that. To be precise I meant that Shader.SetGlobalConstantBuffer allows you to work with structs as it basically just copies bytes directly. While its not very safe as in you can get alignment wrong, it is still possible

    Will be waiting for update :)
     
  6. Saniell

    Saniell

    Joined:
    Oct 24, 2015
    Posts:
    181
    So... Any news? This keeps being annoying limitation with seemingly no good reason for it
     
  7. aleksandrk

    aleksandrk

    Unity Technologies

    Joined:
    Jul 3, 2017
    Posts:
    2,948
    Hi!
    I checked and it may be that this restriction is not needed anymore, but it will be a feature request, not a bug fix.
     
  8. Saniell

    Saniell

    Joined:
    Oct 24, 2015
    Posts:
    181
    That's understandable. But I assume this means you won't be able to add this to 2022.3? :(
     
  9. aleksandrk

    aleksandrk

    Unity Technologies

    Joined:
    Jul 3, 2017
    Posts:
    2,948
    You're correct, it's unlikely.