feat: Add QAM display brightness control#1525
Open
xXJSONDeruloXx wants to merge 3 commits into
Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a display brightness adjustment control to the screen effects panel, extends BrightnessManager with activity lookup/read/snap/apply helpers, places a new DisplayBrightnessRow composable at the top of GL and Vulkan tabs, and adds localized ChangesDisplay Brightness Control
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
xXJSONDeruloXx
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: 1
🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/ui/component/ScreenEffectsPanel.kt (1)
157-159: ⚡ Quick winConsider guarding against null activity for better UX.
When
findActivityreturns null (line 157), the brightness controls still render but have no effect:setDisplayBrightnessupdates local state (line 164) but theactivity?.letat line 165 prevents any actual brightness change. This creates a confusing UX where the slider appears interactive but does nothing.While activity should rarely be null in normal Compose UI contexts, consider either:
- Not rendering the row when activity is null
- Showing a disabled/grayed-out state
- Adding a brief log or comment explaining the fallback behavior
Example: skip rendering when activity is null
`@Composable` private fun DisplayBrightnessRow( focusRequester: FocusRequester? = null, ) { val context = LocalContext.current val activity = remember(context) { BrightnessManager.findActivity(context) } + + // Don't render brightness controls if we can't find an Activity to control + if (activity == null) return + var displayBrightness by remember(activity) { - mutableFloatStateOf(activity?.let(BrightnessManager::readDisplayBrightness) ?: 0.5f) + mutableFloatStateOf(BrightnessManager.readDisplayBrightness(activity)) } fun setDisplayBrightness(value: Float) { val next = BrightnessManager.snapDisplayBrightness(value) displayBrightness = next - activity?.let { BrightnessManager.applyDisplayBrightness(it, next) } + BrightnessManager.applyDisplayBrightness(activity, next) }🤖 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 157 - 159, The brightness slider currently renders even when BrightnessManager.findActivity(context) returns null, so setDisplayBrightness updates local displayBrightness but activity?.let(BrightnessManager::setDisplayBrightness) is skipped; update ScreenEffectsPanel to guard on the activity from remember(context) { BrightnessManager.findActivity(context) }—either skip rendering the brightness Row entirely when activity is null, or render it in a disabled/greyed-out state (disable the Slider and related controls) and reflect that in displayBrightness state; also add a brief log or comment near BrightnessManager.findActivity and where setDisplayBrightness is called to document the fallback behavior so UX is not misleading.
🤖 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/utils/BrightnessManager.kt`:
- Around line 57-71: The readDisplayBrightness(Activity) function reads UI state
via activity.window.attributes and must be executed on Android's main thread;
add the `@MainThread` annotation to the readDisplayBrightness method (the same
annotation already used on applyDisplayBrightness) so the compiler/lint enforce
main-thread usage and keep consistency in BrightnessManager; ensure you import
androidx.annotation.MainThread if not already imported.
---
Nitpick comments:
In `@app/src/main/java/app/gamenative/ui/component/ScreenEffectsPanel.kt`:
- Around line 157-159: The brightness slider currently renders even when
BrightnessManager.findActivity(context) returns null, so setDisplayBrightness
updates local displayBrightness but
activity?.let(BrightnessManager::setDisplayBrightness) is skipped; update
ScreenEffectsPanel to guard on the activity from remember(context) {
BrightnessManager.findActivity(context) }—either skip rendering the brightness
Row entirely when activity is null, or render it in a disabled/greyed-out state
(disable the Slider and related controls) and reflect that in displayBrightness
state; also add a brief log or comment near BrightnessManager.findActivity and
where setDisplayBrightness is called to document the fallback behavior so UX is
not misleading.
🪄 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: eb37c767-a8af-4047-a65c-44933e91dff5
📒 Files selected for processing (3)
app/src/main/java/app/gamenative/ui/component/ScreenEffectsPanel.ktapp/src/main/java/app/gamenative/utils/BrightnessManager.ktapp/src/main/res/values/strings.xml
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
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
2 participants
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
Adds a transient Display brightness slider to QAM screen effects for adjusting the current app window brightness.
prevents the annoying android paradigm of swiping multiple times to show status bar then pull notification shade then expand shade then finally move brightness slider, this way it can be controller driven and done in very few presses.
Recording
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 a display brightness slider to the QAM Screen Effects panel to adjust the current app window’s brightness in real time. Values snap to 5% steps (5%–100%) and only affect the active window, not system brightness.
BrightnessManagerwith 5% snapping, safe Activity detection, and clamping.display_brightnessstring with translations across supported locales.Written for commit 0b7c54e. Summary will update on new commits.
Summary by CodeRabbit