zeux / niagara

A Vulkan renderer written from scratch on stream
MIT License
1.26k stars 72 forks source link

Possible negation error in frustum culling code. #26

Closed JesseRMeyer closed 2 years ago

JesseRMeyer commented 2 years ago

I found implementing the frustum culling code in my engine required changing

visible = visible && center.z * cullData.frustum[3] - abs(center.y) * cullData.frustum[2] > -radius;

to

visible = visible && center.z * cullData.frustum[3] + abs(center.y) * cullData.frustum[2] > -radius;

to work as expected. (The difference turns into a sum). Otherwise objects going out of view off the top or bottom of the camera were not handled correctly with a look camera I implemented. I don't have a derivation that this is correct or that your code is wrong, so I apologize if this is a 'two wrongs make a right' in my code base. The projection matrix I use is reverse Z with infinite projection modified to support rendering objects at infinity correctly, but after some checking I could not see how these changes would alter the behavior of culling along the Y planes of the frustum.

If this is true, then it would not have been easily detectable as-is since most objects are within the view frustum anyway, and the cost of the (alleged) error is just more objects to process.

JesseRMeyer commented 2 years ago

Closing as the code change to this project is incorrect. I suppose it must have something to do with the implicit identity view transform in this project, and the spatial orientation convention of my look camera. But hopefully this serves as a launch point for others learning from this project.