Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document fog functions Gfx_SetFog(2) and Play_SetFog #1922

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+35 −4
Open

Document fog functions Gfx_SetFog(2) and Play_SetFog #1922

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/code/z_play.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,10 @@ void func_800BC88C(PlayState* this) {
this->transitionCtx.transitionType = -1;
}

/**
* 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.
Comment on lines +177 to +178
Copy link
Collaborator

@Dragorn421 Dragorn421 Mar 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

forgive me but I find this comment somewhat clunky, I tried to rewrite it more to my liking

 * 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,
this->lightCtx.fogNear, 1000);
Expand Down
35 changes: 31 additions & 4 deletions src/code/z_rcp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -847,28 +847,50 @@ Gfx gEmptyDL[] = {
gsSPEndDisplayList(),
};

/**
* 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.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* 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.

it is "some z"-linear but not view space z linear when using perspective projection (I forgot the details (if I ever grasped them)), anyway so I don't think specifying linear is all that useful

*
* 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this only ensures that near != far, right?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it ensures that far is greater than near only in the case where they are initially the same. like, the important bit that tharo is stressing here is that far gets incremented by 1 to be bigger.
but its prob good to clarify that in the wording

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Ensure fog far is greater than near
// Avoid 0 divisor in gSPFogPosition below

if (far == near) {
far++;
}

ASSERT(near != far, "n != f", "../z_rcp.c", 1155);

// Set the fog color, far away geometry will be rendered as this solid color.
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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// 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

I wouldn't write anything on why, this whole function is sloppy without good "why"s anyway

gSPFogFactor(gfx++, 0, 0);
} else if (near >= 997) {
gSPFogFactor(gfx++, 0x7FFF, 0x8100);
} 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)
Comment on lines +873 to +875
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest moving details to the general case else { below and making this part lighter

Suggested change
// 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)
// 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); // Fixed point: 0x7FFF = 1.0, -0x7F00 = -0.992
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fm and fo are s0.15 ? Where is this from
Seems inconsistent with the formula
fogAlpha=fm*z+fo z in [-1;1] fogAlpha in [0;255]
(cf gbi.h)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

f(z) = clamp(32767 * z - 32512, 0, 255) for z in [-1,1] produces the curve (only the interesting part shown)

image

32512 is chosen so that f(z)=255 at z=1

There's some weirdness in how the microcode actually computes this since z itself isn't really in the interval [-1,1], it's in s15.16 format. The calculation is something like

vmudh $v__, fo, [1,1,1,1,1,1,1,1] // ACC = (fo * [1,1,1,1,1,1,1,1]) << 16
vmadh $v__, [0, 0, 0, 1, 0, 0, 0, 1], v31_0x7F00 // ACC += ([0,0,0,1,0,0,0,1] * 0x7F00) << 16
vmadn $v26, pos, fm // frac. part, $v26 = low16(ACC += pos * fm)
vmadh $v25, pos, fm // int. part, $v25 = mid16(ACC += (pos * fm) << 16)

https://github.com/Mr-Wiseguy/f3dex2/blob/master/f3dex2.s#L1432
I have yet to take the time to carefully check the units involved here

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then if to interpret 0x7FFF and the other as s0.15 you rely on interpreting z as s15.16 that should be written somewhere too, otherwise it's kinda arbitrary/useless to use any unit

} else if (near < 0) {
// Fog near is out of range. Set a constant shade alpha of 255 for fully fogged
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// 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.
Comment on lines +882 to +883
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//! @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;
}

/**
* Like Gfx_SetFog but issues a pipesync before changing fog color.
*
* @see Gfx_SetFog
*/
Gfx* Gfx_SetFogWithSync(Gfx* gfx, s32 r, s32 g, s32 b, s32 a, s32 near, s32 far) {
if (far == near) {
far++;
Expand All @@ -891,6 +913,11 @@ Gfx* Gfx_SetFogWithSync(Gfx* gfx, s32 r, s32 g, s32 b, s32 a, s32 near, s32 far)
return gfx;
}

/**
* Wrapper for Gfx_SetFog
*
* @see Gfx_SetFog
*/
Gfx* Gfx_SetFog2(Gfx* gfx, s32 r, s32 g, s32 b, s32 a, s32 near, s32 far) {
return Gfx_SetFog(gfx, r, g, b, a, near, far);
}
Expand Down