HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Water generating waves and reflection

Submitted by: @import:stackexchange-codereview··
0
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;

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 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.