benilla-octo/docs/WATER_PORT.md

Porting Improved Water from deepthought84/benilla-watershader

grom, 2026-11-09: "this guy has nicer looking water than we do after we messed with it, so I want to replace our water visuals with his".

This is the measurement, the method and what is left to do. Nothing here is a plan in the abstract: every number below was taken off the two trees.

What his fork is

A fork of samwhosung/benilla that adds Improved Water: planar mirrors for lakes, rivers and sea, fixed cube probes for falls, rapids and small pools, a screen-space march, a wave simulation, shoreline foam and per-zone water colour on the clock. No ray tracing — every reflection is rasterized. It is off by default behind an options checkbox, and the look lives in its own crate, crates/benilla-water, behind a seam at crates/benilla-world/src/liquid/hooks.rs.

The measurement

Both forks descend from samwhosung/benilla, so a real common ancestor exists even though our own history is one squash and git cannot see it. scripts/water-port-merge.sh stitches one together and runs the merge; these numbers come from that.

merge base	3f777291 (loading_screen: hold a cross-map screen...), 4 commits behind upstream/main
his delta	203 commits, 168 files, +46 240 / −742
of those, brand-new files	98 files, 28 382 lines — graft, no merge
of those, files that already existed	70 files, 18 600 changed lines
files we have also changed	59
conflicts after a real three-way merge	34 files, 82 hunks, 3 831 lines

134 of his 168 files merge clean. The whole job is those 82 hunks, plus a compile loop, plus taking our own water out.

Only 742 deletions on his side: he adds a parallel system and leaves the reference water standing. That is what makes the graft possible at all.

Where the conflicts are

 11 hunks  crates/benilla-world/src/water_fx/mod.rs
  9        crates/benilla-app/src/capture/mod.rs
  8        crates/benilla-world/src/static_gx/render.rs
  7        crates/benilla-world/src/liquid/surface.rs
  4        crates/benilla-app/src/game_plugins.rs
  4        README.md
  3        crates/benilla-world/src/lighting/mod.rs
  3        crates/benilla-app/src/ui_script/options_tests.rs
  3        crates/benilla-world/src/sky.rs
  3        crates/benilla-world/src/ffx_glow.rs
  3        crates/benilla-assets/src/shaders/liquid.wgsl
  2        liquid/mod.rs, pipe_warm/menagerie.rs, portrait/mod.rs, shaders/sky.wgsl
  1        lifecycle.rs, terrain_stream.rs, video.rs, capture/scenarios.rs, sun/mod.rs,
           water_fx/params.rs, gates.sh, world_assets/images.rs, liquid/query.rs, .gitignore,
           cover_input_wall.rs, OptionsFrame.xml, minimap/blips/mod.rs, assets/lib.rs,
           sun/follow.rs, minimap/mod.rs
  DU       cvars/mod.rs, cvars/table.rs, portrait/panes.rs   (we deleted, he changed)

Three of the small ones are not small: both forks added a parameter to the same SystemParam tuple in minimap/blips/mod.rs, which our own comment already says is full at 16, so taking both sides needs the tuple nested rather than extended. The three DU files are code we moved somewhere else; his change has to be routed to wherever it went.

What of ours it replaces

These five files are ours alone — not upstream's, not his — and they are our water look. "Replace ours with his" means they go:

crates/benilla-assets/src/shaders/enhanced_water.wgsl
crates/benilla-world/src/liquid/reflect.rs        (planar water)
crates/benilla-world/src/liquid/scene_depth.rs
crates/benilla-world/src/liquid/waves.rs
crates/benilla-world/src/liquid/lod.rs

with WaterQuality, the waterReflection CVar and every call site. water_fx/ stays: it is upstream's and both forks changed it, which is why it carries the most conflicts.

The shape grom chose

2026-11-09, asked with the numbers above in front of him: full swap, drop the extras.

deleted	our five water files; water_fx/bob.rs, the swimmer bob; the Water Quality dropdown
added	his Improved Water checkbox, and the waterStyle / waterQuality CVars behind it
kept	the Ironforge magma palette, and our shader hooks (emissive_hook, fog_hook, moonlight_hook, shadow_hook, wet_hook, wind_hook)

The bob rode our vertex swell (liquid/waves.rs, query.rs's has_vertex_swell), which goes with the rest; his water has its own wave field (benilla-water/src/ripple_sim.rs, a real surface height) to hang it back on if it is ever wanted.

Where the three DU files route

We deleted what he changed, so these three are hand-placed rather than merged:

his file	what he adds	where it goes here
cvars/table.rs	the waterStyle and waterQuality rows	crates/benilla-app/src/cvars.rs, the REGISTERED table (line ~153)
cvars/mod.rs	their observers	the same file
portrait/panes.rs	PANE_LAYER_BASE must start past WATER_RENDER_LAYER (31)	crates/benilla-app/src/portrait/mod.rs:149, UNIT_PANE_LAYER_BASE

The layer one is not cosmetic: hooks.rs claims layer 31 for liquid and 30 for the unmirrored labels, and our booth ladder currently grows from UI_MODELS_LAYER + 1 with nothing stopping it reaching 30. A silent collision there is what that ladder exists to prevent.

Resolution log

file	verdict
.gitignore, README.md, tests/cover_input_wall.rs	ours — his side is his own workflow and upstream drift past our squash base, not water
the other 31	open

liquid/surface.rs is the one to be careful in: his liquid_bevy_mesh takes four more arguments (lattice, planar, coarse/fine, shoreline) and his WMO spawn collects a batch, so hunk 4 drops our let tone = MagmaTone::for_wmo(wmo_id) if taken wholesale. Both sides are wanted there.

Two decisions taken, not asked

1. Ours goes, his stays. That is what "replace" means, and keeping both would leave two systems fighting over the same seam — the 11 hunks in water_fx/mod.rs are already that fight.
2. His option row stays, defaulted ON. He ships Improved Water off by default because his fork is a fork; here it is the water. The checkbox stays because this project does not ship a hidden graphics setting, and because "draw it the way 1.12 does" is worth keeping reachable.

Still to do

1. Resolve the 82 hunks (scripts/water-port-merge.sh puts a mid-merge tree in front of you).
2. Delete our five files and their call sites.
3. Compile. The merge brings his benilla-assets/benilla-world API with it, so the symbols his crate reaches for (materials::LiquidKey, ATTRIBUTE_WOW_PLANAR, liquid::{WaterQuality, ProbeMarks, MIRROR*, PROBE_SLOT_MAX, CLOCK_LANES, DOME_LANES, LOOK_WATER_RECT}, formats::{WaterClasses, ProbeSpot, CellWater, NO_SPOT, adt_liquids}) arrive with it — but every one of them lands next to our own version of the same file.
4. Re-apply our own liquid work onto his 9 707-line liquid.wgsl: the hook modules (emissive_hook, fog_hook, moonlight_hook, shadow_hook, wet_hook, wind_hook) and the Ironforge magma palette (MagmaTone), neither of which exists in his tree.
5. The options row, the CVar, the benilla-config/Cache/ classification pass, the debug panel's Water section.
6. Re-capture the water scenarios and re-record whatever goldens move.

---

Where it stands, 2026-11-09 (end of day)

It works. On branch water-port, six commits.

gate
cargo fmt --all -- --check	clean
cargo clippy --workspace --all-targets -- -D warnings	clean
cargo test --workspace	12 failures, all of them this fork's pre-existing data-half goldens — the same 12 main carries
the look	reference vs Improved Water over water-elwynn-falls: RMSE 7492 (0.114)
the CVar path	waterStyle written at runtime, the path the checkbox takes: RMSE 7268

The Improved Water checkbox is on the Graphics page, under Field of View, with waterStyle behind it and waterQuality registered beside it.

The one fault that hid everything

A block of ours survived the merge at the top of liquid.wgsl's fragment entry:

if water_active() { ... return enhanced_water(...); }

It returns before a line of his code runs, and the module it called was deleted with the rest of our water. Every liquid surface came out invisible — the Stormwind canal drew dry while 253 surfaces were view-visible in that frame, which is why this looked like a culling or render-layer fault. The render-layer lead recorded above was wrong, and measuring it is what proved it: the world camera carries layers [0, 30, 31] and the surfaces carry [31].

Lesson for the next graft of this shape: when two forks both rewrote the same shader entry point, the auto-merge keeps both preludes and the first return wins silently. Grep the merged entry point for the losing side's symbols before trusting a frame.

Known gaps

1. Fog. fog_law and apply_fog share the linear law through fog_hook, but the stylised terms do not take the Modern model (fogModel 1, which every graphics preset above the lowest turns on) — apply_fog applies it and they do not. Threading the model through his terms is real work and is not done.
2. The quality dropdown. The existing Water Quality row was kept and re-pointed at his three tiers rather than deleted as the plan said, because deleting it would leave his tier knob reachable only from the console.
3. The static_gx prepass compiles and is wired to this fork's four bind groups, but nothing has confirmed the Hi-Z march reads real depth from it.
4. His probe cache writes to benilla-config/Cache/; untested across a map change here.

Superseded

Where it stands, 2026-11-09

The graft is on branch water-port, in three commits. It builds, runs and lints clean (cargo clippy --workspace --all-targets -- -D warnings): the Improved Water plugin comes up, its map classifier runs (Azeroth in 5.1 s, 240 probe spots), 5 water frame sets load, the stylised liquid pipeline compiles with its own shader defs, and the bind-group budget is comfortable (51 sampled textures of 8 388 606).

What is not done is the look. The water is not visible.

What the measurement says

liquid surfaces spawned in the Stormwind canal scene	25
of them on screen	0
material lookups that failed	0 (probed on both spawn paths)
$WOW_WATER_STYLE 0 vs 1 over water-ocean	byte-identical

The ocean that is visible in water-ocean is the WDL horizon mesh, not a liquid surface — which is why the style A/B over it is byte-identical and why the toggle looked dead. It is not dead: WATER_STYLISED appears in the liquid pipeline's defs at 1 and not at 0.

So the fault is not the shader, not the materials and not the classifier: the surfaces exist and nothing draws them.

The lead

liquid/hooks.rs puts every liquid surface on render layer 31 and stamps the world camera with world_camera_layers() from an Added<WorldCamera> query. If this fork's camera is spawned before that system runs — or if anything re-inserts RenderLayers on it afterwards — the water is drawn into a layer no camera renders, which is exactly this symptom. Check that before reading a line of WGSL.

After it: hide_liquid_surfaces, the exterior cull and his own visibility authority are the other three things that can take a spawned surface off screen.

Then

1. The look, scene by scene, against his goldens (scenarios::water_scene_names).
2. The Improved Water options row (the CVars waterStyle and waterQuality are registered and bridged; nothing in the UI writes them yet). Note the departure from the plan: the existing Water Quality dropdown was kept and re-pointed at his three tiers rather than deleted, because deleting it would have left his tier knob reachable only from the console, which this project does not do with graphics settings.
3. cargo test --workspace — not run yet on the branch.
4. The static_gx prepass: it compiles and is wired to this fork's four bind groups, but nothing has yet confirmed the Hi-Z march reads real depth from it.

---

2026-11-09, grom's two clips: the washed frames

grom, with Improved Water on: "seems to be messing with our volumetric fog, that is terrible and needs to be fixed" — in the Thunderbrew Distillery, and again across Stormwind. The frames carry a pale flat wash with straight edges over the scene; the canal water itself and its reflections he calls good.

Not reproduced. Everything below was tried and the two styles came out byte-identical, or clean:

tried	result
volfog-inn, inn-interior, inn-noon, style 0 vs 1	RMSE 0
the same three with grom's own config.toml (WOW_CAPTURE_CVARS)	RMSE 0
water-canal-dusk with his config, three runs	clean frames, no wash
stormwind-canal-near, water-ocean, water-lake	no wash
volumetricFog=3 + fogModel=1 + waterQuality=3 explicitly	no wash

So it is not a property of a settled frame. Both clips show it while moving, and both reach it after a zone change. That shape — invisible when still, visible in motion — points at something frame-accumulated. The froxel volume (volumetricFog 3, which grom runs) is the only temporally reprojected lane in this client, and the water adds a per-frame glow accumulator of its own (benilla-water/src/glow_acc.rs) that feeds our FFX glow.

Checked and cleared by reading: the glow accumulator is sized for 8K and cleared per view before the water draws; FroxelView is stamped on WorldCamera alone, and his mirror camera is explicitly never WorldCamera, so the froxel history is not shared with it.

The one question that would settle it, with the wash on screen, in this order:

/console volumetricFog 1      -- the froxel volume off, the cheap haze on
/console ffxGlow 0            -- the FFX glow pass off
/console bloom 0
/console waterQuality 1       -- his mirrors and probes down a tier

Whichever one makes it go is the lane. Without that, the next step is a scripted fly-through capture rather than a still, which the harness cannot do today.

Done in the same pass

• water_fog now goes through fog_hook::fog_sample, so the water's body, glints, foam and reflected sky fog by our model and not by its own copy of 1.12's linear law. At fogModel 1 those were two different hazes. Measured: fogModel 0 vs 1 with the water on now moves the frame by RMSE 428; it moved the water's own pixels by nothing before.
• Ironforge's lava is fitted to the building's own art — see the commit.

2026-11-09, round two on the washed frames

grom tried the four console lines: none of them killed it, and ffxGlow is not a CVar this fork registers at all. He also reported a second symptom — "in lots of the city rain doesn't fall all the way to the ground, almost like it hits an invisible roof".

Four more theories raised and killed by measurement, so nobody re-runs them:

theory	how it died
a stray liquid surface at a wrong height, which the rain would land on and the fog would terminate at	a census of every LiquidSurface in the Stormwind canal scene: 764 of them, the highest at y 144.5, none wider than 33 yd. No plane over the city.
liquid colliders catching the raindrops	liquid rides CollisionLayer::Liquid; the precip ground oracle filters to Default | Walk, which cannot see it
the shared light buffer's rows shifted, so the fog model and the shelter map read the wrong words	the merged WowLight is row-for-row what it was: 0-19, point_count at 20, points 21-532, monkey after
grom's resolution (2560x1440) breaking a half-res pass	A/B at 2560x1440: clean, RMSE 4424 between the lanes and no wash in either

One thing did reproduce, and it is worth chasing on its own: with rain forced on (WOW_WEATHER=1,0.9) over water-canal-dusk, the two lanes differ by RMSE 877, and in the stylised frame the rain's impact rings on the water are gone while the reference lane shows them. His own preview levers list rings and rain, so they are meant to be there.

Still not reproduced in a still: the wash itself. Both clips show it in motion and after a zone change — a capture loads one map and never changes it, and the harness cannot fly a path.

What would settle it, and all it costs grom is one line while the wash is on screen (131 characters, tested):

/script local x,y,z=UnitPosition("player") ChatFrame1:AddMessage(format("%d %d %d %s|%s",x,y,z,GetRealZoneText(),GetSubZoneText()))

With his world position a scenario goes exactly there, and a still either shows the wash or proves it needs motion — which is itself the answer.

2026-11-09, round three: the wash is the MIST VEIL, and it was indoors

His pins came back, and with them the answer. The pin he gave for the wash (-8451 579 94 Stormwind City|Dwarven District) is an outdoor street, and a still there shows almost nothing: style 0 vs style 1 at that spot is RMSE 0.0098 normalised, and a 360-degree pan burst over eight yaws never exceeds 0.0106. The earlier 0.0524 at the same pin was a 960x540 run that had not settled; it is not evidence and it is retired here.

What his two SCREENSHOTS show, though, is that he was inside a building both times — the Deeprun Tram arch and the inn — and that is the whole thing. An interior scenario reproduces it at once:

scene	style 0 vs style 1	mean
grom-fog (his pin, outdoors)	0.0098	+0.4
stormwind-cathedral-interior	0.0010	+0.0
volfog-inn	0.0846	+16.7

Bisected with his own levers at volfog-inn, every run at age 900 so it settles:

run	vs reference
style 1, everything on	0.0846
WOW_PREVIEW=0	0.0031
WOW_PREVIEW_GLOW=0	0.0846
the underwater family off (window tint shafts waterline caustics)	0.0843
WOW_PREVIEW_GRADE=0	0.0843
WOW_PREVIEW_MIST=0	0.0036

So: not the glow (the round-two lead, now dead), not the underwater combine, not the grade. It is WX-2, the mist veil (preview_rows.rs's veil_on → ffx_glow.wgsl's fs_combine_veil).

Why a veil over open water fills a room. veil_apply integrates an exponential mist layer (scale height 2.5 yd) along the ray from the eye to whatever the pixel shows, with the layer's base at the elected mirror plane and the eye's height above it as e0. Nothing in it asks where the eye is standing. Its own log at volfog-inn prints the trap:

veil true: mirror plane 57.7 strength 1.00, in water false, indoors false, mist true

The pond outside the Goldshire inn elects a mirror plane at y 57.7; the inn's floor is at 58.8. The eye is 1.1 yards above the pond's surface, so exp(-e0/2.5) is 0.64 — nearly the layer's full density — and the veil lays the pond's dawn mist across the room, posts, hearth and all.

The fix is the gate the sun shafts already use for the same reason (volumetric_light: "Interiors (the camera in a WMO room) take no shafts"): CameraInteriorClaim. A room's air is the room's. One line in veil_on, and the log now reads indoors true → veil false.

Measured after: volfog-inn style 1 vs the reference lane 0.0034 (mean −0.15), against 0.0036 for mist switched off entirely — i.e. the veil is the whole of it, and nothing else of his mist was lost.

Still open from this round: the rain's missing impact rings (above), and the invisible-roof report, which is a precipitation question rather than a water one.