New lighting engine #137

Thar0 · 2024-06-16T23:14:35Z

Features

Microcode point lights for both F3DEX2 and F3DEX3.
Makes fake point lights (directional lights whose direction is recomputed every frame) more consistent with F3DEX2 microcode point lights. This type of light is no longer used when F3DEX3 is enabled as we can always use microcode point lights.
A priority system is used to select which lights to bind when there are more than G_MAX_LIGHTS (7 in F3DEX2, 9 in F3DEX3) registered with the system. Directional lights have highest priority, point lights are prioritized based on distance to a reference position. Vanilla would just take the most recent 7 lights registered and ignore the rest.
New functions for setting point light attenuations directly (kc, kl, kq), and a new function for setting the F3DEX3 specular parameter directly. The vanilla functions that only take a light radius use a fixed model to convert this radius to these parameters.
If lights were bound but nothing was drawn with them, Lights_Pop() can delete the queued lighting commands to avoid loading them.
Binding lights will check that it's not submitting an identical lighting configuration to the previous call, also reducing the number of times lights are loaded. The previous light state is invalidated per-frame and whenever s2dex2 is loaded, since both of these invalidate the microcode lighting state. It is also invalidated when lights were popped from the previous call.
Fixes a vanilla bug where switching rooms did not delete the lights for the previous room, eventually causing the light list to fill up completely.

Since F3DEX3 requires no additional cooperation from geometry drawn with point lights, F3DEX3 can always use point lights. For F3DEX2, a new room behavior field has been added that determines whether the room geometry is compatible with point lights. For actors, I've added a new actor flag which communicates that the geometry drawn by the actor is point-lighting compatible. At least the room behavior change will need fast64 integration.

This lighting engine is based on a lighting engine rewrite featured in the Team Dampé's Hut Escape Room competition submission (https://github.com/Dampes-Hut/escaperoom), as well as a couple of other projects in various forms. Special thanks to @Dragorn421 for contributions to that lighting engine that made it in here as well.

Dragorn421 · 2024-06-17T11:58:11Z

src/code/z_lights.c

+ * If we let POINT_LIGHT_R0=3000, the attenuation I(d) will have the characteristics
+ *      I(r/4) ~ 0.5I
+ *      I(r/2) ~ 0.2I
+ *      I(r)   ~ 0.06I


