Cubitect / cubiomes

C library that mimics the Minecraft biome generation.
MIT License
556 stars 98 forks source link

Suggestion: Move unchanging climate/octave values to a one-time initializing function #82

Closed Nel-S closed 1 year ago

Nel-S commented 1 year ago

At the current moment, cubiomes, when initializing the BiomeNoise's climate and octave structures via init_climate_seed() and xOctaveInit(), must recompute every single attribute each time: the climate amplitudes, the octave counts and start positions of each octave, and each octave's amplitude, lacunarity, and the a, b, c, and 512 d values. However, of these, only the a, b, c, and d values of each octave actually change between seeds: every other value is constant due to them being completely independent of the world seed (or in the case of lacunarity, only depending on whether Large Biomes is enabled or not). As such, I would imagine it would be far more efficient to simply initialize those attributes once, and then have xOctaveInit only focus on generating the a, b, c, and d values afterward.

(A possible implementation of this might be something akin to splitting the Xoroshiro manipulations that generate a, b, c, and d into their own function--e.g. applySeedToOctaves(xlo, xhi, large)--and having that take the place of init_climate_seed in SetBiomeSeed/SetClimateParaSeed, while init_climate_seed and the remnants of xOctaveInit can be moved into the one-time initializer initBiomeNoise. I suppose one could also just create a lookup table for all of the values, but that obviously wouldn't be terribly future-proof. Of course, feel free to implement whatever you think is best.)

To me, the primary drawback of such an implementation would be that the md5_octave_n lookup table would have to be duplicated in two separate functions---one in the reduced xOctaveInit for creating the amplitudes/lacunarities, one in the new applySeedToOctaves for creating the a-d values. But given this is already an unmodifiable constant, I personally think this is worth the speedup we would get from not forcing setBiomeSeed to recalculate the same 122 values every time, given the function effectively has to be called anew every single time cubiomes examines a new seed, to my knowledge.

Cubitect commented 1 year ago

I'm not sure I can see the optimization potential. The a, b, c and 512 d values account for pretty much the entire computational expense for the octave initialization. The overhead for the floating point arithmetic for the amplitude and lacunarity was around 1-2%, and I was able to reduce this to pretty much zero by replacing the pow() calls with a lookup table. So I believe there to be no benefit to moving the amplitude and lacunarity initialization to another function.