Skip to content

Commit 636b34d

Browse files
OkladnojOkladnoj
committed
refactor(math): Address xezon review feedback on PR TheSuperHackers#2670
- Merge gmath.h include + USE_DETERMINISTIC_MATH into single __has_include block - Replace all #ifdef/#if defined() with #if USE_DETERMINISTIC_MATH - Remove TheSuperHackers @fix prefix from cmake comment - Expand ODR abbreviation in gamemath.cmake comment - Add blank lines after setFPMode() in benchmark - Fix iters abbreviation in printf - Simplify benchmark: remove replay dependency, auto-trigger at frame 400
1 parent afe36e5 commit 636b34d

6 files changed

Lines changed: 23 additions & 32 deletions

File tree

‎Core/GameEngine/Include/Common/Diagnostic/SimulationMathCrc.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
#pragma once
2020

2121
// Flags for diagnostic math benchmarks
22-
// #define RUN_MATH_BENCHMARK_REPLAY400_FLAG
22+
#define RUN_MATH_BENCHMARK_REPLAY400_FLAG (0)
2323

2424
class SimulationMathCrc
2525
{

‎Core/GameEngine/Source/Common/Diagnostic/SimulationMathCrc.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ void SimulationMathCrc::runBenchmark(int iterations)
111111
UnsignedInt crcDet = 0;
112112

113113
setFPMode();
114+
114115
for (i = 0; i < iterations; ++i)
115116
{
116117
XferCRC xfer;
@@ -127,6 +128,7 @@ void SimulationMathCrc::runBenchmark(int iterations)
127128
UnsignedInt crcNat = 0;
128129

129130
setFPMode();
131+
130132
for (i = 0; i < iterations; ++i)
131133
{
132134
XferCRC xfer;
@@ -139,7 +141,7 @@ void SimulationMathCrc::runBenchmark(int iterations)
139141
clock_t endNat = clock();
140142
double timeNatMs = (double)(endNat - startNat) / CLOCKS_PER_SEC * 1000.0;
141143

142-
printf("\n================ MATH BENCHMARK (%d iters) ================\n", iterations);
144+
printf("\n================ MATH BENCHMARK (%d iterations) ================\n", iterations);
143145
printf("Deterministic (WWMath): CRC = %08X, Time = %.2f ms\n", crcDet, timeDetMs);
144146
printf("Native (system math): CRC = %08X, Time = %.2f ms\n", crcNat, timeNatMs);
145147
printf("===========================================================\n\n");

‎Core/Libraries/Source/WWVegas/WWMath/wwmath.h‎

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -41,16 +41,10 @@
4141
#include <float.h>
4242
#include <assert.h>
4343

44-
// TheSuperHackers @fix VC6 does not support long long or <stdint.h> required by GameMath (gmath.h).
4544
// Use __has_include because wwmath.h is transitively included by targets that may not link gamemath.
46-
#if !(defined(_MSC_VER) && _MSC_VER < 1300)
4745
#if defined(__has_include) && __has_include("gmath.h")
4846
#include "gmath.h"
49-
#endif
50-
#endif
51-
52-
#if !(defined(_MSC_VER) && _MSC_VER < 1300)
53-
#define USE_DETERMINISTIC_MATH
47+
#define USE_DETERMINISTIC_MATH (1)
5448
#endif
5549

5650
/*
@@ -145,7 +139,7 @@ static WWINLINE float Fast_Asin(float val);
145139
static WWINLINE float Asin(float val);
146140

147141

148-
#ifdef USE_DETERMINISTIC_MATH
142+
#if USE_DETERMINISTIC_MATH
149143
static WWINLINE float Atan(float x) { return gm_atanf(x); }
150144
static WWINLINE float Atan2(float y,float x) { return gm_atan2f(y,x); }
151145
#else
@@ -155,7 +149,7 @@ static WWINLINE float Atan2(float y,float x) { return static_cast<float>(atan2(
155149

156150
// Trig wrappers: replace global Sin/Cos/Tan/ACos/ASin from deleted Trig.cpp.
157151
// Original Trig.cpp called CRT float functions (sinf, cosf, etc.).
158-
#ifdef USE_DETERMINISTIC_MATH
152+
#if USE_DETERMINISTIC_MATH
159153
static WWINLINE float SinTrig(float x) { return gm_sinf(x); }
160154
static WWINLINE float CosTrig(float x) { return gm_cosf(x); }
161155
static WWINLINE float TanTrig(float x) { return gm_tanf(x); }
@@ -174,7 +168,7 @@ static WWINLINE float Atan2(float y,float x) { return static_cast<float>(atan2(
174168
// Note: double overloads narrow to float before calling GameMath (gm_*f).
175169
// GameMath only provides float-precision functions. All call sites pass float-width
176170
// values, so the narrowing is lossless in practice.
177-
#ifdef USE_DETERMINISTIC_MATH
171+
#if USE_DETERMINISTIC_MATH
178172
static WWINLINE double SqrtOrigin(double x) { return (double)gm_sqrtf((float)x); }
179173
static WWINLINE float SqrtfOrigin(float x) { return gm_sqrtf(x); }
180174
static WWINLINE double Atan2Origin(double y, double x) { return (double)gm_atan2f((float)y, (float)x); }
@@ -225,6 +219,7 @@ static WWINLINE float Atan2(float y,float x) { return static_cast<float>(atan2(
225219
static WWINLINE float CoshfOrigin(float x) { return coshf(x); }
226220
static WWINLINE float TanhfOrigin(float x) { return tanhf(x); }
227221
#endif
222+
228223
static WWINLINE float Sign(float val);
229224
static WWINLINE float Ceil(float val) { return ceilf(val); }
230225
static WWINLINE float Floor(float val) { return floorf(val); }
@@ -403,7 +398,7 @@ WWINLINE bool WWMath::Is_Valid_Double(double x)
403398
// Float to long
404399
// ----------------------------------------------------------------------------
405400

406-
#if defined(USE_DETERMINISTIC_MATH)
401+
#if USE_DETERMINISTIC_MATH
407402
WWINLINE long WWMath::Float_To_Long(float f)
408403
{
409404
return gm_lrintf(f);
@@ -429,7 +424,7 @@ WWINLINE long WWMath::Float_To_Long(float f)
429424

430425
WWINLINE long WWMath::Float_To_Long(double f)
431426
{
432-
#if defined(USE_DETERMINISTIC_MATH)
427+
#if USE_DETERMINISTIC_MATH
433428
return gm_lrint(f);
434429
#elif defined(_MSC_VER) && defined(_M_IX86)
435430
long retval;
@@ -445,7 +440,7 @@ WWINLINE long WWMath::Float_To_Long(double f)
445440
// Cos
446441
// ----------------------------------------------------------------------------
447442

448-
#if defined(USE_DETERMINISTIC_MATH)
443+
#if USE_DETERMINISTIC_MATH
449444
WWINLINE float WWMath::Cos(float val)
450445
{
451446
return gm_cosf(val);
@@ -472,7 +467,7 @@ WWINLINE float WWMath::Cos(float val)
472467
// Sin
473468
// ----------------------------------------------------------------------------
474469

475-
#if defined(USE_DETERMINISTIC_MATH)
470+
#if USE_DETERMINISTIC_MATH
476471
WWINLINE float WWMath::Sin(float val)
477472
{
478473
return gm_sinf(val);
@@ -623,7 +618,7 @@ WWINLINE float WWMath::Fast_Acos(float val)
623618

624619
WWINLINE float WWMath::Acos(float val)
625620
{
626-
#ifdef USE_DETERMINISTIC_MATH
621+
#if USE_DETERMINISTIC_MATH
627622
return gm_acosf(val);
628623
#else
629624
return (float)acos(val);
@@ -664,7 +659,7 @@ WWINLINE float WWMath::Fast_Asin(float val)
664659

665660
WWINLINE float WWMath::Asin(float val)
666661
{
667-
#ifdef USE_DETERMINISTIC_MATH
662+
#if USE_DETERMINISTIC_MATH
668663
return gm_asinf(val);
669664
#else
670665
return (float)asin(val);
@@ -675,7 +670,7 @@ WWINLINE float WWMath::Asin(float val)
675670
// Sqrt
676671
// ----------------------------------------------------------------------------
677672

678-
#if defined(USE_DETERMINISTIC_MATH)
673+
#if USE_DETERMINISTIC_MATH
679674
WWINLINE float WWMath::Sqrt(float val)
680675
{
681676
return gm_sqrtf(val);
@@ -728,7 +723,7 @@ WWINLINE int WWMath::Float_To_Int_Floor (const float& f)
728723
// Inverse square root
729724
// ----------------------------------------------------------------------------
730725

731-
#if defined(USE_DETERMINISTIC_MATH)
726+
#if USE_DETERMINISTIC_MATH
732727
WWINLINE float WWMath::Inv_Sqrt(float val)
733728
{
734729
return 1.0f / gm_sqrtf(val);

‎GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3739,19 +3739,13 @@ void GameLogic::update()
37393739
TheTerrainLogic->UPDATE();
37403740
}
37413741

3742-
#ifdef RUN_MATH_BENCHMARK_REPLAY400_FLAG
3743-
static int s_replayStartFrame = -1;
3742+
#if RUN_MATH_BENCHMARK_REPLAY400_FLAG
37443743
static bool s_benchmarkRun = false;
37453744

3746-
if (!s_benchmarkRun && TheRecorder && TheRecorder->isPlaybackMode())
3745+
if (!s_benchmarkRun && m_frame == 400)
37473746
{
3748-
if (s_replayStartFrame == -1) {
3749-
s_replayStartFrame = m_frame;
3750-
}
3751-
else if (m_frame == s_replayStartFrame + 400) {
3752-
SimulationMathCrc::runBenchmark(10000);
3753-
s_benchmarkRun = true;
3754-
}
3747+
SimulationMathCrc::runBenchmark(10000);
3748+
s_benchmarkRun = true;
37553749
}
37563750
#endif
37573751

‎cmake/compilers.cmake‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ if (NOT IS_VS6_BUILD)
5151
add_compile_options(/Zc:__cplusplus)
5252
else()
5353
add_compile_options(-Wsuggest-override)
54-
# TheSuperHackers @fix Prevent FMA contraction (a*b+c -> fmadd) which skips intermediate
54+
# Prevent FMA contraction (a*b+c -> fmadd) which skips intermediate
5555
# rounding and breaks cross-platform deterministic math parity with MSVC (/fp:precise).
5656
add_compile_options(-ffp-contract=off)
5757
endif()

‎cmake/gamemath.cmake‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,5 +12,5 @@ FetchContent_Declare(
1212
FetchContent_MakeAvailable(gamemath)
1313

1414
# Ensure GameMath includes are available to ALL targets
15-
# to prevent ODR violations and ensure USE_DETERMINISTIC_MATH activates consistently.
15+
# to prevent one-definition-rule violations and ensure USE_DETERMINISTIC_MATH activates consistently.
1616
include_directories(${gamemath_SOURCE_DIR}/include)

0 commit comments

Comments
 (0)