From 034a684f02d2779bfb21872e2a4457e7dd04168b Mon Sep 17 00:00:00 2001 From: Erik Date: Mon, 27 Apr 2026 22:43:14 +0200 Subject: [PATCH] fix(sky): partition sky pass on Properties bit 0x01, not bit 0x04 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pre/post-scene sky pass split was using SkyObjectData.IsWeather (bit 0x04) — the wrong bit. Per the named retail decomp: GameSky::CreateDeletePhysicsObjects at 0x005073c0 / decomp 269036: MakeObject(this, gfx_id, &tex_velocity, (properties & 1), // arg4: post-scene flag (properties & 4)); // arg5: weather gate GameSky::MakeObject at 0x00506ee0 / decomp 268656: if (arg4 != 0) AddObjectToSingleCell(result, after_sky_cell); // post-scene else AddObjectToSingleCell(result, before_sky_cell); // pre-scene So bit 0x01 routes between before_sky_cell (rendered pre-scene by GameSky::Draw(0)) and after_sky_cell (rendered post-scene by GameSky::Draw(1)). Bit 0x04 is independent — it gates whether the object is instantiated at all when LScape::weather_enabled is false. In Dereth's Rainy DayGroup this matters for the rain cylinders: 0x01004C42 Props=0x04 (bit 0x04 only) → pre-scene + weather-gated 0x01004C44 Props=0x05 (bits 0x01+0x04) → post-scene + weather-gated 0x01004C35 Props=0x02 (bit 0x02 only) → pre-scene (cloud, fog-hide) Before this fix acdream put BOTH rain cylinders in the post-scene pass (because both have bit 0x04). That double-rendered foreground rain — explained why acdream's foreground rain looked thicker than retail's. Now only 0x01004C44 is foreground; 0x01004C42 renders with the sky dome. Added SkyObjectData.IsPostScene (bit 0x01) with citations. Renamed the internal RenderPass parameter weatherPass → postScenePass and updated both the partition criterion and the -120m foreground-rain Z offset to gate on it. Public RenderSky / RenderWeather entry points kept their names for API stability; doc comments updated to explain the bit semantics. Independent confirmation from one of the user's external code-review agents — the report's Setup-objects-silently-dropped finding is the remaining defect in the same family (Setup IDs 0x020xxx aren't loaded by EnsureMeshUploaded; deferred to a separate phase). 1227 tests pass. --- src/AcDream.App/Rendering/Sky/SkyRenderer.cs | 63 ++++++++++++-------- src/AcDream.Core/World/SkyDescLoader.cs | 46 ++++++++++---- 2 files changed, 72 insertions(+), 37 deletions(-) diff --git a/src/AcDream.App/Rendering/Sky/SkyRenderer.cs b/src/AcDream.App/Rendering/Sky/SkyRenderer.cs index 76f5fa6..6b91c35 100644 --- a/src/AcDream.App/Rendering/Sky/SkyRenderer.cs +++ b/src/AcDream.App/Rendering/Sky/SkyRenderer.cs @@ -107,21 +107,27 @@ public sealed unsafe class SkyRenderer : IDisposable float dayFraction, DayGroupData? group, SkyKeyframe keyframe) - => RenderPass(camera, cameraWorldPos, dayFraction, group, keyframe, weatherPass: false); + => RenderPass(camera, cameraWorldPos, dayFraction, group, keyframe, postScenePass: false); /// - /// Draw the WEATHER sky objects (the foreground rain mesh - /// 0x01004C42/0x01004C44 on Rainy DayGroups, plus the - /// per-storm 5cm flash dummies — every SkyObject with - /// Properties & 0x04 != 0). Called AFTER the scene so the - /// rain meshes paint on top of terrain and entities — that's the - /// retail-faithful order from LScape::draw at - /// 0x00506330, where GameSky::Draw(1) fires after the - /// DrawBlock loop. With depth-test disabled and additive blend - /// (the rain Surface flag 0x080000C5 includes Additive), the - /// 815m-tall rain cylinder's bright streak texels add over the scene - /// — making rain appear in the air between camera and character - /// instead of only at the horizon. + /// Draw the POST-SCENE sky objects (the foreground rain mesh + /// 0x01004C44 on Rainy DayGroups, plus any other SkyObject with + /// Properties & 0x01 != 0). Called AFTER the scene so these + /// meshes paint on top of terrain and entities — retail-faithful order + /// from LScape::draw at 0x00506330, where + /// GameSky::Draw(1) fires after the DrawBlock loop and + /// renders the after_sky_cell contents. With depth-test + /// disabled and additive blend (the rain Surface flag includes + /// Additive), the 815m-tall rain cylinder's bright streak texels add + /// over the scene — making rain appear in the air between camera and + /// character instead of only at the horizon. + /// + /// Method name kept as RenderWeather for API stability; the + /// pass actually partitions on + /// (Properties bit 0x01), not + /// (bit 0x04). The two bits are independent in retail per + /// GameSky::CreateDeletePhysicsObjects at 0x005073c0. + /// /// public void RenderWeather( ICamera camera, @@ -129,14 +135,15 @@ public sealed unsafe class SkyRenderer : IDisposable float dayFraction, DayGroupData? group, SkyKeyframe keyframe) - => RenderPass(camera, cameraWorldPos, dayFraction, group, keyframe, weatherPass: true); + => RenderPass(camera, cameraWorldPos, dayFraction, group, keyframe, postScenePass: true); /// /// Shared pass for and . /// Sets up the same GL state for both (depth-test off, additive + /// alpha-blend per submesh, camera-anchored translation) and iterates /// only the SkyObjects matching the requested partition by - /// . + /// — bit 0x01 per the + /// retail decomp at GameSky::MakeObject (0x00506ee0). /// private void RenderPass( ICamera camera, @@ -144,7 +151,7 @@ public sealed unsafe class SkyRenderer : IDisposable float dayFraction, DayGroupData? group, SkyKeyframe keyframe, - bool weatherPass) + bool postScenePass) { if (group is null || group.SkyObjects.Count == 0) return; @@ -197,14 +204,20 @@ public sealed unsafe class SkyRenderer : IDisposable for (int i = 0; i < group.SkyObjects.Count; i++) { var obj = group.SkyObjects[i]; - // Partition by weather flag — the caller chose either the - // pre-scene sky pass (non-weather) or the post-scene weather - // pass (weather only). Mirrors retail GameSky::Draw at - // 0x00506ff0 where arg2==0 iterates non-weather sky_obj - // entries (filtered by property bit 0x04 == 0 inside the - // loop) and arg2==1 draws after_sky_cell which only contains - // weather objects. - if (obj.IsWeather != weatherPass) continue; + // Partition by post-scene flag (Properties bit 0x01) — the + // caller chose either the pre-scene sky pass (bit clear) or + // the post-scene pass (bit set). Mirrors retail + // GameSky::CreateDeletePhysicsObjects at 0x005073c0 / decomp + // line 269036 which routes (Properties & 1) into + // before_sky_cell vs after_sky_cell, and GameSky::Draw at + // 0x00506ff0 which renders those cells in the two passes. + // NOTE: bit 0x04 (IsWeather) is independent — it gates whether + // the object is instantiated when weather_enabled is false. + // Earlier acdream incorrectly used IsWeather for this + // partition, putting the outer rain cylinder 0x01004C42 + // (Props=0x04, NO bit 0x01) into the post-scene pass with the + // foreground rain — double-thick rain not matching retail. + if (obj.IsPostScene != postScenePass) continue; if (!obj.IsVisible(dayFraction)) continue; // Apply per-keyframe replace overrides. @@ -267,7 +280,7 @@ public sealed unsafe class SkyRenderer : IDisposable // (camera-119.89)..(camera+694.90) in view space — camera // is inside, looking in any direction shows surrounding // walls — the volumetric foreground-rain look retail has. - if (weatherPass) + if (postScenePass) model = model * Matrix4x4.CreateTranslation(0f, 0f, -120f); _shader.SetMatrix4("uModel", model); diff --git a/src/AcDream.Core/World/SkyDescLoader.cs b/src/AcDream.Core/World/SkyDescLoader.cs index 5ef8245..ada2753 100644 --- a/src/AcDream.Core/World/SkyDescLoader.cs +++ b/src/AcDream.Core/World/SkyDescLoader.cs @@ -37,22 +37,44 @@ public sealed class SkyObjectData public uint Properties; /// - /// True when this SkyObject is flagged as weather (Properties bit - /// 0x04). Per the named retail decomp, + /// True when this SkyObject is gated on the weather system (Properties + /// bit 0x04). Per the named retail decomp, /// GameSky::CreateDeletePhysicsObjects at 0x005073c0 - /// passes Properties & 0x04 as arg5 of - /// GameSky::MakeObject (0x00506ee0) — when set, the - /// CPhysicsObj is added to after_sky_cell instead of - /// before_sky_cell, and GameSky::Draw(arg2=1) at - /// 0x00506ff0 draws that cell after the scene. acdream - /// uses this flag to split the sky pass: non-weather objects render - /// pre-scene (so terrain and entities z-test on top), weather meshes - /// (e.g. the 815m-tall rain cylinders 0x01004C42/0x01004C44) - /// render post-scene with depth-test off so they overlay foreground - /// geometry — matching retail's volumetric foreground-rain look. + /// passes Properties & 4 as arg5 of + /// GameSky::MakeObject (0x00506ee0); the inner + /// (arg5 == 0 || LScape::weather_enabled != 0) guard at decomp + /// line 268630 means weather-flagged objects only get instantiated when + /// the global weather flag is on. This bit does not control + /// pre/post-scene placement — that's . + /// acdream currently always renders weather-flagged objects (we don't + /// honor a weather_enabled toggle yet); when we add one, this flag is + /// the gate. /// public bool IsWeather => (Properties & 0x04u) != 0u; + /// + /// True when this SkyObject renders after the world scene + /// (Properties bit 0x01) — i.e. as foreground over terrain and + /// entities. Per the named retail decomp, + /// GameSky::CreateDeletePhysicsObjects passes + /// Properties & 1 as arg4 of + /// GameSky::MakeObject (decomp line 269036); MakeObject at + /// decomp 268656 routes arg4 != 0 objects into + /// after_sky_cell instead of before_sky_cell, and + /// GameSky::Draw(arg2=1) at 0x00506ff0 draws + /// after_sky_cell as a separate post-scene pass. + /// + /// In Dereth's Rainy DayGroup this distinguishes the two rain + /// cylinders: 0x01004C44 (Props=0x05) is foreground rain + /// rendered after terrain; 0x01004C42 (Props=0x04 alone) is + /// background rain rendered with the sky dome. Earlier + /// versions of acdream incorrectly split on + /// (bit 0x04) so both rain meshes ended up in the post-scene pass, + /// double-rendering rain in the foreground. + /// + /// + public bool IsPostScene => (Properties & 0x01u) != 0u; + /// Object is visible at day-fraction /// by retail's begin/end semantics (r12 §2). Three cases: ///