Skip to content

chore: improve contrast function#1470

Merged
doodlum merged 1 commit into
devfrom
contrast-bettere
Sep 13, 2025
Merged

chore: improve contrast function#1470
doodlum merged 1 commit into
devfrom
contrast-bettere

Conversation

@doodlum
Copy link
Copy Markdown
Collaborator

@doodlum doodlum commented Sep 13, 2025

Summary by CodeRabbit

  • New Features
    • Enhanced HDR tone mapping for a more cinematic look with improved contrast.
    • Better preservation of shadow detail and highlight control across varied scenes.
    • Smoother, more natural color transitions without requiring user setting changes.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Sep 13, 2025

Walkthrough

Introduces a two-path contrast mapping in ISHDR.hlsl: computes contrastOriginal via lerp and contrastShadows via a pow-based curve, then selects between them per-channel based on a comparison with hdrColor. Function signatures and external interfaces remain unchanged.

Changes

Cohort / File(s) Summary
HDR Contrast Mapping Update
package/Shaders/ISHDR.hlsl
Replaces single-line HDR remap with dual-path contrast: contrastOriginal = lerp(avgValue.x, hdrColor, Cinematic.z) and contrastShadows = pow(abs(hdrColor)/avgValue.x, Cinematic.z)*avgValue.x*sign(hdrColor), choosing between them with a ternary comparison against hdrColor. No public API changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Scene as Scene Input
  participant ISHDR as ISHDR.hlsl
  participant Output as Color Out

  Scene->>ISHDR: hdrColor, avgValue.x, Cinematic.z
  rect rgba(200,230,255,0.25)
    note over ISHDR: Compute two contrast paths
    ISHDR->>ISHDR: contrastOriginal = lerp(avgValue.x, hdrColor, Cinematic.z)
    ISHDR->>ISHDR: contrastShadows = pow(abs(hdrColor)/avgValue.x, Cinematic.z)\n * avgValue.x * sign(hdrColor)
  end
  alt contrastOriginal < hdrColor
    ISHDR->>ISHDR: hdrColor = contrastShadows
  else
    ISHDR->>ISHDR: hdrColor = contrastOriginal
  end
  ISHDR-->>Output: hdrColor
  note over Output: Public interface unchanged
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • jiayev

Poem

I tweaked the tones with gentle care,
Two paths for light, a bunny’s flair—
A pow for shadows, lerp for glow,
We pick the best, then let it flow.
Hop hop—contrast finds its art,
In pixels bright, a rabbit’s heart. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "chore: improve contrast function" accurately and concisely reflects the main change in the diff — updates to the HDR contrast/remap logic in package/Shaders/ISHDR.hlsl introducing a two-path contrast strategy — and is clear enough for a teammate scanning history to understand the primary intent.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch contrast-bettere

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 05f5c40 and 1030cea.

📒 Files selected for processing (1)
  • package/Shaders/ISHDR.hlsl (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{cpp,cxx,cc,c,h,hpp,hxx,hlsl,hlsli,fx,fxh,py}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Do not include TODO/FIXME placeholders; provide complete, working solutions

Files:

  • package/Shaders/ISHDR.hlsl
🧠 Learnings (2)
📓 Common learnings
Learnt from: jiayev
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-08-03T18:37:19.690Z
Learning: ISReflectionsRayTracing.hlsl and ISWorldMap.hlsl in the skyrim-community-shaders repository are image-space post-processing shaders that perform color sampling and blending operations that need proper linear color space handling for the linear lighting system. ISReflectionsRayTracing handles screen-space reflections and should use conditional Color::IrradianceToLinear/Gamma conversions similar to ISCompositeLensFlareVolumetricLighting.hlsl. ISWorldMap performs 7x7 color accumulation that should be done in linear space similar to the pattern used in ISSAOComposite.hlsl.
📚 Learning: 2025-08-03T18:37:19.690Z
Learnt from: jiayev
PR: doodlum/skyrim-community-shaders#0
File: :0-0
Timestamp: 2025-08-03T18:37:19.690Z
Learning: ISReflectionsRayTracing.hlsl and ISWorldMap.hlsl in the skyrim-community-shaders repository are image-space post-processing shaders that perform color sampling and blending operations that need proper linear color space handling for the linear lighting system. ISReflectionsRayTracing handles screen-space reflections and should use conditional Color::IrradianceToLinear/Gamma conversions similar to ISCompositeLensFlareVolumetricLighting.hlsl. ISWorldMap performs 7x7 color accumulation that should be done in linear space similar to the pattern used in ISSAOComposite.hlsl.

Applied to files:

  • package/Shaders/ISHDR.hlsl
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Validate shader compilation (VR, .github/configs/shader-validation-vr.yaml)
  • GitHub Check: Validate shader compilation (Flatrim, .github/configs/shader-validation.yaml)
  • GitHub Check: Build plugin and addons
🔇 Additional comments (1)
package/Shaders/ISHDR.hlsl (1)

119-121: Fix component-wise selection and guard against divide-by-zero in contrast step

  • Use per-channel selection (mask + lerp), explicit broadcast for avgValue, and add an epsilon to avoid division-by-zero.
-	float3 contrastOriginal = lerp(avgValue.x, hdrColor, Cinematic.z);
-	float3 contrastShadows = pow(abs(hdrColor) / avgValue.x, Cinematic.z) * avgValue.x * sign(hdrColor);
-	hdrColor = contrastOriginal < hdrColor ? contrastShadows : contrastOriginal;
+	// Use explicit broadcast, add epsilon for robustness, and select per-channel via mask
+	float  avg = max(avgValue.x, 1e-6);
+	float3 contrastOriginal = lerp(float3(avg, avg, avg), hdrColor, Cinematic.z);
+	float3 contrastShadows  = pow(abs(hdrColor) / avg, Cinematic.z) * avg * sign(hdrColor);
+	float3 mask = saturate(sign(hdrColor - contrastOriginal)); // 1 where hdrColor > contrastOriginal, else 0
+	hdrColor = lerp(contrastOriginal, contrastShadows, mask);
  • Optional perf notes: use rcp(avg) instead of division, consider approximating pow if hot, and clamp Cinematic.z with max(Cinematic.z, 0.0) if negatives are possible.

Verification: compile-check failed in the provided environment — "dxc: command not found". Run the dxc compile-check in your local/CI environment to confirm.


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.

❤️ Share

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

@github-actions
Copy link
Copy Markdown

Using provided base ref: 05f5c40
Using base ref: 05f5c40
Base commit date: 2025-09-13T01:06:55+01:00 (Saturday, September 13, 2025 01:06 AM)
No actionable suggestions for changed features.

@github-actions
Copy link
Copy Markdown

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

@doodlum doodlum merged commit 4b28a85 into dev Sep 13, 2025
17 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request Feb 2, 2026
@alandtse alandtse deleted the contrast-bettere branch February 6, 2026 05:20
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.

1 participant