HomeWildfire Games

Commit Detail
AuditedrP2519

Description

Details

Auditors
elexis
Committed
mateiJul 23 2005, 9:53 AM
Parents
rP2518: Renamed the GUI object functions (probably missed a few calls), and made first…
Branches
Unknown
Tags
Unknown

Event Timeline

elexis raised a concern with this commit.Oct 19 2017, 6:27 PM
elexis added a subscriber: elexis.

The Smoothing behaves differently at the map border and can be simplified by removing magic numbers.

(Commenting on this commit, because the logic is still exactly the same as of rP20313 (rP9096 only translated the SmoothElevationPainter to JS).)

Terms:
The Smoothing Area of a given vertex designates the set of vertices that contains this vertex and the vertices adjacent to that vertex.
SUM(term[i]) designates the sum of that term over all vertices i of the Smoothing Area.

Observation 1 (number of neighbors):
The number of vertices in the Smoothing Area (vertex_count_smoothing_area) of a given vertex is
4 if the current vertex is a corner,
6 if the current vertex is on the edge of the border and
9 if the current vertex is not at the map border.

Observation 2 (current formula):
The smoothened elevation determined by the code is:
(target_elevation * 8 + SUM(elevation[i])) / (8 + vertex_count_smoothing_area).

Observation 3 (target elevation gravity depends on location):
With the formula of the current code, the ratio of the weight of the target elevation to the weight of the elevation of the vertices of the Smoothing Area depends on the location of the vertex.
I.e. the the target height will influence much more at the map border.

Corner: (target_elevation * 8 + 4 elevations)) / (8 + 4).
Inside: (target_elevation * 8 + 9 elevations)) / (8 + 9).

So the target elevation has a gravity of 2 on the corner and a gravity of 1 inside the map.
Sounds wrong to me.

Observation 4 (magic-free averaging of planes):
Consider vertices that aren't at the border of the heightmap grid.
If the number 8 in the formula is changed to vertex_count_smoothing_area,
then the smoothened elevation is
(target_elevation * vertex_count_smoothing_area + SUM(elevation[i])) / (2 * vertex_count_smoothing_area)
== (target_elevation + SUM(elevation[i]) / vertex_count_smoothing_area) / 2.
== AVERAGE(target_elevation, AVERAGE(smoothing area))

Hypothesis:
This equation
(1) is similar enough to speculate wheather it was originally intended
(2) simpler
(3) contains no magic weights
(4) doesn't weigh the target height differently at the map border

/ps/trunk/source/tools/rmgen/smoothelevationpainter.cpp
124

See smoothing code here.

This commit now has outstanding concerns.Oct 19 2017, 6:27 PM
elexis added inline comments.Oct 19 2017, 8:43 PM
/ps/trunk/source/tools/rmgen/smoothelevationpainter.cpp
77

(Copied from the LayeredPainter` in rP2378 instead of adding a helper as complained in rP9096)

104

(The three if-else statements can become two if-statement without else.)

elexis accepted this commit.Oct 26 2017, 3:33 PM

Removal of weighting constants in rP20353.

I'm twelve years too late to the party, but thanks for the awesome groundwork matei and much success in the bigdata world in case you ever read this!
Consider the code well audited.

/ps/trunk/source/tools/rmgen/smoothelevationpainter.cpp
77

Unified in rP20350.

104

Unified in rP20350.

All concerns with this commit have now been addressed.Oct 26 2017, 3:33 PM