From f968f873beef1d5d788cc2cc0c95aaa9fa57817e Mon Sep 17 00:00:00 2001 From: Dragorn421 Date: Sat, 7 Jun 2025 01:00:04 +0200 Subject: [PATCH] review --- src/code/z_play.c | 4 ++-- src/code/z_rcp.c | 20 ++++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/code/z_play.c b/src/code/z_play.c index b903c1f03b..7d13d7f061 100644 --- a/src/code/z_play.c +++ b/src/code/z_play.c @@ -225,8 +225,8 @@ void func_800BC88C(PlayState* this) { } /** - * Set the default fog state controlled by the environment system. - * If a custom fog state is used in rendering it is expected that it be reset back to this default state. + * Set the environment fog, from parameters controlled by the environment system. + * If a custom fog state is momentarily used in drawing, the environment fog is expected to be restored afterwards. */ Gfx* Play_SetFog(PlayState* this, Gfx* gfx) { return Gfx_SetFog2(gfx, this->lightCtx.fogColor[0], this->lightCtx.fogColor[1], this->lightCtx.fogColor[2], 0, diff --git a/src/code/z_rcp.c b/src/code/z_rcp.c index 858ac6573f..9bfb9b6cc3 100644 --- a/src/code/z_rcp.c +++ b/src/code/z_rcp.c @@ -858,13 +858,13 @@ Gfx gEmptyDL[] = { * Set fog color and range. * * At or prior to fog near, geometry is unaffected by fog. At or beyond fog far, geometry is fully fogged. - * Between near and far pixels will be linearly interpolated between the unfogged color and the supplied fog color. + * Between near and far the geometry color will be interpolated between the unfogged color and the supplied fog color. * * Fog far should be in the range 0 to 1000 and greater than or equal to fog near. If fog near is negative everything * will be fully fogged. If fog near is 1000 or greater there is no fog. */ Gfx* Gfx_SetFog(Gfx* gfx, s32 r, s32 g, s32 b, s32 a, s32 near, s32 far) { - // Ensure fog far is greater than near + // Avoid 0 divisor in gSPFogPosition below if (far == near) { far++; } @@ -874,20 +874,20 @@ Gfx* Gfx_SetFog(Gfx* gfx, s32 r, s32 g, s32 b, s32 a, s32 near, s32 far) { gDPSetFogColor(gfx++, r, g, b, a); if (near >= 1000) { - // Fog far is expected to be at most 1000 so near >= far. Set a constant shade alpha of 0 for no fog + // Set a constant shade alpha of 0 for no fog gSPFogFactor(gfx++, 0, 0); } else if (near > 996) { - // Calculating the fog multiplier when far = 1000 and near >= 997 using `128000 / (far - near)` as in - // SPFogPosition would result in an overflow since 128000 / (1000 - 997) > 32767. Avoid this overflow by - // effectively clamping near to ~996. This is almost SPFogPosition(996.0937f, 1000) - gSPFogFactor(gfx++, 0x7FFF, -0x7F00); // Fixed point: 0x7FFF = 1.0, -0x7F00 = -0.992 + // Avoid an overflow when near and far are close (see bug below), by effectively clamping near to ~996. + // This is almost SPFogPosition(996.0937f, 1000) + gSPFogFactor(gfx++, 0x7FFF, -0x7F00); } else if (near < 0) { - // Fog near is out of range. Set a constant shade alpha of 255 for fully fogged + // Set a constant shade alpha of 255 for fully fogged gSPFogFactor(gfx++, 0, 255); } else { // Normal range. Shade alpha is 0 at z <= near and 255 at z >= far, linearly interpolated in between. - //! @bug If the difference between near and far is not at least 4, the fog multiplier will overflow. This is - //! handled above when fog far is 1000 but for closer fog far it is not accounted for. + //! @bug If far - near < 4, the computed `fm` fog factor coefficient will overflow. + //! For example: 128000 / (983 - 980) > 32767 + //! This is handled above in the case of near > 996, but the general case is not accounted for. gSPFogPosition(gfx++, near, far); } return gfx;