patterncppMinor
Fluid Simulation with SDL
Viewed 0 times
withsdlsimulationfluid
Problem
I have always wanted to write a fluid simulation, and with the help of a paper and some StackOverflow users I've got something that works. My goal is to have a program that someone can run right away without having to install anything.
My plans:
Beginning Code
Math routines (If anyone can suggest a method to implement boundaries correctly in advect, please do so!)
```
// Bounds (currently a box with solid walls)
void set_bnd(const int b, vfloat &x, std::vector &bound)
{
for (int i=1; i &bound)
{
for (int k=0; k &bound)
{
float a = dtdiffN*N;
lin_solve(b, x, x0, a, 1+4*a+dt, bound); // Amazing fix due to Iwillnotexist Idonotexist
}
// Backwards advection
void advect(int b, vfloat &d, const vfloat &d0, const vfloat &u, const vfloat &v, float dt, std::vector &bound)
{
float dt0 = dt*N;
for (int i=1; iN+0.5) x=N+0.5;
int i0=(int)x; int i1=i0+1;
if (yN+0.5) y=N+0.5;
int j0=(int)y; int j1=j0+1;
float s1 = x-i0; float s0 = 1-s1; floa
My plans:
- Fix framerate (eventually)
- More configurable user input
- Have almost all constants configurable by the user at runtime, so not
const
- Have starting velocity and density outside the main loop
Beginning Code
#include
#include
#include
#include
#include
#include
typedef std::vector vfloat;
// Constants
const int SCREEN_WIDTH = 800;
const int SCREEN_HEIGHT = 800; // Should match SCREEN_WIDTH
const int N = 50; // Grid size
const int SIM_LEN = 3000; // Based on actual framerate
// Locks framerate at ~64, see stackoverflow.com/q/23258650/3163618
const std::chrono::milliseconds DELAY_LENGTH(5);
const float VISC = 0.01;
const float dt = 0.005;
const float DIFF = 0.01;
const bool DISPLAY_CONSOLE = false; // Console or graphics
const bool DRAW_GRID = false; // implement later
const bool DRAW_VEL = true;
const float MOUSE_DENS = 100.0;
// Code begins here
const int nsize = (N+2)*(N+2);
inline int IX(int i, int j){return i + (N+2)*j;}Math routines (If anyone can suggest a method to implement boundaries correctly in advect, please do so!)
```
// Bounds (currently a box with solid walls)
void set_bnd(const int b, vfloat &x, std::vector &bound)
{
for (int i=1; i &bound)
{
for (int k=0; k &bound)
{
float a = dtdiffN*N;
lin_solve(b, x, x0, a, 1+4*a+dt, bound); // Amazing fix due to Iwillnotexist Idonotexist
}
// Backwards advection
void advect(int b, vfloat &d, const vfloat &d0, const vfloat &u, const vfloat &v, float dt, std::vector &bound)
{
float dt0 = dt*N;
for (int i=1; iN+0.5) x=N+0.5;
int i0=(int)x; int i1=i0+1;
if (yN+0.5) y=N+0.5;
int j0=(int)y; int j1=j0+1;
float s1 = x-i0; float s0 = 1-s1; floa
Solution
I won't comment on the things you already mentioned you are planning to implement in the future, except that as Ilya Popov already said, you can use
Don't abbreviate names too much
Computers have enough RAM and storage space nowadays that you don't have to skimp on characters for variable and function names. Try to only use abbreviations that are so common that there is little chance that someone would not understand what is meant. While
If you do need to abbreviate a name to keep the code readable, such as with
Use descriptive names
Some variables don't make sense to me at all. They are not even abbreviations, just one-character placeholders. For example, in
Another example is
Use ALL_CAPS names only for macros
Using ALL_CAPS names is normally only used to warn the reader that something is not a regular variable or function, but a macro. Since you are declaring const variables, use lower case for all of them.
Don't put multiple statements on one line
It's easy to miss that a line consists of multiple statements. And especially when a line contains
Add spaces around operators, after commas
Whitespace around operators and between elements of a comma-separated lists really help readability of the code.
Use a code formatter
Try to use a code formatter such as
Make variables and functions
If a variable or function is only used in the same file as where it is declared, then make them
Use
Instead of calling
The proper way to do this is to do this in
Consider making a struct for all grid element properties
Each grid element has several properties, such as velocity, density, whether it is part of the boundary or not, and so on. You have split the grid into several arrays, one for each property. It is usually much better to then create a class or struct that describes all the properties of a grid element, and then make an single array out of that struct. This reduces the number of pointers you have to pass around to the various functions. And if you are dealing with vectors, you might want to make a struct for that too. For example:
Use a library for vectors and/or multidimensional arrays
You can make your life easier by using one of the many C++ libraries around that implement vector types and multidimensional arrays. The only issue is that there are many to choose from, each with their own pros and cons.
Don't mix stdio and iostream
In
SDL_RENDERER_PRESENTVSYNC to have SDL limit the frame rate for you.Don't abbreviate names too much
Computers have enough RAM and storage space nowadays that you don't have to skimp on characters for variable and function names. Try to only use abbreviations that are so common that there is little chance that someone would not understand what is meant. While
N is a common abbreviation for "number-of-things", and i is standard idiom for a loop iterator, someone who isn't familiar with fluid dynamics might not know that VISC is short for viscosity or DIFF for diffusion. Just spell them out. If names get very long, there might be good reasons to abbreviate. But resist removing just a few vowel here and there for no good reason; write set_bounds() instead of set_bnd(), and so on.If you do need to abbreviate a name to keep the code readable, such as with
IX(), then add a comment to explain what this function is supposed to do.Use descriptive names
Some variables don't make sense to me at all. They are not even abbreviations, just one-character placeholders. For example, in
set_bnd(), what is b? What is x? Even for a physicist or mathematician, x usually means the x-coordinate, so having this be a two-dimensional array is very confusing.Another example is
nsize. I know N is the grid size (it said so in the comments, and it's not too uncommon a name for a size, and we are simulating a grid here), the name nsize looks kind of redundant. But actually, it's the number of elements in the simulation grid with an added border. If there is no better name for it, describe this constant in a comment.Use ALL_CAPS names only for macros
Using ALL_CAPS names is normally only used to warn the reader that something is not a regular variable or function, but a macro. Since you are declaring const variables, use lower case for all of them.
Don't put multiple statements on one line
It's easy to miss that a line consists of multiple statements. And especially when a line contains
if-statements, it becomes hard to see what is part of the if-statement and what not.Add spaces around operators, after commas
Whitespace around operators and between elements of a comma-separated lists really help readability of the code.
Use a code formatter
Try to use a code formatter such as
indent, astyle or clang-format to automatically reformat your code. Have a look at the output of those tools with their default settings.Make variables and functions
static if possibleIf a variable or function is only used in the same file as where it is declared, then make them
static. This prevents the names of those variables and functions from clobbering the global namespace, and can help the compiler produce more efficient code.Use
SDL_PollEvent()Instead of calling
SDL_PumpEvents() and ignoring all events (you are just querying the mouse state), you should call SDL_PollEvent() process the events. Indeed, most events are not important for your program and you can ignore them, and in this case the mouse handling is probably fine, but at least handle SDL_QUIT events so the user can stop your program by closing the window.The proper way to do this is to do this in
process_input():SDL_Event event;
while (SDL_PollEvent(&event)) {
switch(event.type) {
case SDL_QUIT:
// signal the main loop to quit somehow
break;
default:
break;
}
}Consider making a struct for all grid element properties
Each grid element has several properties, such as velocity, density, whether it is part of the boundary or not, and so on. You have split the grid into several arrays, one for each property. It is usually much better to then create a class or struct that describes all the properties of a grid element, and then make an single array out of that struct. This reduces the number of pointers you have to pass around to the various functions. And if you are dealing with vectors, you might want to make a struct for that too. For example:
// A 2-dimensional vector
struct vec2 {
float x;
float y;
};
struct grid_element {
vec2 velocity = {};
float density = {};
bool bound = {};
};
std::vector grid(nsize);Use a library for vectors and/or multidimensional arrays
You can make your life easier by using one of the many C++ libraries around that implement vector types and multidimensional arrays. The only issue is that there are many to choose from, each with their own pros and cons.
Don't mix stdio and iostream
In
console_write(), you use both printf() and std::cout << "\n";. The problem is that printf() is a C stdio function, and std::cout is a C++ iostream. These are two different things, they can each have their own implementation of buffering. The resulCode Snippets
SDL_Event event;
while (SDL_PollEvent(&event)) {
switch(event.type) {
case SDL_QUIT:
// signal the main loop to quit somehow
break;
default:
break;
}
}// A 2-dimensional vector
struct vec2 {
float x;
float y;
};
struct grid_element {
vec2 velocity = {};
float density = {};
bool bound = {};
};
std::vector<grid_element> grid(nsize);Context
StackExchange Code Review Q#115240, answer score: 5
Revisions (0)
No revisions yet.