pierotofy / OpenSplat

Production-grade 3D gaussian splatting with CPU/GPU support for Windows, Mac and Linux 🚀
https://antimatter15.com/splat/?url=https://splat.uav4geo.com/banana.splat
GNU Affero General Public License v3.0
924 stars 87 forks source link

Fix fov xy mismatch #99

Closed Willyzw closed 5 months ago

Willyzw commented 5 months ago

Hi, I found another inconsistency in the CPU pipeline, which is the mismatched x and y in the fov calculation.

pierotofy commented 5 months ago

Hey @Willyzw :wave: thanks the PR. Have you tested these changes with the simple_trainer app?

In particular, try to run it with --width 100 --height 50 --render ./render --cpu and compare the output of the render files with the GPU run (toggling --cpu).

pierotofy commented 5 months ago

Now that I think about it, this should also be tested via a normal run (e.g. ./opensplat /path/to/banana --render-val ./render --cpu -n 300) and compare outputs to the GPU, since fovx/fovy values are equal in the simple_trainer.

Willyzw commented 5 months ago

Now that I think about it, this should also be tested via a normal run (e.g. ./opensplat /path/to/banana --render-val ./render --cpu -n 300) and compare outputs to the GPU, since fovx/fovy values are equal in the simple_trainer.

During my testing with the simple_trainer and the banana dataset, I got same results with and without the changes I made. However, based on my experience, training the model using the CUDA pipeline and validating the rendered image with CPU, with my changes it can lead to improved validation. Also intuitively, we use “x” to refer to width and “y” to refer to height, right?

Regarding the failed check, I have no clue why it failed. Could you help check it?

pierotofy commented 5 months ago

Did some more analysis and it does indeed look like these are swapped. It wasn't apparent during testing because the banana dataset has the same focal values for X/Y.

The truck dataset has different values and is a better one to test against.

Thanks for the fix!

The Windows build issue is unrelated to this PR.