fix: use Smith GGX visibility approximation - #70
Merged
Conversation
- Add source, analytical, GPU, and external evidence contracts for the shared Smith GGX visibility change. - Validate formula and fixture controls while preserving the intended missing-helper RED contract.
- Replace the direct PBR visibility roots with a shared p-squared linearized Smith GGX helper. - Verify source, analytical, GPU, roughness, and brightness contracts in Unity EditMode.
- Regenerate schema v4 direct-light evidence and document the approved shared Smith GGX visibility behavior. - Verify the canonical isolated Daily suite passes 99 tests without baseline or protected-state changes.
6 tasks
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 114 |
| Duplication | 6 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Contributor
There was a problem hiding this comment.
Pull request overview
Updates Pure-Base’s shared PBR/Hybrid direct-light GGX visibility term to use a linearized Smith joint GGX approximation (using academic roughness a = p^2), and adds focused source/analytical/GPU-contract evidence plus documentation updates to support the behavior change.
Changes:
- Replace the direct GGX visibility calculation in
PureBasePbrEvaluateDirectwith a new shared helper implementing the linearized Smith joint GGX visibility approximation (Switch-aware denominator handling included). - Extend Daily EditMode tests with analytical/source-layout contracts and optional external evidence export to validate finiteness, degeneracies, and pass isolation behavior.
- Update technical documentation and NOTICE with provenance, formula details, and evidence metadata.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Tests/Daily/Editor/PureBaseToonLightingContractTests.PbrRoughnessRuntime.cs | Adds runtime capture support for full-frame visibility observations and evidence artifact writing. |
| Tests/Daily/Editor/PureBaseToonLightingContractTests.PbrRoughness.cs | Adds visibility reference-case tests plus opt-in external evidence export flow and JSON manifest building. |
| Tests/Daily/Editor/PureBasePbrVisibilityApproximationTests.cs.meta | Adds Unity .meta for the new test file. |
| Tests/Daily/Editor/PureBasePbrVisibilityApproximationTests.cs | Adds analytical characterization and source-layout contracts ensuring the intended fast visibility helper is used and shaped correctly. |
| Shaders/Common/pbr_brdf.hlsl | Introduces PureBasePbrEvaluateSmithJointGgxVisibility and updates direct BRDF evaluation to use it with widened float arithmetic. |
| NOTICE | Adds visibility-term provenance and Unity behavior-reference clarification. |
| Docs/technical-information.md | Documents the new direct GGX visibility helper, formula, and evidence scope/limits (English). |
| Docs/technical-information.ja.md | Documents the new direct GGX visibility helper, formula, and evidence scope/limits (Japanese). |
| Docs/pure-base-shader-contract.md | Updates shader contract documentation to reflect the new direct GGX visibility behavior and evidence summary. |
Files not reviewed (1)
- Tests/Daily/Editor/PureBasePbrVisibilityApproximationTests.cs.meta: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Refactor private visibility test inputs and numerical checks to remove actionable Codacy findings. - Verify focused Unity tests and WSL Codacy analyzers pass without changing capture semantics.
- Refactor private visibility test inputs and symmetry assertions to remove actionable Codacy findings. - Verify diagnostics and local Codacy analyzers; confirm the Cloud PR gate after push.
- Format visibility observation labels and diagnostic filenames with invariant culture. - Verify the fr-FR regression contract and focused Daily visibility tests pass.
PenguinDOOM
enabled auto-merge (squash)
August 19, 2026 17:36
PenguinMaintainer
approved these changes
Aug 19, 2026
PenguinMaintainer
deleted the
future/smith-ggx-visibility-approximation
branch
August 19, 2026 17:36
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.
Summary
p^2p^4ownership, add normal/Switch denominator handling, and retain direct-specular arithmetic infloatthroughNdotLForwardBaseas one-light minus zero-light andForwardAddas two-lights minus one-lightValidation
pwsh -NoProfile -File "D:\VRChatProjects\Pure-Base\Packages\jp.penguin.purebase\Tests\Run-PureBaseRegression.ps1" -Mode Daily: 99 passed, 0 failed, 0 skippedLimitations
SHADER_API_SWITCHhas source/formula coverage but no Switch runtime execution.