ValveSoftware / steam-audio

Steam Audio
https://valvesoftware.github.io/steam-audio/
Apache License 2.0
2.2k stars 152 forks source link

Crashing iplSimulatorRunReflections with baking #347

Closed dkotd closed 2 months ago

dkotd commented 2 months ago

System Information

Issue Description Reflections work fine in my project but it's compute-heavy, so I want to bake my reflections. I have one source that moves around in a circle. My listener is in the middle of circle. I want to perform a static listener bake for the reflections. I've generally followed the programmer's guide for baking, but changed it up a bit to use static listener instead of static sources. However, when I call iplSimulatorRunReflections after baking, it crashes with an "Access violation reading location 0x0000000000000000." error.

Steps To Reproduce

  1. I have one probe array, one probe batch, and one identifier.
  2. When I run the simulation, I set the simulator inputs baked flag and baked data identifier to the same one I used to initially bake.
  3. Crashes when I call iplSimulatorRunReflections

One thing I noticed is that my call to iplReflectionsBakerBake is instantaneous. In the Steam Audio docs, it says that this function will block, so I assumed that it would take a while to simulate the reflections. However, that doesn't seem to be the case for me. I'm wondering if this may be the reason why running reflections is crashing after I bake.

I have also tested with the static source variation but I still get the exact same crash.

dkotd commented 2 months ago

After debugging, I noticed that using the UNIFORMFLOOR generation type doesn't populate my probe array.

IPLMatrix4x4 boxTransform = {
       {{100.0f, 0.0f,   0.0f,   0.0f},
    {0.0f,   100.0f, 0.0f,   0.0f},
    {0.0f,   0.0f,   100.0f, 0.0f},
    {0.0f,   0.0f,   0.0f,   1.0f}}
};

IPLProbeGenerationParams probeParams {};
probeParams.type = IPL_PROBEGENERATIONTYPE_UNIFORMFLOOR;
probeParams.spacing = 2.0f;
probeParams.height = 1.5f;
probeParams.transform = boxTransform;

IPLProbeArray probeArray = nullptr;
iplProbeArrayCreate(context, &probeArray);
iplProbeArrayGenerateProbes(probeArray, scene, &probeParams);
int count = iplProbeArrayGetNumProbes(probeArray); // Returns 0

On the other hand, if I use CENTROID, the probe array has a size of 1 (which is expected since CENTROID only generates one probe in the center of the box) and iplSimulatorRunReflections does not crash.

IPLMatrix4x4 boxTransform = {
     {{100.0f, 0.0f,   0.0f,   0.0f},
      {0.0f,   100.0f, 0.0f,   0.0f},
      {0.0f,   0.0f,   100.0f, 0.0f},
      {0.0f,   0.0f,   0.0f,   1.0f}}
};

IPLProbeGenerationParams probeParams {};
probeParams.type = IPL_PROBEGENERATIONTYPE_CENTROID;
probeParams.transform = boxTransform;

IPLProbeArray probeArray = nullptr;
iplProbeArrayCreate(context, &probeArray);
iplProbeArrayGenerateProbes(probeArray, scene, &probeParams);
int count = iplProbeArrayGetNumProbes(probeArray);  // Returns 1 and no crash

Wondering if this may be the root cause? The API wasn't populating the probes after generating them and so it wasn't baking anything?

achandak commented 2 months ago

Below are three main causes when no probes are generated:

All that said, we should probably do graceful failure here.

dkotd commented 2 months ago

Thank you for your response.

I did a bit more testing, and my scene does have geometry in it. The geometry is specifically a 2D square. I don't think I'm understanding how the probes are generated for UNIFORMFLOOR. Could you explain to me how probe placement works in this example?

IPLMatrix4x4 boxTransform = {
     {{100.0f, 0.0f,   0.0f,   0.0f},
      {0.0f,   100.0f, 0.0f,   0.0f},
      {0.0f,   0.0f,   100.0f, 0.0f},
      {0.0f,   0.0f,   0.0f,   1.0f}}
};

IPLProbeGenerationParams probeParams {};
probeParams.transform = boxTransform;
probeParams.type = IPL_PROBEGENERATIONTYPE_UNIFORMFLOOR; 
probeParams.spacing = 0.2f;
probeParams.height = 0.15f;

