Skip to content

Align Toon SH lighting with OpenLit - #64

Merged
PenguinMaintainer merged 8 commits into
masterfrom
future/openlit-sh-lighting-alignment
Aug 15, 2026
Merged

Align Toon SH lighting with OpenLit#64
PenguinMaintainer merged 8 commits into
masterfrom
future/openlit-sh-lighting-alignment

Conversation

@PenguinDOOM

Copy link
Copy Markdown
Collaborator

Summary

  • align Toon dominant direction and two-band spherical-harmonics evaluation with the OpenLit 1.0.2-derived contract
  • preserve Shader-Core light aggregation, Issue ToonのShadow attenuationをlightColor / lightDirectionから分離する #60 shadow visibility separation, lightmap ownership, direct-only ForwardAdd, and PBR/Hybrid/Unlit behavior
  • add numerical, source, Linear D3D11, forced-Gamma, ForwardAdd, and transient lilToon 2.3.4 classification coverage
  • document the bounded OpenLit/lilToon provenance and behavior in English and Japanese

Validation

  • Tests/Run-PureBaseRegression.ps1 -Mode Daily: 86 passed, 0 failed, 0 skipped; canonical BIRP baseline included
  • Unity EditMode Daily after supplemental observation: 86 passed, 0 failed, 1 expected ignored; the ignored canonical test is batchmode-only and is covered by the clean batchmode run
  • focused OpenLit source/runtime/Gamma/ForwardAdd/lightmap/shadow/non-Toon contracts passed
  • git diff --check passed

Notes

  • No Shader-Core, lilToon package, ProjectSettings, baseline, release identity, shader ABI, or product module-selection changes.
  • The lilToon observation records top/side/bottom band classification and transition orientation; it is descriptive and does not use final RGB equality as a parity threshold.

- Add OpenLit 1.0.2 direction, two-band SH, Gamma, and ForwardAdd diagnostic contracts.
- Validate pure reference behavior and record expected RED results against the current Toon implementation.
- Align Toon direction weighting and two-band SH evaluation with the OpenLit-derived contract.
- Validate focused lighting contracts and the clean batchmode Daily regression lane.
- Document the OpenLit-derived Toon lighting contract and attribution boundaries in English and Japanese.
- Add transient lilToon classification observation and validate the Daily lighting integration suite.
@PenguinDOOM
PenguinDOOM requested review from a team and a lite review from Copilot August 15, 2026 07:17
@PenguinDOOM PenguinDOOM added this to the 0.2.0 milestone Aug 15, 2026
@PenguinDOOM PenguinDOOM linked an issue Aug 15, 2026 that may be closed by this pull request
9 tasks
@codacy-production

codacy-production Bot commented Aug 15, 2026

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 7 minor

Results:
7 new issues

Category Results
CodeStyle 6 minor
Complexity 1 minor

View in Codacy

🟢 Metrics 80 complexity · 0 duplication

Metric Results
Complexity 80
Duplication 0

View in Codacy

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

Run reviewer

TIP This summary will be updated as you push new changes.

@codacy-production codacy-production Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

The PR is currently not up to standards according to Codacy analysis. While the implementation aligns with the OpenLit 1.0.2 contract for Toon lighting—including color-space luminance weighting and dominant direction logic—the code introduces a dependency on global Unity state (unity_SH...) within the BIRP host. This breaks the existing abstraction where SH coefficients are passed as parameters, potentially impacting custom host implementations or modular overrides. Furthermore, the test suite, specifically ShaderCoreTestHostManifestTests.cs, has exceeded maintainability limits regarding file length and method complexity, requiring refactoring before further expansion.

About this PR

  • The test suite is showing signs of maintainability decay. Multiple issues were identified in the primary manifest test file regarding length and duplication, which should be addressed to ensure future lighting contract changes can be easily validated.
1 comment outside of the diff
Tests/Daily/Editor/ShaderCoreTestHostManifestTests.cs

line 1 🔴 HIGH RISK
This file has reached 1024 non-comment lines, exceeding the 1000-line limit. Consider refactoring the ShaderCoreTestHostManifestTests class by splitting it into multiple partial classes or separate test files categorized by functional area (e.g., ToonManifestTests.cs, PbrManifestTests.cs).

Test suggestions

  • Verify color-space specific luminance weighting for direct light in both Gamma and Linear modes.\n- [x] Verify dominant direction calculation includes absolute-Y SH and pre-normalization fallback.\n- [x] Numerical validation of two-band SH evaluation (bright vs dark) against the OpenLit oracle.\n- [x] Confirm ForwardAdd isolation ensures direct-only direction without SH or fallback contributions.\n- [x] Verify degenerate direction handling (zero or nonfinite vectors) uses the specified fallback.\n- [x] Validate source code contracts via regex to ensure unscaled V is used for SH and fallback precedes normalization.\n- [x] Regression test to ensure PBR, Hybrid, and Unlit models are unaffected by Toon-specific lighting changes.

TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback

Comment thread Tests/Daily/Editor/ShaderCoreTestHostManifestTests.cs Outdated
Comment thread Tests/Daily/Editor/ShaderCoreTestHostManifestTests.cs
Comment thread Shaders/Common/birp_host.hlsl Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Aligns PureBase/Toon dominant-direction aggregation and two-band spherical-harmonics evaluation with an OpenLit 1.0.2-derived contract (as observed via lilToon 2.3.4), while preserving Shader-Core ownership boundaries (light aggregation, lightmaps, ForwardAdd direct-only behavior, and shadow visibility separation).

Changes:

  • Updates Toon lighting helpers/host integration to use OpenLit-style luminance weighting, fallback-inclusive dominant direction, and OpenLit two-band SH structure (including Gamma conversion behavior).
  • Adds a fixed forced-Gamma Toon OpenLit Shader-Core host fixture plus manifest/source-contract assertions to prevent pass/placement regressions.
  • Expands Daily tests with numerical oracle, runtime D3D11 Linear/Gamma readbacks, ForwardAdd direct-only diagnostics, and lilToon observation logging; documents provenance/behavior in EN/JA docs.

Reviewed changes

Copilot reviewed 25 out of 34 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
Tests/Fixtures/Hosts/ToonOpenLit.meta Adds Unity folder metadata for the Toon OpenLit host fixture.
Tests/Fixtures/Hosts/ToonOpenLit/sc_common.hlsl Fixture include that redirects Shader-Core host-local includes through the product Toon path.
Tests/Fixtures/Hosts/ToonOpenLit/sc_common.hlsl.meta Unity metadata for the fixture include.
Tests/Fixtures/Hosts/ToonOpenLit/PureBaseTestToonOpenLitGamma.scshader Adds a forced-Gamma, two-pass (ForwardBase/ForwardAdd) diagnostic host used for OpenLit contract verification.
Tests/Fixtures/Hosts/ToonOpenLit/PureBaseTestToonOpenLitGamma.scshader.meta Unity metadata for the scshader asset.
Tests/Fixtures/Hosts/ToonOpenLit/PureBaseTestToonOpenLitGamma_properties.hlsl Defines the host’s Shader-Core property block for the diagnostic shader.
Tests/Fixtures/Hosts/ToonOpenLit/PureBaseTestToonOpenLitGamma_properties.hlsl.meta Unity metadata for the properties include.
Tests/Fixtures/Hosts/ToonOpenLit/phase_shade.hlsl Adds a ForwardAdd-only diagnostic that encodes the aggregated direction for readback.
Tests/Fixtures/Hosts/ToonOpenLit/phase_shade.hlsl.meta Unity metadata for the phase include.
Tests/Fixtures/Hosts/ToonOpenLit/jp.penguin.purebase.tests.shadercore.toonopenlitgamma.scmodule Test-only module identifier for the OpenLit Gamma host selection.
Tests/Fixtures/Hosts/ToonOpenLit/jp.penguin.purebase.tests.shadercore.toonopenlitgamma.scmodule.meta Unity metadata for the scmodule.
Tests/Daily/Editor/ShaderCoreTestHostManifestTests.cs Extends manifest/source checks with OpenLit Gamma host runtime/source placement contracts.
Tests/Daily/Editor/PureBaseToonLightingContractTests.cs Removes superseded legacy Toon SH oracle helpers now replaced by OpenLit-specific tests.
Tests/Daily/Editor/PureBaseToonLightingContractTests.Shadow.cs Updates shadow oracle to use the OpenLit dominant direction and two-band SH evaluation path.
Tests/Daily/Editor/PureBaseToonLightingContractTests.Runtime.cs Refactors capture runtime scope into partial and generalizes fixed-host selection scope for reuse.
Tests/Daily/Editor/PureBaseToonLightingContractTests.OpenLit.cs Adds pure numerical OpenLit 1.0.2 oracle tests plus runtime contracts for D3D11 Linear and forced-Gamma host readbacks.
Tests/Daily/Editor/PureBaseToonLightingContractTests.OpenLit.cs.meta Unity metadata for the new test file.
Tests/Daily/Editor/PureBaseToonLightingContractTests.OpenLitRuntime.cs Adds capture-scope extensions for rendering the ForwardAdd diagnostic delta.
Tests/Daily/Editor/PureBaseToonLightingContractTests.OpenLitRuntime.cs.meta Unity metadata for the new capture extension file.
Tests/Daily/Editor/PureBaseToonLightingContractTests.OpenLitRegression.cs Adds source-level regression tests to lock in OpenLit-specific helper/host gating behavior.
Tests/Daily/Editor/PureBaseToonLightingContractTests.OpenLitRegression.cs.meta Unity metadata for the new regression test file.
Tests/Daily/Editor/PureBaseToonLightingContractTests.LilToonObservation.cs Adds supplemental lilToon 2.3.4 observation logging for classification/orientation without parity assertions.
Tests/Daily/Editor/PureBaseToonLightingContractTests.LilToonObservation.cs.meta Unity metadata for the observation test file.
Tests/Daily/Editor/PureBaseRenderingModeRenderingTests.SourceContracts.cs Updates source contract expectations to delegate OpenLit-specific helper assertions into a dedicated test file.
Tests/Daily/Editor/PureBaseRenderingModeRenderingTests.OpenLitSourceContracts.cs Adds focused source contracts for OpenLit-derived Toon helper and host gating behavior.
Tests/Daily/Editor/PureBaseRenderingModeRenderingTests.OpenLitSourceContracts.cs.meta Unity metadata for the new source contracts file.
Tests/Config/shader-core-test-hosts.json Registers the new forced-Gamma Toon OpenLit host entry and its runtime/source evidence contract.
Shaders/Models/toon.hlsl Updates Toon model integration: uses OpenLit-derived ambient description and defines direction-weight macro hook.
Shaders/Common/toon_lighting.hlsl Reworks Toon direction + two-band SH evaluation to OpenLit-derived luminance/fallback/L0-L2/L1 structure.
Shaders/Common/birp_host.hlsl Introduces customizable light-direction weighting and updates Toon ForwardBase/ForwardAdd direction + SH gating logic.
NOTICE Updates third-party provenance acknowledgment details for the OpenLit/lilToon-inspected concepts.
Docs/technical-information.md Documents bounded OpenLit-derived Toon behavior and ownership boundaries (English).
Docs/technical-information.ja.md Documents bounded OpenLit-derived Toon behavior and ownership boundaries (Japanese).
Docs/pure-base-shader-contract.md Adds formal shader-contract documentation for OpenLit-derived Toon direction/SH behavior.
Files not reviewed (9)
  • Tests/Daily/Editor/PureBaseRenderingModeRenderingTests.OpenLitSourceContracts.cs.meta: Generated file
  • Tests/Daily/Editor/PureBaseToonLightingContractTests.LilToonObservation.cs.meta: Generated file
  • Tests/Daily/Editor/PureBaseToonLightingContractTests.OpenLit.cs.meta: Generated file
  • Tests/Daily/Editor/PureBaseToonLightingContractTests.OpenLitRegression.cs.meta: Generated file
  • Tests/Daily/Editor/PureBaseToonLightingContractTests.OpenLitRuntime.cs.meta: Generated file
  • Tests/Fixtures/Hosts/ToonOpenLit.meta: Generated file
  • Tests/Fixtures/Hosts/ToonOpenLit/PureBaseTestToonOpenLitGamma.scshader.meta: Generated file
  • Tests/Fixtures/Hosts/ToonOpenLit/PureBaseTestToonOpenLitGamma_properties.hlsl.meta: Generated file
  • Tests/Fixtures/Hosts/ToonOpenLit/jp.penguin.purebase.tests.shadercore.toonopenlitgamma.scmodule.meta: Generated file

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread Tests/Daily/Editor/ShaderCoreTestHostManifestTests.cs
Comment thread Tests/Daily/Editor/PureBaseToonLightingContractTests.LilToonObservation.cs Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 20f259c9d0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Tests/Daily/Editor/PureBaseToonLightingContractTests.LilToonObservation.cs Outdated
- Record missing optional lilToon as a passing supplemental observation before rendering.
- Verify installed and CI-equivalent Daily runs pass with no skipped tests.
- Normalize generated-source assertions and nest OpenLit contract helpers without changing test fixture behavior.
- Verify focused Daily tests and local static analysis pass with no new findings.
- Keep OpenLit source-contract helpers private to the fixture while allowing partial callers to compile.

- Restore Toon ambient assertions and move OpenLit tests into their fixture partial for Codacy analysis.
- Guard optional manifest evidence and centralize imported host source validation.
- Validate exact lilToon version parsing with focused Unity tests and Codacy analysis.
- Require caller-provided SH coefficients in Toon generated-source assertions.
- Verify the complete Daily lane passes without skips or protected-file changes.
@PenguinMaintainer
PenguinMaintainer merged commit 67c9e32 into master Aug 15, 2026
10 checks passed
@PenguinMaintainer
PenguinMaintainer deleted the future/openlit-sh-lighting-alignment branch August 15, 2026 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ToonのSHライティングをOpenLitの計算へ忠実に合わせる

3 participants