Skip to content

fix(ll): remove gamma correction and set light mult to PI#2305

Merged
alandtse merged 4 commits into
community-shaders:devfrom
jiayev:ll-fix
May 10, 2026
Merged

fix(ll): remove gamma correction and set light mult to PI#2305
alandtse merged 4 commits into
community-shaders:devfrom
jiayev:ll-fix

Conversation

@jiayev
Copy link
Copy Markdown
Collaborator

@jiayev jiayev commented May 9, 2026

This pull request removes the enableGammaCorrection setting and related code across the linear lighting system, and makes minor improvements to lighting calculations and data structure alignment. The main focus is on simplifying configuration and data passed to shaders, while ensuring correct alignment and lighting math.

Linear Lighting Settings and Data Structure Simplification:

  • Removed the enableGammaCorrection field from the LinearLighting::Settings struct, associated UI, serialization, and all per-frame data structures, streamlining the configuration and data sent to shaders. [1] [2] [3] [4] [5] [6]
  • Added a padding field (pad0) and a static assertion to ensure PerFrameData is 16-byte aligned, preventing potential GPU alignment issues. [1] [2]

Shader Logic Improvements:

  • Updated the DirectionalLight and PointLight functions in Color.hlsli to multiply light multipliers by Math::PI when linear lighting is enabled, improving physical correctness.
  • Removed redundant gamma correction logic from the output color calculation in ISHDR.hlsl that depended on the removed setting.

Code Formatting:

  • Minor formatting improvement to the bloomMask calculation for readability in ISHDR.hlsl.

Summary by CodeRabbit

  • Bug Fixes

    • More consistent rendering: light contribution multipliers and bloom/HDR gamma handling adjusted for linear-lighting mode.
  • UI Changes

    • Removed the "Enable Gamma Correction" option from Linear Lighting settings.
  • Chores

    • Internal lighting settings layout updated and feature version bumped to 1-1-1.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 9, 2026

📝 Walkthrough

Walkthrough

This PR centralizes linear-lighting enablement by replacing a gamma-correction flag with enableLinearLighting across C++ and shader code, updates constant-buffer layout with padding and alignment, multiplies directional/point lights by π in linear mode, reformats HDR/SDR bloom logic, and bumps feature metadata version.

Changes

Linear Lighting Settings Refactoring

Layer / File(s) Summary
C++ Data Structure Layout
src/Features/LinearLighting.h
LinearLighting::Settings and PerFrameData replace enableGammaCorrection with enableLinearLighting; PerFrameData adds pad0 and enforces 16-byte alignment.
Shader Constant Buffer
package/Shaders/Common/SharedData.hlsli
SharedData::LinearLightingSettings removes enableGammaCorrection and adds pad0 trailing field to maintain constant-buffer layout.
Directional & Point Light Normalization
package/Shaders/Common/Color.hlsli
Color::DirectionalLight and Color::PointLight multiply output by Math::PI * lightMult when ENABLE_LL && !isLinear, replacing previous lightMult-only scaling.
Shader Output & Tone Mapping
package/Shaders/ISHDR.hlsl
Bloom mask expression is reformatted; conditional gamma-to-linear conversion gated by the removed flag is deleted, leaving gamma handling to ENABLE_LL/isHDR logic.
C++ Configuration & UI
src/Features/LinearLighting.cpp
JSON serialization mapping and ImGui checkbox for enableGammaCorrection removed; GetCommonBufferData() stops assigning the removed gamma flag.
Feature Metadata
features/Linear Lighting/Shaders/Features/LinearLighting.ini
Increment Version from 1-1-0 to 1-1-1.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • doodlum
  • alandtse
  • davo0411

Poem

🐰 I hopped through shaders, bytes, and light,

I nudged a flag and set it right,
Padding snug where bytes align,
Lights now wear π for extra shine,
A tiny hop — the render's bright!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title clearly and concisely summarizes the two main objectives: removing gamma correction and setting light multipliers to PI in the linear lighting system.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 OpenGrep (1.20.0)

OpenGrep fatal error (exit code 2): [00.11][ERROR]: Error: exception Unix_error: No such file or directory stat src/Features/LinearLighting.h
Raised by primitive operation at UTmp.replace_named_pipe_by_regular_file_if_needed in file "libs/commons/UTmp.ml", line 145, characters 8-27
Called from Scan_CLI.replace_target_roots_by_regular_files_where_needed.(fun) in file "src/osemgrep/cli_scan/Scan_CLI.ml", lines 1086-1087, characters 19-65
Called from List_.fast_map in file "libs/commons/List_.ml", line 81, characters 17-20
Called fro


Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 9, 2026

No actionable suggestions for changed features.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 9, 2026

✅ A pre-release build is available for this PR:
Download

Comment thread src/Features/LinearLighting.h Outdated
@alandtse alandtse changed the title fix(ll): remove gamma correction settings and set light mult to PI fix(ll): remove gamma correction and set light mult to PI May 10, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/Features/LinearLighting.h (1)

1-1: ⚡ Quick win

Consider a shorter conventional title and add issue linkage

The title is descriptive, but for strict Conventional Commit formatting you could shorten it to something like: fix(ll): drop gamma toggle, apply pi scaling.
If this PR resolves tracked work, add a footer such as Fixes #<issue> or Addresses #<issue>.

As per coding guidelines, "When reviewing PRs, please provide suggestions for: 1. Conventional Commit Titles ... 2. Issue References ..."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Features/LinearLighting.h` at line 1, Update the PR title to follow
Conventional Commit style (e.g., "fix(ll): drop gamma toggle, apply pi scaling")
and add an issue linkage footer in the PR description or commit message (e.g.,
"Fixes #<issue>" or "Addresses #<issue>") so it maps to tracked work; edit the
PR title field and the last commit message or PR description accordingly to
include the shorter conventional title and the Fixes/Addresses footer.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/Features/LinearLighting.h`:
- Line 1: Update the PR title to follow Conventional Commit style (e.g.,
"fix(ll): drop gamma toggle, apply pi scaling") and add an issue linkage footer
in the PR description or commit message (e.g., "Fixes #<issue>" or "Addresses
#<issue>") so it maps to tracked work; edit the PR title field and the last
commit message or PR description accordingly to include the shorter conventional
title and the Fixes/Addresses footer.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c7e84f9c-27f0-4284-9729-0022d12fc683

📥 Commits

Reviewing files that changed from the base of the PR and between c64338a and ea0f896.

📒 Files selected for processing (2)
  • package/Shaders/Common/SharedData.hlsli
  • src/Features/LinearLighting.h
🚧 Files skipped from review as they are similar to previous changes (1)
  • package/Shaders/Common/SharedData.hlsli

@alandtse alandtse merged commit 8e9d25b into community-shaders:dev May 10, 2026
13 checks passed
ParticleTroned pushed a commit to ParticleTroned/skyrim-community-shaders that referenced this pull request May 15, 2026
…shaders#2305)

Branch-preserving adaptation: remove this branch's Linear Lighting gamma-correction toggle and ISHDR gamma-conversion block without importing upstream HDR display mapping changes. Keep a branch-local Linear Lighting cache bump.
ParticleTroned pushed a commit to ParticleTroned/skyrim-community-shaders that referenced this pull request May 16, 2026
…shaders#2305)

Branch-preserving adaptation: remove this branch's Linear Lighting gamma-correction toggle and ISHDR gamma-conversion block without importing upstream HDR display mapping changes. Keep a branch-local Linear Lighting cache bump.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants