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

Incorrect sensor data: Low level materials and other cameras #544

Closed
darksylinc opened this issue Jan 22, 2022 · 0 comments
Closed

Incorrect sensor data: Low level materials and other cameras #544

darksylinc opened this issue Jan 22, 2022 · 0 comments
Assignees
Labels
enhancement New feature or request

Comments

@darksylinc
Copy link
Contributor

darksylinc commented Jan 22, 2022

This is a similar / sister ticket of #543:

After gaining the ability of custom low level materials on objects, it raises an issue: If the material implements custom vertex deformation (i.e. like the waves sample from PR #541) then this deformation won't happen in the other cameras (Thermal, LaserRetro, Segmentation, SelectionBuffer).

The solutions I see are (some of them are not mutually exclusive):

  1. Attempt to clone the material while overriding the pixel shader while keeping the vertex shader (should work 95% of time, as a fallback)
  2. Let the interface define alternate materials for each of the 4 cameras
  3. Patch the pixel shader ourselves. Ask the user to write a special string pattern e.g.
#version 330

// IGN CUSTOM VARIABLES

void main()
{
   gl_FragColor = ...;
// IGN CUSTOM POST PS EXECUTION
}

and then when the switchers run, we look at the shader source for string matching // IGN CUSTOM POST PS EXECUTION and // IGN CUSTOM VARIABLES and we replace those strings with our custom strings, clone the pixel shader and compile that.

Update

So, talking with @iche033 , solution 1 is likely the simplest and most viable.

I also remembered what was that "5% incompatibility" I mentioned. Shaders have signatures. What the VS sends to the PS must be compatible; and declaration order is important.

For example if Vertex Shader declares:

struct ToPixelShader
{
    float2 uv;
    float3 otherData;
}

Then our pixel shader can use:

struct ToPixelShader
{
    float2 uv;
}

However if the VS had declared:

struct ToPixelShader
{
    float3 otherData;
    float2 uv;
}

In other words, the declaration order has been reversed, then linkage will fail because the first float3 does not match with the float2 that the pixel shader will be expecting.

As I told Ian, it would be wise to make a "tester tool"; i.e. a program or routine that will attempt to run that low level material through all the cameras. If it passes, the materials is good. If it fails, the user can check the logs to see what's causing trouble (which is almost always going to be an incompatible VS to PS signature).

When it comes to HLSL (ie. Windows / D3D11) position is part of the signature, thus to maximize compatibility the signature should always look like this:

struct ToPixelShader
{
    float2 uv : TEXCOORD0;
    float3 position : SV_Position;

    // The rest
    float3 otherData : TEXCOORD1;
}

Desired behavior

The 4 cameras should consider vertex deformation and pixel shader's discard functionality.

@darksylinc darksylinc added the enhancement New feature or request label Jan 22, 2022
@darksylinc darksylinc changed the title Low level materials and other cameras Incorrect sensor data: Low level materials and other cameras Jan 23, 2022
darksylinc added a commit to darksylinc/ign-rendering that referenced this issue Mar 6, 2022
iche033 pushed a commit that referenced this issue Mar 14, 2022
#578)

* Respect vertex shader from low level materials in sensor passes (#544)

clone the material while overriding the pixel shader while keeping the vertex shader

Signed-off-by: Matias N. Goldberg <[email protected]>
@iche033 iche033 closed this as completed Mar 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants