Repository navigation
Shadow cascade fade, the uniform cache's bits, and WindLight's water glint - #442
Merged
Merged
Conversation
Contributor
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 24 minutes. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Summary
Merge Risk: ⚪ Minimal · up to No confirmed issue blocks merging. Complete the planned build and in-world rendering checks as normal validation. Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 23.53% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 51 functions across 6 files. (2 skipped: … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
-
-
Check name Status Explanation Title check The title clearly summarizes the pull request's three main renderer changes: shadow cascade fading, uniform cache bit comparison, and WindLight water glint. Description check The description provides detailed context, implementation scope, and testing status for all three changes. It does not include the template's Related Issues section, checklist, or Additional Notes sec… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
-
Full details: Docstring Coverage
-
-
Explanation
Docstring coverage is 23.53% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 51 functions across 6 files. (2 skipped: 2 unsupported.)
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
A rabbit checks the shader’s glow,
Bits stay true as values flow.
Water glints with roughness set,
Blend and alpha follow yet.
I twitch my nose; the tests all run,
Then hop beneath the setting sun.
Comment @coderabbitai help to get the list of available commands.
sampleDirectionalShadow faded the last cascade out toward its far split by adding the fade to the filtered shadow, so a lit receiver there came to twice lit at the split and dropped back to lit past it: a hard line across the far view that moved with the draw distance and the camera's angle. Opaque surfaces never showed the line, as sunLightF clamps the shadow it writes to the light map at 1, but the water, alpha and PBR alpha shaders read the shadow raw, and the water's sun glint drew the line. The fade now mixes the filtered shadow toward lit. On opaque surfaces lit and fully shadowed receivers fade as they did; a penumbra in the far half of the cascade, which the clamped sum took to lit sooner, now fades in proportion and is a little darker than it was. sampleSpotShadow carried the same addition and mixes too. It also weighted its one map as a cascade and divided by the weight, which is zero three quarters of the way to the sun's third split, so a receiver at that distance came out NaN. With one map there is nothing to weight, and the division goes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…iv uploads all four ints The cache that lets a uniform setter skip a value the program already holds kept a float LLVector4 per location in a boost::unordered_map. An int or uint went through a float on the way in, so past 2^24 two values compared equal and the change was dropped: uniform1ui(FRAME_ID) after about 77 hours at 60 fps, and the CAS parameters, uploaded as the bits of floats through uniform4uiv, whenever they moved by less than about 64 ulps. Values of every type shared one slot, the fast setters uploaded without telling it, and a bare link() kept values the relink had reset. Every set paid a hash and a bucket walk. The cache is a flat array indexed by uniform location, sized at mapUniforms past the largest location a setter can reach. A slot holds the kind of upload that wrote it (the GL component type and count) and the uploaded components' bit patterns; a set matches only when both do, so ints and uints compare as themselves and floats by their bits, and a location written through one kind and set through another uploads. The reserved-index setters and the named one share a location's slot. A set costs a bounds compare, one 20-byte slot and an OR of XORs; there is no hashing and no allocation. Arrays (count != 1) always upload and forget their location's slot; matrices neither read nor write it. The fast setters record what they upload, and link, mapUniforms and unload empty the cache. uniform4iv uploaded with glUniform1iv, which on an ivec4 uniform is GL_INVALID_OPERATION and sets nothing; nothing in the tree called it yet. It uploads with glUniform4iv, cached by its four ints. llglslshader_gl reads back what the program holds: ints, ivec4s, uints and uvec4s set past 2^24 and one ulp apart; set A, B, A again; a slot empty after link against a value put there behind the cache; link and remapping forgetting; a bool set through int and float; the named and reserved setters sharing a slot; a fast set between two normal ones; arrays forgetting. Taking an int through a float again fails it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The sun's reflection on the water was a hard, narrow line, from three faults stacked in its specular. The wave normal was pulled flat with the distance, so far water was a mirror. Its roughness was the environment's blur multiplier, around 0.04, which blurs the reflections and has nothing to do with the sun. And its F0 was the pool's legacy sun colour, scaled by magic numbers to around 110 at a low sun, with a metallic of 1: the glint was clamped flat at its ceiling wherever it showed. It is lit as WindLight's water lit it, from 2010 to the PBR viewer. The wave normal's slopes are steepened as the view grazes the surface, by up to ten times, so seen low the faces turned toward the sun fill more of each pixel and the path breaks into a field of glints. The lobe is WindLight's Phong about the mirror direction, normalized, its exponent 2 / alpha^2 - 2 from AlchemyRenderWaterGlintRoughness, whose default of 0.35 is WindLight's 128. A GGX lobe on the steepened slopes was tried: its long tail lit every face within a few tens of degrees of the sun and drew each crest across the path as a streak. Water is a dielectric: the glint is the light's colour at the Fresnel reflectance of the angle between the view and the light, worked from v.l alone, as the half vector of a view and a setting sun at the horizon would not normalize. The pool's magic numbers and the specular uniform go, and underWaterF's unread specular with them. The glint writes its glow, as WindLight's did: the lobe, by 0.1 rising to 0.3 with the light's elevation, unless shadowed, into the screen's alpha, which the legacy glow and the bloom both read. The pool adds it to the glow already there, so what glows beneath the surface keeps its own. Only the class 3 shader writes it, as class 1's placeholder writes 1, and not from below or in probe captures. What is seen through the surface is displaced as WindLight displaced it: by the steepened wave, shrinking with the square root of the distance along the surface, and less where the surface mostly reflects. It was displaced by six times the plain wave, as strongly at a grazing view as looking straight down into it. The probes' reflection stays a lookup by the wave normal's direction, as the PBR viewer has it; WindLight displaced a mirrored render of the scene on the screen, which the probes are not. Two things in that lookup change. The water settings' normal scale holds one value per wave layer, as WindLight used it; the PBR water multiplied the composite normal's x, y and z by the three values and then normalized, so equal values, the default among them, cancelled and the setting did nothing. Each layer's slopes are scaled by its own value, against the default of 2, which leaves the reflection as it was. And a wave tilted across a low view reflected the view ray downward, so the probes returned their lower faces, the ground seen from above, on water where the next wave would hide anything below the horizon. The reflection is held on the horizon, and the probes and screen space reflections take the normal that reflects the view ray there; reflections above the horizon are untouched. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
RyeMutt
force-pushed
the
rye/render-fixes
branch
from
October 11, 2026 06:34
2f73bc0 to
2c5aa45
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three fixes to the renderer, one commit each.
The last shadow cascade fades its receivers to lit, not past it.
sampleDirectionalShadowfaded the last cascade out by adding the fade to the filtered shadow. A lit receiver near the far split came out twice as lit, then dropped back to lit past it: a hard line across the far view that moved with the draw distance and the camera.sunLightFclamps its output, so opaque surfaces hid the line. Water, alpha and PBR alpha read the shadow raw, and the water's sun glint showed it. The fade now mixes toward lit.sampleSpotShadowmade the same addition and now mixes too. It also divided its one map by a cascade weight that is zero at three quarters ofshadow_clip.z, which gave NaN at that distance. That weight is gone.LLGLSLShader's uniform cache compares the bits it uploaded, and
uniform4ivuploads all four ints. The cache that skips redundantglUniform*calls kept a floatLLVector4per location in a hash map. Ints and uints went through a float, so past 2^24 two different values compared equal and the change was dropped:uniform1ui(FRAME_ID)after about 77 hours at 60 fps;uniform4uiv, whenever they moved by less than ~64 ulps.The cache is now a flat array indexed by location. Each slot holds the upload's GL type and component count plus the raw bits, so a set skips only when both match exactly.
link,mapUniformsandunloadclear the cache.uniform4ivcalledglUniform1iv, which isGL_INVALID_OPERATIONon anivec4. Nothing called it yet.llglslshader_glreads the values back from GL.The water's sun glint, its glow and its ripples are WindLight's again. The sun's reflection on the water was a hard, narrow line, for three reasons:
It now follows WindLight's water, checked against the WindLight shaders in history:
AlchemyRenderWaterGlintRoughness, whose default 0.35 is WindLight's exponent 128. Its brightness is dielectric Fresnel.Two changes to the probes' reflection:
Testing
llglslshader.cpp,llshadermgr.cpp,lldrawpoolwater.cpp,llglslshader_gl_test.cpp) compile from this branch with the Windows build's flags and warnings as errors.llglslshader_glpasses on a full build of a branch stacked on this one. A deliberately broken copy of the test fails.waterF,underWaterF,shadowUtil) pass glslang in every define combination they are built with.🤖 Generated with Claude Code