IPLBakedDataIdentifier identifier;
identifier.type = IPL_BAKEDDATATYPE_REFLECTIONS;
identifier.variation = IPL_BAKEDDATAVARIATION_STATICLISTENER;
identifier.endpointInfluence.center = listenerPosition; // (1, 1.5, 1)
identifier.endpointInfluence.radius = 100.0f;
  1. Is the center of the probe box always (0, 0, 0)?
  2. When we apply a transform matrix, does it scale it in both the positive and negative axes but at half the scale value?
  3. I'm a little confused on how the height works. What exactly is the "floor"?
  4. After rereading the UNIFORMFLOOR description, it says it's for "solid geometry". Does a 2D square not count as a solid geometry? Hence, no probes are being generated?
achandak commented 2 months ago

A 4 x 4 matrix basically can represent - scaling, rotation, and transform. This link has some basic information that might be helpful on specifying translation in a matrix.

To answer your questions.

  1. Probe box center can be specified through the boxTransform.
  2. The box is centered around the center and its size is defined by the scale.
  3. Imagine dropping probes and them falling on geometry, wherever they hit the geometry is the floor. Height is the distance above the floor at which we place the probe.
  4. As long as the triangles are part of the scene, this should be fine.
dkotd commented 2 months ago

Thank you very much.

  1. It seems like vertical rectangles can't be detected in the probe box because they have no horizontal component. If I angle the rectangle so that it's slightly slanted about the x axis, it'll detect it. Is this expected behavior?
  2. Ideally, I would like both the source and listener to be able to move around. I tried using IPL_BAKEDDATAVARIATION_DYNAMIC for baking, but then my bakes don't seem to reflect anymore. Is there an additional parameter I should toggle? I am specifically trying to bake reflections, not paths.
achandak commented 2 months ago

If you have some minimal viable repro, I can take a quick look. Especially, for issue#2 you mentioned above.

For issue#1 - We assume +Y is up and down is where we let probes fall to the ground. That can be changed by your boxTransform though. Here is how we do it internally, if that helps. So, you can adjust the matrix to rotate -Y to whatever your down vector is.

achandak commented 2 months ago

That said, it can be tricky to get this right. If possible, having a debugging tool for visualizing your geometry and probe box could help.

dkotd commented 2 months ago

Thank you again for all the help.

  1. Further elaboration on the vertical rectangle issue. My positive y is also up. The four points of the rectangle are (0, 0, 0), (1, 0, 0), (1, 1, 0), and (0, 1, 0) and I've added these vertices to the scene as two triangles. My probe box is centered at the origin and has a scale of (100, 100, 100). However, the geometry for the rectangle is not being detected during probe generation even though it exists in the scene. I know it exists in the scene because I am able to apply occlusion on this geometry.
achandak commented 2 months ago

Your spacing between probes is 2.0f and your rectangle is basically of size 1. You can either reduce the spacing between probes or increase the size of rectangle.

dkotd commented 2 months ago

I've tested this with spacing as 0.1f. I've also tried changing the size of the rectangle to have side length of 10 for both sides. Neither worked.

achandak commented 2 months ago

Looks like your quad is in X-Y axis. Can you have a quad in X-Z axis and make sure it is 10 across in size and try again. If that does not work, feel free to share a repro code and I can take a look.

dkotd commented 2 months ago

Yes, if I have a horizontal (i.e. X-Z) quad, I am able to detect probes for it. My root question was if it's possible to have vertical (i.e. X-Y or Y-Z) planes.

achandak commented 2 months ago

Are you trying to place probes on that thin edge? Your down vector is -Y and if there is a just a thin plane, no probe is going to land on that thin edge.

You can manually add probes using iplProbeBatchAddProbe to a probe batch before baking reflections if iplProbeArrayGenerateProbes does not generate probes that you want.

dkotd commented 2 months ago

Regarding my other question about dynamic baking, is dynamic baking only for path and not for reflections? Because I see in the source code that there is an assert for reflection baking.

https://github.com/ValveSoftware/steam-audio/blob/master/core/src/core/reflection_baker.cpp#L54

achandak commented 2 months ago

Oh yes, Nice catch. That is true, primarily because storing N^2 IRs requires a lot of memory.

dkotd commented 2 months ago

I see, thank you very much for all the help.