patterncsharpMinor
Water generating waves and reflection
Viewed 0 times
reflectionwavesgeneratingandwater
Problem
For the last few days I have been working on wave generation script. As there are a lot of calculations involved I was looking for possible optimizations to improve the performance of the code.
This specifically applies to the update function, as most of the calculations are done in here. As the full code is quite large, I made a pastebin of it which can be found over here.
```
void Update()
{
//Get sun reflection dir from sun object
if (Sun != null)
{
SunDirection = Sun.transform.forward;
OceanMaterial.SetVector("_SunDir", SunDirection);
}
if (this.IsRenderingReflection)
RenderObject();
OceanMaterial.SetTextureOffset("_Foam", Time.time * new Vector2(33,33));
var halfHeight = height / 2f;
var halfWidth = width / 2f;
var time = Time.time;
for (var y = 0; y 0 ? height - y : 0;
var nx = x > 0 ? width - x : 0;
waterHeightMatrix[idx] = h0[idx] coeffA + h0[width ny + nx].GetConjugate() * coeffB;
t_x[idx] = waterHeightMatrix[idx] new ComplexF(0.0f, vec_k.x) - waterHeightMatrix[idx] vec_k.y;
// Choppy wave calculations
if (x + y > 0)
waterHeightMatrix[idx] += waterHeightMatrix[idx] * vec_k.x / sqrtMagnitude;
}
}
Fourier.FFT2(waterHeightMatrix, width, height, FourierDirection.Backward);
Fourier.FFT2(t_x, width, height, FourierDirection.Backward);
// Get base values for vertices and uv coordinates.
if (baseHeight == null)
{
baseHeight = baseMesh.vertices;
vertices = new Vector3[baseHeight.Length];
normals = new Vector3[baseHeight.Length];
tangents = new Vector4[baseHeight.Length];
}
var wh = width * height;
var scaleA = ChoppyScale / wh;
var scaleB = WaveScale / wh;
var scaleBinv = 1.0f / scaleB;
for (var i = 0; i = geometryWidth)
{
tangents[item].w = tangents[geometryWidth * y].w;
This specifically applies to the update function, as most of the calculations are done in here. As the full code is quite large, I made a pastebin of it which can be found over here.
```
void Update()
{
//Get sun reflection dir from sun object
if (Sun != null)
{
SunDirection = Sun.transform.forward;
OceanMaterial.SetVector("_SunDir", SunDirection);
}
if (this.IsRenderingReflection)
RenderObject();
OceanMaterial.SetTextureOffset("_Foam", Time.time * new Vector2(33,33));
var halfHeight = height / 2f;
var halfWidth = width / 2f;
var time = Time.time;
for (var y = 0; y 0 ? height - y : 0;
var nx = x > 0 ? width - x : 0;
waterHeightMatrix[idx] = h0[idx] coeffA + h0[width ny + nx].GetConjugate() * coeffB;
t_x[idx] = waterHeightMatrix[idx] new ComplexF(0.0f, vec_k.x) - waterHeightMatrix[idx] vec_k.y;
// Choppy wave calculations
if (x + y > 0)
waterHeightMatrix[idx] += waterHeightMatrix[idx] * vec_k.x / sqrtMagnitude;
}
}
Fourier.FFT2(waterHeightMatrix, width, height, FourierDirection.Backward);
Fourier.FFT2(t_x, width, height, FourierDirection.Backward);
// Get base values for vertices and uv coordinates.
if (baseHeight == null)
{
baseHeight = baseMesh.vertices;
vertices = new Vector3[baseHeight.Length];
normals = new Vector3[baseHeight.Length];
tangents = new Vector4[baseHeight.Length];
}
var wh = width * height;
var scaleA = ChoppyScale / wh;
var scaleB = WaveScale / wh;
var scaleBinv = 1.0f / scaleB;
for (var i = 0; i = geometryWidth)
{
tangents[item].w = tangents[geometryWidth * y].w;
Solution
I'm not familiar with unity or simulation of water waves so just some generic remarks:
-
There are several variables used in your method which are not declared in your method but look like they are local variables such as
-
As mentioned the method is quite long and it's not easy to at least get a basic understanding of what all the different blocks do. I think it would help to possibly break out the different blocks into individual methods with descriptive names. Alternatively leave a descriptive comment before each block denoting what the next step is doing.
-
There is some code duplication in this block:
which can be shortened to:
Also you create
-
In the next loop here:
you repeat the term
and use
-
This is not really a good way to calculate
you are converting the
can be turned into
-
There are several variables used in your method which are not declared in your method but look like they are local variables such as
width, height, maxLOD. If these are in fact class members then the two common C# conventions are to either access them with this. or to prefix the names with _ - this way it will be easy to see by the reader that these variables are in fact class members. This is especially important as your method is very long and introduces a fair amount of local variables. Increases readability significantly for someone unfamiliar with your code (or for yourself when you come back to this in 12 month time trying to fix some obscure bug).-
As mentioned the method is quite long and it's not easy to at least get a basic understanding of what all the different blocks do. I think it would help to possibly break out the different blocks into individual methods with descriptive names. Alternatively leave a descriptive comment before each block denoting what the next step is doing.
-
There is some code duplication in this block:
var geoWidthHeight = geometryWidth * geometryHeight - 1;
for (var i = 0; i < geoWidthHeight; i++)
{
//Need to preserve w in refraction/reflection mode
if (!reflectionRefractionEnabled)
{
if (((i + 1) % geometryWidth) == 0)
{
tangents[i] = Vector3.Normalize((vertices[i - width + 1] + new Vector3(ChunkSize.x, 0.0f, 0.0f) - vertices[i]));
}
else
{
tangents[i] = Vector3.Normalize((vertices[i + 1] - vertices[i]));
}
tangents[i].w = 1.0f;
}
else
{
Vector3 tmp; // = Vector3.zero;
if (((i + 1) % geometryWidth) == 0)
{
tmp = Vector3.Normalize(vertices[i - width + 1] + new Vector3(ChunkSize.x, 0.0f, 0.0f) - vertices[i]);
}
else
{
tmp = Vector3.Normalize(vertices[i + 1] - vertices[i]);
}
tangents[i] = new Vector4(tmp.x, tmp.y, tmp.z, tangents[i].w);
}
}which can be shortened to:
var geoWidthHeight = geometryWidth * geometryHeight - 1;
for (var i = 0; i < geoWidthHeight; i++)
{
Vector3 tmp;
if (((i + 1) % geometryWidth) == 0)
{
tmp = Vector3.Normalize((vertices[i - width + 1] + new Vector3(ChunkSize.x, 0.0f, 0.0f) - vertices[i]));
}
else
{
tmp = Vector3.Normalize((vertices[i + 1] - vertices[i]));
}
//Need to preserve w in refraction/reflection mode
if (!reflectionRefractionEnabled)
{
tangents[i] = tmp;
tangents[i].w = 1.0f;
}
else
{
tangents[i] = new Vector4(tmp.x, tmp.y, tmp.z, tangents[i].w);
}
}Also you create
new Vector3(ChunkSize.x, 0.0f, 0.0f) on every loop iteration (which is width x height number of iterations). It's possible that either the compiler or the JIT can optimize it but I'd consider extracting it out of the loop and store it in a variable.-
In the next loop here:
//In reflection mode, use tangent w for foam strength
if (reflectionRefractionEnabled)
{
for (var y = 0; y < geometryHeight; y++)
{
for (var x = 0; x < geometryWidth; x++)
{
var item = x + geometryWidth * y;
...you repeat the term
geometryWidth * y several times. However this only changes with the outer loop so I'd consider extracting it into a stride variable like this:if (reflectionRefractionEnabled)
{
var stride = 0;
for (var y = 0; y < geometryHeight; y++, stride += geometryWidth)
{
for (var x = 0; x < geometryWidth; x++)
{
var item = x + stride;
...and use
stride in the inner loop wherever you used geometryWidth * y before. Again the JIT might perform this optimization but I wouldn't count on it.-
This is not really a good way to calculate
2^n:var den = (int)System.Math.Pow(2f, L0D);you are converting the
int L0D to float and then the result back to int. The classic way to calculate x * 2^n where n is int is to simply do x > n. So this var itemcount = (int)((height / den + 1) * (width / den + 1));can be turned into
var itemCount = ((height >> L0D) + 1) * ((width >> L0D) + 1);Code Snippets
var geoWidthHeight = geometryWidth * geometryHeight - 1;
for (var i = 0; i < geoWidthHeight; i++)
{
//Need to preserve w in refraction/reflection mode
if (!reflectionRefractionEnabled)
{
if (((i + 1) % geometryWidth) == 0)
{
tangents[i] = Vector3.Normalize((vertices[i - width + 1] + new Vector3(ChunkSize.x, 0.0f, 0.0f) - vertices[i]));
}
else
{
tangents[i] = Vector3.Normalize((vertices[i + 1] - vertices[i]));
}
tangents[i].w = 1.0f;
}
else
{
Vector3 tmp; // = Vector3.zero;
if (((i + 1) % geometryWidth) == 0)
{
tmp = Vector3.Normalize(vertices[i - width + 1] + new Vector3(ChunkSize.x, 0.0f, 0.0f) - vertices[i]);
}
else
{
tmp = Vector3.Normalize(vertices[i + 1] - vertices[i]);
}
tangents[i] = new Vector4(tmp.x, tmp.y, tmp.z, tangents[i].w);
}
}var geoWidthHeight = geometryWidth * geometryHeight - 1;
for (var i = 0; i < geoWidthHeight; i++)
{
Vector3 tmp;
if (((i + 1) % geometryWidth) == 0)
{
tmp = Vector3.Normalize((vertices[i - width + 1] + new Vector3(ChunkSize.x, 0.0f, 0.0f) - vertices[i]));
}
else
{
tmp = Vector3.Normalize((vertices[i + 1] - vertices[i]));
}
//Need to preserve w in refraction/reflection mode
if (!reflectionRefractionEnabled)
{
tangents[i] = tmp;
tangents[i].w = 1.0f;
}
else
{
tangents[i] = new Vector4(tmp.x, tmp.y, tmp.z, tangents[i].w);
}
}//In reflection mode, use tangent w for foam strength
if (reflectionRefractionEnabled)
{
for (var y = 0; y < geometryHeight; y++)
{
for (var x = 0; x < geometryWidth; x++)
{
var item = x + geometryWidth * y;
...if (reflectionRefractionEnabled)
{
var stride = 0;
for (var y = 0; y < geometryHeight; y++, stride += geometryWidth)
{
for (var x = 0; x < geometryWidth; x++)
{
var item = x + stride;
...var den = (int)System.Math.Pow(2f, L0D);Context
StackExchange Code Review Q#76979, answer score: 3
Revisions (0)
No revisions yet.