Probably should also mention this is only valid for r (the light radius) in a certain range (which I forgot but is documented somewhere in the escaperoom repo's z_lights)

Edit: or at least on the case of f3dex2, f3dex3 and its better idea of having a floating point value may be way better about it

sauraen

One overall comment: Because of how much slower point lights are than directional lights in both EX3 and EX2, and because real point lights require the scene to be triangulated a bit more in order to look good (won't always look good in vanilla scenes with gigantic tris), I think it should be a config option for whether the user wants fake or real point lights.

Generally I think this is cool and I'm looking forward to trying it.

sauraen · 2024-06-19T16:29:55Z

include/macros.h

+// assertf, print a message before abort
+#if IS_DEBUG
+# define assertf(cond, fmt, ...)                    \
+    do {                                            \
+        if (!(cond)) {                              \
+            PRINTF(fmt, __VA_ARGS__);               \
+            __assert(#cond, __FILE__, __LINE__);    \
+        }                                           \
+    } while (0)
+#else
+# define assertf(cond, fmt, ...) ((void)0)
+#endif
+


Isn't there some sort of assert system in vanilla?

This is essentially vanilla asserts with a console message attached. It's just a convenience macro for providing some extra information only when the assert is triggered.

Okay, I think it would be best if new debugging systems were not introduced in unrelated PRs, but if there was really nothing in vanilla that covers this use case, go ahead.

It's here because I needed it to debug this PR, it feels counterproductive to remove it after the fact especially if there are still obscure double-frees lurking in vanilla code since they're a pain to debug without the extra info this macro provides. If we're really concerned about clean history, I can make a new PR and stall this one until that one makes it in.

sauraen · 2024-06-19T16:30:24Z

include/functions.h

-void Lights_PointSetInfo(LightInfo* info, s16 x, s16 y, s16 z, u8 r, u8 g, u8 b, s16 radius, s32 type);
-void Lights_PointNoGlowSetInfo(LightInfo* info, s16 x, s16 y, s16 z, u8 r, u8 g, u8 b, s16 radius);
-void Lights_PointGlowSetInfo(LightInfo* info, s16 x, s16 y, s16 z, u8 r, u8 g, u8 b, s16 radius);
-void Lights_PointSetColorAndRadius(LightInfo* info, u8 r, u8 g, u8 b, s16 radius);
-void Lights_DirectionalSetInfo(LightInfo* info, s8 x, s8 y, s8 z, u8 r, u8 g, u8 b);
-void Lights_Reset(Lights* lights, u8 ambentR, u8 ambentG, u8 ambentB);
-void Lights_Draw(Lights* lights, GraphicsContext* gfxCtx);
-void Lights_BindAll(Lights* lights, LightNode* listHead, Vec3f* vec);
-void LightContext_Init(PlayState* play, LightContext* lightCtx);
-void LightContext_SetAmbientColor(LightContext* lightCtx, u8 r, u8 g, u8 b);
-void LightContext_SetFog(LightContext* lightCtx, u8 r, u8 g, u8 b, s16 fogNear, s16 zFar);
-Lights* LightContext_NewLights(LightContext* lightCtx, GraphicsContext* gfxCtx);
-void LightContext_InitList(PlayState* play, LightContext* lightCtx);
-void LightContext_DestroyList(PlayState* play, LightContext* lightCtx);
-LightNode* LightContext_InsertLight(PlayState* play, LightContext* lightCtx, LightInfo* info);
-void LightContext_RemoveLight(PlayState* play, LightContext* lightCtx, LightNode* node);
-Lights* Lights_NewAndDraw(GraphicsContext* gfxCtx, u8 ambientR, u8 ambientG, u8 ambientB, u8 numLights, u8 r, u8 g,
-                          u8 b, s8 x, s8 y, s8 z);
-Lights* Lights_New(GraphicsContext* gfxCtx, u8 ambientR, u8 ambientG, u8 ambientB);
-void Lights_GlowCheck(PlayState* play);
-void Lights_DrawGlow(PlayState* play);


I would expect new functions to be added to this file for the new functionality in this PR.

I've removed all the lighting functions from here, since various functions no longer exist.

include/ultra64/gbi.f3dex3.h

sauraen · 2024-06-19T16:32:56Z

include/ultra64/gbi.h

@@ -11,7 +11,20 @@
        #define NO_SYNCS_IN_TEXTURE_LOADS
    #endif
    #include "gbi.f3dex3.h"
+    #define G_MAX_LIGHTS 9


G_MAX_LIGHTS is already in the F3DEX3 GBI.

sauraen · 2024-06-19T16:33:44Z

include/ultra64/gbi.h

+    unsigned char kl;       /* linear attenuation Kl */
+    short pos[3];           /* light position x, y, z in world space */
+    unsigned char kq;       /* quadratic attenuation Kq */
+    unsigned char size;     /* For specular only; reasonable values are 1-4 */


The size field does not exist in F3DEX2 point lighting.

sauraen · 2024-06-19T16:35:36Z

include/ultra64/gbi.h

-    Light_t l;
+    Light_t    l;
+#ifdef F3DEX_GBI_PL
+    PosLight_t p;
+#endif


Generally I think one of the two existing community F3DEX2 gbi.h's which support point lights should be brought in, instead of making a new GBI implementation of point lights.

If you can share these existing community gbi.h implementations I can look at what they do, but my predisposition is that I'm not especially interested in using an old implementation when this implementation is already correct and does what we want.

This is the community's pos light GBI for use with the SGI original GBI: https://github.com/HackerN64/HackerSM64/blob/master/include/n64/PR/gbi-poslight.h This is what I based the F3DEX3 pos light GBI on.

This is the community's reverse-engineered (non SGI) implementation: https://github.com/z64tools/z64hdr/blob/main/include/ultra64/gbi.h

Out of curiosity, I checked what the MM decomp uses (I don't think this existed when F3DEX3 was being started): https://github.com/zeldaret/mm/blob/main/include/PR/gbi.h They do have the union of the light type and the point light type called Light, but they also have the point light type named PointLight_t instead of PosLight_t.

If you think there is a good reason to switch away from the first community implementation already in use (for example, easier use of F3DEX3 in MM in the future), I'm open to it, but we should switch to some other standard community implementation, not make something up just for this repo.

That first header in HackerSM64 is very suspicious. This comment /* version 990705 */ suggests to me that this is probably a leaked header being appropriated as a community header. I personally have no wish to import this into HackerOoT.

The z64hdr gbi is glank's gbi with an added structure. The new geometry mode is altogether missing and the LightPos_t structure has poor field names. This implementation is simply not up to standard.

Of these choices, MM decomp's is far and away the best choice but it's still not as complete as this PR's. I'd rather upstream changes to MM's gbi.h than adopt any of these.

sauraen · 2024-06-19T16:37:11Z

include/z64light.h

-} Lights; // size = 0x80
+    u8 numLights;
+    f32 distances[G_MAX_LIGHTS];
+    Lightsn l;


Should be PosLightsn

sauraen · 2024-06-19T16:41:00Z

include/z64light.h

+void Lights_PointSetInfo(LightInfo* info, s16 x, s16 y, s16 z, u8 r, u8 g, u8 b, s16 radius, s32 type);
+void Lights_PointNoGlowSetInfo(LightInfo* info, s16 x, s16 y, s16 z, u8 r, u8 g, u8 b, s16 radius);
+void Lights_PointGlowSetInfo(LightInfo* info, s16 x, s16 y, s16 z, u8 r, u8 g, u8 b, s16 radius);
+void Lights_PointSetColorAndRadius(LightInfo* info, u8 r, u8 g, u8 b, s16 radius);
+void Lights_PointSetPosition(LightInfo* info, s16 x, s16 y, s16 z);
+void Lights_PointSetAttenuation(LightInfo* info, u8 kc, u8 kl, u8 kq);
+void Lights_PointSetSpecularSize(LightInfo* info, u8 size);
+void Lights_DirectionalSetInfo(LightInfo* info, s8 x, s8 y, s8 z, u8 r, u8 g, u8 b);
+void Lights_DirectionalSetSpecularSize(LightInfo* info, u8 size);
+
+//  Light Context
+void LightContext_Init(struct PlayState* play, LightContext* lightCtx);
+void LightContext_DestroyList(struct PlayState* play, LightContext* lightCtx);


Function declarations should always be in functions.h; the headers for each subsystem should only include the struct definitions. Note that you have had to forward declare PlayState and use struct PlayState as a result of putting them here, which is ugly and does not match the code style in this repo.

This is not the case for upstream and shouldn't be the case here either. Eventually functions.h will cease to exist as more work is done upstream to move functions and variables to their respective header files. Since I removed and added a bunch of functions, I felt I should just move these now.

I think PRs to this project should stick to the code standards of HackerOoT currently, not the code standards that upstream may be moving to in the future. Personally I think putting functions with each subsystem is nicer organization, but I also think writing struct PlayState everywhere is ugly and a major regression in terms of how modern the code looks. But anyway I will not block the PR over this.

Some headers in this project have already adopted these changes as they're being rolled out incrementally upstream. See irqmgr.h, sched.h, padmgr.h, tha.h, thga.h, and probably others (these are just the ones I've already done this for upstream). There's little point in waiting for upstream in this case, it's eventually going to cover all headers in this way.

sauraen · 2024-06-19T16:44:49Z

src/code/z_lights.c

+#if !(ENABLE_F3DEX3 || defined(F3DEX_GBI_PL))
+#error A microcode that supports point lighting is required
+#endif


What? The options in HackerOoT are F3DEX3 or F3DEX2 non point lighting. F3DEX2 point lighting is not supported and even if it was, the game has to work with F3DEX2 non point lighting too.

The F3DEX2 version in gamecube versions of OoT has point lighting support, so F3DEX2 with point lighting is fully supported with these engine changes. I dropped support for non-point-light F3DEX2 since it was never used in the first place and I didn't see much need for it, but I need to think about this again if we're worried about performance implications of always using ucode point lights.

I dropped support for non-point-light F3DEX2 since it was never used in the first place

What about when (eventually tm) decomp supports like oot 1.0, that wouldn't rock the f3dex2-PL ucode? What would be your plan then

The plan would be don't use an outdated ucode version in any build and always use the current version

Wait, you're saying that Master Quest Debug has been using F3DEX2 with point lighting this whole time???

That's correct, they updated the ucode for gamecube and for some reason it was updated to a version with point light support.

sauraen · 2024-06-19T16:45:41Z

src/code/z_lights.c

+// Toggleable static for debugging, recommended for release is to use static
+#ifndef STATIC
+#if IS_DEBUG
+#define STATIC
+#else
+#define STATIC static
+#endif
+#endif


Why would we not want locally used data and functions to be labeled static?

Marking a function or data as static prevents it from appearing in the linker map file, which some don't like as they'd like to lookup addresses in the map file while debugging. This is a big reason why we've never moved to add static keywords to functions upstream.

Since debugging tools have gotten more sophisticated, like ares getting a gdb stub implementation this may start to no longer be a problem, but some may still appreciate the toggle.

Got it. I think this PR should not introduce any new systems like this and stick with how the code is in the rest of the repo. If you would like to make a separate PR to globally introduce a static toggle, feel free.

New lighting engine

3688f53

Dragorn421 reviewed Jun 17, 2024

View reviewed changes

sauraen requested changes Jun 19, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New lighting engine #137

New lighting engine #137

Thar0 commented Jun 16, 2024

Dragorn421 Jun 17, 2024 •

edited

Loading

sauraen left a comment •

edited

Loading

sauraen Jun 19, 2024

Thar0 Jun 19, 2024

sauraen Jun 19, 2024

Thar0 Jun 19, 2024

sauraen Jun 19, 2024

Thar0 Jun 19, 2024

sauraen Jun 19, 2024

sauraen Jun 19, 2024

sauraen Jun 19, 2024

Thar0 Jun 19, 2024

sauraen Jun 19, 2024

Thar0 Jun 19, 2024

sauraen Jun 19, 2024

sauraen Jun 19, 2024

Thar0 Jun 19, 2024

sauraen Jun 19, 2024 •

edited

Loading

Thar0 Jun 19, 2024

sauraen Jun 19, 2024

Thar0 Jun 19, 2024

Dragorn421 Jun 19, 2024

Thar0 Jun 19, 2024

sauraen Jun 19, 2024

Thar0 Jun 19, 2024

sauraen Jun 19, 2024

Thar0 Jun 19, 2024

sauraen Jun 19, 2024

New lighting engine #137

Are you sure you want to change the base?

New lighting engine #137

Conversation

Thar0 commented Jun 16, 2024

Dragorn421 Jun 17, 2024 • edited Loading

Choose a reason for hiding this comment

sauraen left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sauraen Jun 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dragorn421 Jun 17, 2024 •

edited

Loading

sauraen left a comment •

edited

Loading

sauraen Jun 19, 2024 •

edited

Loading