feat(accessibility): Barrel Zoom for 3D shooters - #1741
Open
feilen wants to merge 2 commits into
Open
Conversation
- Makes it more comfortable to play 3D or FPS games on a tiny screen! - Accessability feature which accepts strength, VFOV, and cylindrical ratio as tunables. - Allows for a sort of center-zoom which trends towards 'angle-area' to 'screen area' ratio being 1:1, instead of the sides of the screen taking up more space overall. - Hides all tunables when the feature is disabled - GL and Vulkan implementations - Currently does not compose with other screen effects meaningfully - probably could (need to think over which effects should go in what order - e.g. FXAA should go first but FSR should go after) - Does not remap mouse or touch input (doesn't matter for perspective-heavy games which typically use mouse-look/controller)
Contributor
📝 WalkthroughWalkthroughAdds configurable barrel distortion to screen effects, including UI controls, persisted settings, OpenGL and Vulkan rendering paths, JNI plumbing, push constants, and shader-based UV remapping. ChangesBarrel distortion screen effect
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ScreenEffectsConfig
participant VulkanRenderer
participant JNI
participant VulkanRendererContext
participant VulkanShaders
ScreenEffectsConfig->>VulkanRenderer: Apply barrel settings
VulkanRenderer->>JNI: Pass barrel parameters
JNI->>VulkanRendererContext: Update native effect state
VulkanRendererContext->>VulkanShaders: Upload push constants
VulkanShaders->>VulkanShaders: Compute and sample distorted UVs
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
feilen
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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.
Inline comments:
In `@app/src/main/java/app/gamenative/ui/component/ScreenEffectsPanel.kt`:
- Around line 895-897: Update ScreenEffectsPanel’s resetEffects() to also set
enableBarrelDistortion to false. In the rendered Column, add the
barrel-distortion ScreenEffectToggleRow and its matching AnimatedVisibility
slider block, mirroring the implementation in GLScreenEffectsTabContent or
ScreenEffectsTabContent so users can toggle and adjust the existing
barrel-distortion state.
In `@app/src/main/java/app/gamenative/ui/util/ScreenEffectsConfig.kt`:
- Around line 176-185: Update the barrel distortion branch in the apply logic to
retrieve the existing BarrelDistortionEffect through
composer.getEffect(BarrelDistortionEffect::class.java), falling back to a new
instance only when absent, then continue updating its strength, height, and
cylindricalRatio before adding it to effects.
In `@app/src/main/java/com/winlator/renderer/effects/BarrelDistortionEffect.java`:
- Around line 41-46: Update BarrelDistortionEffect.onUse so the height uniform
uses the same lower-bound-only behavior as VulkanRendererContext.setEffect:
clamp height to a minimum of 0.05 without imposing an upper bound. Leave the
strength and cylindricalRatio handling unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f3da31c3-5e78-4c70-aa69-c8c5f338ef03
⛔ Files ignored due to path filters (2)
app/src/legacy/jniLibs/arm64-v8a/libvulkan_renderer.sois excluded by!**/*.soapp/src/modern/jniLibs/arm64-v8a/libvulkan_renderer.sois excluded by!**/*.so
📒 Files selected for processing (11)
app/src/main/cpp/winlator/VulkanRendererContext.cppapp/src/main/cpp/winlator/VulkanRendererContext.happ/src/main/cpp/winlator/vulkan_jni.cppapp/src/main/cpp/winlator/window.fragapp/src/main/cpp/winlator/window.vertapp/src/main/java/app/gamenative/ui/component/ScreenEffectsPanel.ktapp/src/main/java/app/gamenative/ui/component/dialog/ScreenEffectDialog.ktapp/src/main/java/app/gamenative/ui/util/ScreenEffectsConfig.ktapp/src/main/java/com/winlator/renderer/VulkanRenderer.javaapp/src/main/java/com/winlator/renderer/effects/BarrelDistortionEffect.javaapp/src/main/res/values/strings.xml
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 13 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/src/main/cpp/winlator/window.frag">
<violation number="1" location="app/src/main/cpp/winlator/window.frag:298">
P1: Barrel distortion overwrites the brightness/contrast/gamma adjustments applied just above it. The barrel branch re-samples from the raw texture (`texSampler`), discarding the color adjustments that were already applied to `rgb` on line 297. Users who enable barrel zoom will lose their brightness, contrast, and gamma settings entirely.
Suggestion: re-apply `applyColorAdjustments(rgb)` after the barrel sample, or move the barrel distortion before the color-adjustment step.</violation>
</file>
<file name="app/src/main/java/app/gamenative/ui/util/ScreenEffectsConfig.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/util/ScreenEffectsConfig.kt:180">
P2: The FOV slider range (40–120°) extends beyond the range that actually takes effect, because `BarrelDistortionEffect.onUse()` clamps the `height` uniform to `[-1.0, 1.0]`. Since `height = tan(VFOV / 2)`, values above VFOV = 90° produce `tan(VFOV/2) > 1.0` and get clamped down to 1.0 — making FOV=100°, FOV=110°, and FOV=120° produce the identical result as FOV=90°. The effective range is therefore 0–90°, so the upper slider bound of 120° is misleading and may confuse users who try higher values expecting a visible difference.
Consider either capping the UI range to 0–90° to match the effect's effective range, or adjusting the shader clamp in `BarrelDistortionEffect.onUse()` to accept values above 1.0 if higher FOVs are intended to have an effect.</violation>
</file>
Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.
Re-trigger cubic
| if (hasEffect(EFFECT_MASK_NTSC)) rgb = applyNTSC(uv, rgb); | ||
| if (hasEffect(EFFECT_MASK_BARREL_DISTORTION)) { | ||
| vec3 puv = dot(vUVDotBarrel, vUVDotBarrel) * vec3(-0.5, -0.5, -1.0) + vUVBarrel; | ||
| rgb = texture(texSampler, puv.xy / puv.z).rgb; |
Contributor
There was a problem hiding this comment.
P1: Barrel distortion overwrites the brightness/contrast/gamma adjustments applied just above it. The barrel branch re-samples from the raw texture (texSampler), discarding the color adjustments that were already applied to rgb on line 297. Users who enable barrel zoom will lose their brightness, contrast, and gamma settings entirely.
Suggestion: re-apply applyColorAdjustments(rgb) after the barrel sample, or move the barrel distortion before the color-adjustment step.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/cpp/winlator/window.frag, line 298:
<comment>Barrel distortion overwrites the brightness/contrast/gamma adjustments applied just above it. The barrel branch re-samples from the raw texture (`texSampler`), discarding the color adjustments that were already applied to `rgb` on line 297. Users who enable barrel zoom will lose their brightness, contrast, and gamma settings entirely.
Suggestion: re-apply `applyColorAdjustments(rgb)` after the barrel sample, or move the barrel distortion before the color-adjustment step.</comment>
<file context>
@@ -284,7 +293,12 @@ void main() {
if (hasEffect(EFFECT_MASK_NTSC)) rgb = applyNTSC(uv, rgb);
+ if (hasEffect(EFFECT_MASK_BARREL_DISTORTION)) {
+ vec3 puv = dot(vUVDotBarrel, vUVDotBarrel) * vec3(-0.5, -0.5, -1.0) + vUVBarrel;
+ rgb = texture(texSampler, puv.xy / puv.z).rgb;
+ }
}
</file context>
| if (config.enableBarrelDistortion && abs(config.barrelDistortionStrength) > 0.001f) { | ||
| val barrelEffect = BarrelDistortionEffect().apply { | ||
| strength = config.barrelDistortionStrength / 100f | ||
| height = tan(config.barrelDistortionFov * ScreenEffectsConfig.DEGREES_TO_RADIANS / 2f) |
Contributor
There was a problem hiding this comment.
P2: The FOV slider range (40–120°) extends beyond the range that actually takes effect, because BarrelDistortionEffect.onUse() clamps the height uniform to [-1.0, 1.0]. Since height = tan(VFOV / 2), values above VFOV = 90° produce tan(VFOV/2) > 1.0 and get clamped down to 1.0 — making FOV=100°, FOV=110°, and FOV=120° produce the identical result as FOV=90°. The effective range is therefore 0–90°, so the upper slider bound of 120° is misleading and may confuse users who try higher values expecting a visible difference.
Consider either capping the UI range to 0–90° to match the effect's effective range, or adjusting the shader clamp in BarrelDistortionEffect.onUse() to accept values above 1.0 if higher FOVs are intended to have an effect.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/app/gamenative/ui/util/ScreenEffectsConfig.kt, line 180:
<comment>The FOV slider range (40–120°) extends beyond the range that actually takes effect, because `BarrelDistortionEffect.onUse()` clamps the `height` uniform to `[-1.0, 1.0]`. Since `height = tan(VFOV / 2)`, values above VFOV = 90° produce `tan(VFOV/2) > 1.0` and get clamped down to 1.0 — making FOV=100°, FOV=110°, and FOV=120° produce the identical result as FOV=90°. The effective range is therefore 0–90°, so the upper slider bound of 120° is misleading and may confuse users who try higher values expecting a visible difference.
Consider either capping the UI range to 0–90° to match the effect's effective range, or adjusting the shader clamp in `BarrelDistortionEffect.onUse()` to accept values above 1.0 if higher FOVs are intended to have an effect.</comment>
<file context>
@@ -153,6 +173,16 @@ fun applyScreenEffectsConfig(renderer: GLRenderer, config: ScreenEffectsConfig)
+ if (config.enableBarrelDistortion && abs(config.barrelDistortionStrength) > 0.001f) {
+ val barrelEffect = BarrelDistortionEffect().apply {
+ strength = config.barrelDistortionStrength / 100f
+ height = tan(config.barrelDistortionFov * ScreenEffectsConfig.DEGREES_TO_RADIANS / 2f)
+ cylindricalRatio = config.barrelDistortionCylindricalRatio
+ }
</file context>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/src/main/java/app/gamenative/ui/component/ScreenEffectsPanel.kt (1)
895-898: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd the barrel-distortion controls to
ScreenEffectsPanel.This panel now loads, applies, persists, and resets barrel-distortion state, but its rendered toggle list still ends at NTSC. Users therefore cannot enable or adjust barrel distortion from this entry point; mirror the toggle and adjustment block already present in the sibling screen-effects composables.
🤖 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 `@app/src/main/java/app/gamenative/ui/component/ScreenEffectsPanel.kt` around lines 895 - 898, Add the barrel-distortion toggle and adjustment controls to the rendered control list in ScreenEffectsPanel, following the existing implementation in the sibling screen-effects composables. Place them after the NTSC controls and bind them to the existing barrel-distortion state, ensuring users can enable, adjust, persist, and reset the values already handled by the panel.
🤖 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.
Inline comments:
In `@app/src/main/java/com/winlator/renderer/effects/BarrelDistortionEffect.java`:
- Around line 41-42: Remove the backticks surrounding the `@Override` annotation
in BarrelDistortionEffect.onUse so it is valid Java syntax and compilation
succeeds.
---
Duplicate comments:
In `@app/src/main/java/app/gamenative/ui/component/ScreenEffectsPanel.kt`:
- Around line 895-898: Add the barrel-distortion toggle and adjustment controls
to the rendered control list in ScreenEffectsPanel, following the existing
implementation in the sibling screen-effects composables. Place them after the
NTSC controls and bind them to the existing barrel-distortion state, ensuring
users can enable, adjust, persist, and reset the values already handled by the
panel.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 93f77d00-7ee5-4d4c-8df4-7e0b0f57ea8a
📒 Files selected for processing (3)
app/src/main/java/app/gamenative/ui/component/ScreenEffectsPanel.ktapp/src/main/java/app/gamenative/ui/util/ScreenEffectsConfig.ktapp/src/main/java/com/winlator/renderer/effects/BarrelDistortionEffect.java
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/app/gamenative/ui/util/ScreenEffectsConfig.kt
Comment on lines
+41
to
+42
| `@Override` | ||
| protected void onUse(ShaderMaterial material, GLRenderer renderer) { |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="app/src/main/java/com/winlator/renderer/effects/BarrelDistortionEffect.java"
if rg -n '^[[:space:]]*`@Override`' "$file"; then
echo "Invalid backtick-wrapped Java annotation found"
exit 1
fiRepository: utkarshdalal/GameNative
Length of output: 227
Remove the backticks around @Override. This makes BarrelDistortionEffect.java invalid Java and breaks compilation.
🧰 Tools
🪛 PMD (7.26.0)
[High] 41-41: Parse Error: LexException: Lexical error in file 'app/src/main/java/com/winlator/renderer/effects/BarrelDistortionEffect.java' at line 41, column 5: "`" (96), after : "" (in lexical state DEFAULT)
(Parse Error)
🤖 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 `@app/src/main/java/com/winlator/renderer/effects/BarrelDistortionEffect.java`
around lines 41 - 42, Remove the backticks surrounding the `@Override` annotation
in BarrelDistortionEffect.onUse so it is valid Java syntax and compilation
succeeds.
Source: Linters/SAST tools
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
1 participant
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.
Description
Limitations
Recording
out.mp4
Type of Change
Checklist
#code-changes, I have discussed this change there and it has been green-lighted. If I do not have access, I have still provided clear context in this PR. If I skip both, I accept that this change may face delays in review, may not be reviewed at all, or may be closed.CONTRIBUTING.md.Summary by cubic
Add Barrel Zoom (configurable barrel distortion) to screen effects. Improves comfort for 3D/FPS on small screens by centering zoom and reducing edge stretch.
ScreenEffectsConfig.BarrelDistortionEffect; Vulkan addsEFFECT_MASK_BARREL_DISTORTION, extendsVulkanRenderer#setEffect(...)and JNI with barrel params, and updates window shaders and push constants.Written for commit 86bd0a4. Summary will update on new commits.
Summary by CodeRabbit