Conversation
exMachina316
commented
Sep 13, 2025
exMachina316
commented
- Optimized sim logic (not calling redundant functions)
- Visualization for pressure/divergence
- Introduced methods for normalized pressure and divergence retrieval. - Added pressure and divergence arrays to store simulation data. - Updated project method to enforce mass conservation. - Implemented shaders for pressure field visualization. - Enhanced main loop to toggle pressure field rendering.
There was a problem hiding this comment.
Pull Request Overview
This PR optimizes the Euler fluid simulation by removing redundant operations and adds pressure/divergence field visualization. The changes include streamlining the velocity step algorithm, adding pressure field rendering capabilities, and introducing a new FLIP (Fluid Implicit Particle) simulation implementation.
- Removed redundant noise injection and diffusion operations from the Euler simulation
- Added pressure and divergence field visualization with new shaders and keyboard controls
- Introduced complete FLIP simulation implementation with particle-based fluid dynamics
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main.cpp | Added pressure field visualization shaders, keyboard controls (P/D keys), and optimized rendering loop |
| src/fluid_sim.cpp | Streamlined velocity step by removing noise injection and diffusion, added pressure tracking |
| src/flip_sim.cpp | New FLIP simulation implementation with particle-based fluid dynamics |
| include/fluid_sim.h | Added pressure/divergence field accessors and removed unused constants |
| include/flip_sim.h | New header for FLIP simulation class and enums |
| CMakeLists.txt | Added flip_sim.cpp to build targets |
| .clang-format | Added Google-based code formatting configuration |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if (!success) | ||
| { | ||
| glGetShaderInfoLog(pressureFragmentShader, 512, NULL, infoLog); | ||
| std::cout << "ERROR::SHADER::DENSITY_FRAGMENT::COMPILATION_FAILED\n" |
There was a problem hiding this comment.
The error message incorrectly identifies the shader type as 'DENSITY_FRAGMENT' when it should be 'PRESSURE_FRAGMENT' since this is handling pressure fragment shader compilation errors.
| std::cout << "ERROR::SHADER::DENSITY_FRAGMENT::COMPILATION_FAILED\n" | |
| std::cout << "ERROR::SHADER::PRESSURE_FRAGMENT::COMPILATION_FAILED\n" |
| // Link density shaders | ||
| pressureShaderProgram = glCreateProgram(); |
There was a problem hiding this comment.
The comment incorrectly states 'Link density shaders' when the code is actually linking pressure shaders. This should be 'Link pressure shaders'.
| if (!success) | ||
| { | ||
| glGetProgramInfoLog(pressureShaderProgram, 512, NULL, infoLog); | ||
| std::cout << "ERROR::SHADER::DENSITY_PROGRAM::LINKING_FAILED\n" |
There was a problem hiding this comment.
The error message incorrectly identifies the program as 'DENSITY_PROGRAM' when it should be 'PRESSURE_PROGRAM' since this is handling pressure shader program linking errors.
| std::cout << "ERROR::SHADER::DENSITY_PROGRAM::LINKING_FAILED\n" | |
| std::cout << "ERROR::SHADER::PRESSURE_PROGRAM::LINKING_FAILED\n" |
| } | ||
| } | ||
|
|
||
| // Render density field if we have any vertices |
There was a problem hiding this comment.
The comment incorrectly states 'Render density field' when the code block is actually rendering the pressure field. This should be 'Render pressure field if we have any vertices'.
| // Render density field if we have any vertices | |
| // Render pressure field if we have any vertices |
| { | ||
| if (x >= 0 && x < width && y >= 0 && y < height) | ||
| { | ||
| return sqrt(pressure[x][y]/maxPressure); |
There was a problem hiding this comment.
Division by zero will occur if maxPressure is 0.0. This could happen when no pressure calculations have been performed yet or when all pressure values are zero.
| { | ||
| if (x >= 0 && x < width && y >= 0 && y < height) | ||
| { | ||
| return sqrt(divergence[x][y]/maxDivergence); |
There was a problem hiding this comment.
Division by zero will occur if maxDivergence is 0.0. This could happen when no divergence calculations have been performed yet or when all divergence values are zero.
| return sqrt(divergence[x][y]/maxDivergence); | |
| if (maxDivergence > 0.0001f) // Avoid division by zero | |
| { | |
| return sqrt(divergence[x][y] / maxDivergence); | |
| } |
| } | ||
|
|
||
| if (toGrid) { | ||
| int length = sizeof(f) / sizeof(float); |
There was a problem hiding this comment.
This calculation is incorrect. f is a pointer, so sizeof(f) returns the size of the pointer (typically 8 bytes), not the array size. This should use fNumCells_ instead of length.
| int length = sizeof(f) / sizeof(float); | |
| int length = fNumCells_; |
| void simulate(float dt, float gravity, float flipRatio, | ||
| int numPressureIters, int numParticleIters, | ||
| float overRelaxation, bool compensateDrift, | ||
| bool seperateParticles, float obstacleX, float obstacleY, |
There was a problem hiding this comment.
The parameter name 'seperateParticles' is misspelled. It should be 'separateParticles'.
| bool seperateParticles, float obstacleX, float obstacleY, | |
| bool separateParticles, float obstacleX, float obstacleY, |
