Add quick menu performance HUD overlay#777
Add quick menu performance HUD overlay#777utkarshdalal merged 12 commits intoutkarshdalal:masterfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new floating Performance HUD view, a Quick Menu toggle to show/hide it, an FPS accessor, and a string resource; includes HUD attachment to a FrameLayout, periodic metric collection, and lifecycle cleanup to prevent leaks. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant QuickMenu
participant XServerScreen
participant HUD as PerformanceHudView
participant System
User->>QuickMenu: Tap "Performance HUD"
QuickMenu->>XServerScreen: Emit PERFORMANCE_HUD action
XServerScreen->>XServerScreen: updatePerformanceHud(true)
XServerScreen->>HUD: instantiate & attach to FrameLayout (TOP-START)
HUD->>HUD: start metrics loop
loop periodic
HUD->>System: read FPS, CPU, GPU, RAM, Battery, Temps
System-->>HUD: metrics
HUD->>HUD: update UI rows
end
User->>QuickMenu: Tap "Performance HUD" (toggle off)
QuickMenu->>XServerScreen: Emit PERFORMANCE_HUD action
XServerScreen->>HUD: remove/detach and stop loop
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`:
- Around line 362-383: The HUD is being attached to the inner gameHost
FrameLayout found by walking up from xServerView, which causes it to be occluded
by overlays added to the outer frameLayout; instead, capture and use an explicit
reference (e.g., performanceHudHost) to the outer FrameLayout created in the
AndroidView factory (the frameLayout variable) and add the PerformanceHudView to
that host rather than to the parent found from xServerView; set
performanceHudHost = frameLayout when the outer layout is created, use
performanceHudHost?.addView(hud, layoutParams) and hud.bringToFront() to ensure
it sits above overlays, and clear performanceHudHost (and performanceHudView) on
release/dispose to avoid leaks.
In `@app/src/main/java/app/gamenative/ui/widget/PerformanceHudView.kt`:
- Around line 23-24: The updateRunnable currently runs on the main Handler
(Handler(Looper.getMainLooper())) and performs blocking I/O (file reads from
/proc/stat, /sys/class/kgsl/*, thermal zone files, and
context.registerReceiver()), so move all metric collection and parsing off the
UI thread (e.g., use a CoroutineScope with Dispatchers.IO or a dedicated
HandlerThread) inside the class; run the heavy work in a background dispatcher
inside updateRunnable (or replace updateRunnable with a coroutine job) and only
post the final rendered strings/results back to the main Handler for view
updates. Specifically, keep the main Handler/Looper.getMainLooper() usage only
for UI updates and refactor functions referenced from updateRunnable (file-read
logic and the code that calls context.registerReceiver()) to execute on the
IO/background dispatcher, then marshal the prepared display strings back to the
main thread for setText/invalidate calls.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0133bc2b-dcda-47b0-8b42-a5cea0b07c57
📒 Files selected for processing (5)
app/src/main/java/app/gamenative/ui/component/QuickMenu.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/app/gamenative/ui/widget/PerformanceHudView.ktapp/src/main/java/com/winlator/widget/FrameRating.javaapp/src/main/res/values/strings.xml
bffdb18 to
9a8be9b
Compare
There was a problem hiding this comment.
3 issues found across 5 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/java/app/gamenative/ui/widget/PerformanceHudView.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/widget/PerformanceHudView.kt:23">
P2: Synchronous file I/O and system calls on main thread in performance HUD can cause UI jank</violation>
<violation number="2" location="app/src/main/java/app/gamenative/ui/widget/PerformanceHudView.kt:77">
P1: HUD update loop doesn't resume after detach/reattach - polling started only in init but stopped on every onDetachedFromWindow(). If the view is temporarily detached and reattached (common for overlays), stats updates permanently freeze with stale data.</violation>
</file>
<file name="app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt:362">
P2: The parent walk from `xServerView` stops at the first `FrameLayout` it encounters, which is `gameHost`. Since `InputControlsView` and `SwapInputOverlayView` are added to the outer `frameLayout` (a parent of `gameHost`), `bringToFront()` only affects z-order within `gameHost` — the HUD will be hidden behind those overlays. Attach the HUD to the outer `frameLayout` instead by keeping an explicit reference to it.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/ui/widget/PerformanceHudView.kt (1)
197-216: Consider dynamic thermal zone discovery.The hardcoded thermal zone paths (
thermal_zone0,thermal_zone7,thermal_zone11, etc.) are device-specific and may not correspond to CPU/GPU temperatures across different SoCs. The current fallback (hiding the row) is acceptable, but for broader compatibility, consider scanning/sys/class/thermal/and reading thetypefile to identify zones by name (e.g., "cpu-0-0", "gpu", "tsens_tz_sensor*").🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/ui/widget/PerformanceHudView.kt` around lines 197 - 216, The current readCpuTempC and readGpuTempC use hardcoded thermal zone paths which are device-specific; modify them to dynamically discover zones by scanning /sys/class/thermal/, reading each zone's "type" file and matching types that indicate CPU (e.g., names containing "cpu" or "tsens") or GPU (e.g., "gpu" or vendor-specific kgsl), then pass the matched zone temp file paths into readTemperatureC instead of the fixed list; update readCpuTempC and readGpuTempC to prefer discovered matches but fall back to the existing hardcoded paths if no matches are found.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/app/gamenative/ui/widget/PerformanceHudView.kt`:
- Line 168: Replace the implicit-locale String.format call in PerformanceHudView
that formats usedGb with an explicit-locale formatting call (e.g., pass a Locale
such as Locale.US or Locale.getDefault()) so the RAM value is rendered
consistently across locales; update the formatting expression that currently
uses String.format("%.1fGB", usedGb) to include the Locale parameter and add the
necessary java.util.Locale import if missing.
- Around line 83-88: The number formatting in PerformanceHudView uses
String.format without an explicit locale (see fpsText update and the powerText
update where String.format("%.1f", ...) is used), which can yield
locale-dependent decimal separators; update those calls to pass an explicit
locale (e.g., Locale.US or Locale.ROOT) so formatting is consistent across
devices—for example change String.format("%.1f", ...) to
String.format(Locale.US, "%.1f", ...) for the expressions used when setting
fpsText and inside the updateRow for powerText (and any other String.format
usages in this file such as in readPowerWatts handling).
---
Nitpick comments:
In `@app/src/main/java/app/gamenative/ui/widget/PerformanceHudView.kt`:
- Around line 197-216: The current readCpuTempC and readGpuTempC use hardcoded
thermal zone paths which are device-specific; modify them to dynamically
discover zones by scanning /sys/class/thermal/, reading each zone's "type" file
and matching types that indicate CPU (e.g., names containing "cpu" or "tsens")
or GPU (e.g., "gpu" or vendor-specific kgsl), then pass the matched zone temp
file paths into readTemperatureC instead of the fixed list; update readCpuTempC
and readGpuTempC to prefer discovered matches but fall back to the existing
hardcoded paths if no matches are found.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 27706c13-e659-4281-af92-7b86f5a807e8
📒 Files selected for processing (5)
app/src/main/java/app/gamenative/ui/component/QuickMenu.ktapp/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/app/gamenative/ui/widget/PerformanceHudView.ktapp/src/main/java/com/winlator/widget/FrameRating.javaapp/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/res/values/strings.xml
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
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/java/app/gamenative/ui/screen/xserver/XServerScreen.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt:374">
P2: HUD host caching breaks overlay placement during external display swaps</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/main/java/app/gamenative/ui/widget/PerformanceHudView.kt (1)
246-283: Cache thermal sensor discovery instead of rescanning every tick.
discoverThermalZoneTempPaths()walks/sys/class/thermaland rereads every zonetypefile on every update for both CPU and GPU. At a 1s interval that's avoidable background I/O, and the filesystem iteration order is not guaranteed, so the chosen sensor can jump between updates. Cache the candidate lists once per HUD instance and reuse them.♻️ Possible direction
+ private val cpuTempPaths by lazy { + val discovered = discoverThermalZoneTempPaths { type -> + type.contains("cpu") || type.contains("tsens") + }.sorted() + (discovered + listOf( + "/sys/class/thermal/thermal_zone0/temp", + "/sys/class/thermal/thermal_zone1/temp", + "/sys/class/thermal/thermal_zone7/temp", + "/sys/class/thermal/thermal_zone10/temp", + "/sys/class/thermal/thermal_zone11/temp", + )).distinct() + } + + private val gpuTempPaths by lazy { + val discovered = discoverThermalZoneTempPaths { type -> + type.contains("gpu") || type.contains("kgsl") + }.sorted() + (listOf("/sys/class/kgsl/kgsl-3d0/temp") + + discovered + + listOf("/sys/class/thermal/thermal_zone11/temp") + ).distinct() + } + private fun readCpuTempC(): Int? { - return readTemperatureC( - discoverThermalZoneTempPaths { type -> - type.contains("cpu") || type.contains("tsens") - } + listOf( - "/sys/class/thermal/thermal_zone0/temp", - "/sys/class/thermal/thermal_zone1/temp", - "/sys/class/thermal/thermal_zone7/temp", - "/sys/class/thermal/thermal_zone10/temp", - "/sys/class/thermal/thermal_zone11/temp", - ), - ) + return readTemperatureC(cpuTempPaths) } private fun readGpuTempC(): Int? { - return readTemperatureC( - listOf("/sys/class/kgsl/kgsl-3d0/temp") + - discoverThermalZoneTempPaths { type -> - type.contains("gpu") || type.contains("kgsl") - } + - listOf("/sys/class/thermal/thermal_zone11/temp"), - ) + return readTemperatureC(gpuTempPaths) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/ui/widget/PerformanceHudView.kt` around lines 246 - 283, discoverThermalZoneTempPaths is being called on every tick from readCpuTempC and readGpuTempC causing repeated I/O and inconsistent ordering; cache the discovered sensor path lists once on the HUD instance and reuse them. Add lazy properties (e.g., cpuTempPaths and gpuTempPaths) that call discoverThermalZoneTempPaths (and append the same hardcoded fallback paths) only once (or on-demand if empty), then have readCpuTempC and readGpuTempC use those cached lists instead of calling discoverThermalZoneTempPaths each time; keep discoverThermalZoneTempPaths unchanged so it remains the single discovery function.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`:
- Around line 355-392: The HUD visibility is using the actual View instance
(performanceHudView) as the source-of-truth which causes it to be lost when the
host is rebuilt; introduce a separate remembered boolean state (e.g.,
showPerformanceHud by remember { mutableStateOf(false) }) and drive
attachment/detachment from that flag instead of performanceHudView; update
updatePerformanceHud(show: Boolean) to set that flag, always attempt to attach
the HUD when show flag is true and performanceHudHost is non-null (reattach if
host changed), and have removePerformanceHud only clear the view
(performanceHudView) while leaving the boolean flag intact so rebuilds reattach
the HUD to a new performanceHudHost; apply the same pattern for the other
referenced locations (lines ~660-667, ~878-879, ~1365-1368) referencing
performanceHudView/performanceHudHost.
---
Nitpick comments:
In `@app/src/main/java/app/gamenative/ui/widget/PerformanceHudView.kt`:
- Around line 246-283: discoverThermalZoneTempPaths is being called on every
tick from readCpuTempC and readGpuTempC causing repeated I/O and inconsistent
ordering; cache the discovered sensor path lists once on the HUD instance and
reuse them. Add lazy properties (e.g., cpuTempPaths and gpuTempPaths) that call
discoverThermalZoneTempPaths (and append the same hardcoded fallback paths) only
once (or on-demand if empty), then have readCpuTempC and readGpuTempC use those
cached lists instead of calling discoverThermalZoneTempPaths each time; keep
discoverThermalZoneTempPaths unchanged so it remains the single discovery
function.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 36c08ba4-1562-47aa-8cee-b1168396ce79
📒 Files selected for processing (3)
app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.ktapp/src/main/java/app/gamenative/ui/widget/PerformanceHudView.ktapp/src/main/java/com/winlator/widget/FrameRating.java
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/winlator/widget/FrameRating.java
There was a problem hiding this comment.
🧹 Nitpick comments (3)
app/src/main/java/app/gamenative/ui/widget/PerformanceHudView.kt (3)
37-38: CoroutineScope's Job is never cancelled, potential resource leak.The
SupervisorJob()created forscopeis never cancelled. WhileupdateJobis cancelled instopUpdates(), the parent job remains active. If this view is rapidly attached/detached or if additional coroutines are launched onscopein the future, this could cause leaks.♻️ Suggested fix: Cancel the scope's job on detach
- private val scope = CoroutineScope(SupervisorJob() + Dispatchers.Main.immediate) + private val parentJob = SupervisorJob() + private val scope = CoroutineScope(parentJob + Dispatchers.Main.immediate) private var updateJob: Job? = nullThen update
onDetachedFromWindow:override fun onDetachedFromWindow() { stopUpdates() + parentJob.cancel() super.onDetachedFromWindow() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/ui/widget/PerformanceHudView.kt` around lines 37 - 38, The SupervisorJob-backed CoroutineScope (scope) in PerformanceHudView is never cancelled, risking resource leaks; update onDetachedFromWindow to cancel the scope's job (call scope.cancel()) when the view is detached, and also ensure you still call stopUpdates() (which cancels updateJob) before or after cancelling the scope so all coroutines launched on scope are cleaned up; locate the scope declaration and the onDetachedFromWindow/stopUpdates methods to add the cancellation.
210-222: RAM row shows "—" instead of hiding when unavailable.The class documentation states "rows are hidden automatically when a given stat is not available," but
readUsedRamText()returns "—" on failure (line 211), causing the RAM row to always remain visible. Consider returningnulland usingupdateRow()for consistency with other optional metrics.♻️ Suggested fix for consistency
- private fun readUsedRamText(): String { - val activityManager = context.getSystemService(Context.ACTIVITY_SERVICE) as? ActivityManager ?: return "—" + private fun readUsedRamText(): String? { + val activityManager = context.getSystemService(Context.ACTIVITY_SERVICE) as? ActivityManager ?: return nullThen update
collectSnapshot:- ram = "RAM ${readUsedRamText()}", + ram = readUsedRamText()?.let { "RAM $it" },And update
renderSnapshot:- ramText.text = snapshot.ram + updateRow(ramText, snapshot.ram)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/ui/widget/PerformanceHudView.kt` around lines 210 - 222, The RAM row should be hidden when RAM is unavailable: change readUsedRamText() to return null instead of the placeholder "—" when ActivityManager or memory info can't be obtained, then adjust collectSnapshot to accept the nullable result and pass it through, and update renderSnapshot to call updateRow("ram", value) (or similar row key) so the row is removed when the value is null; reference readUsedRamText(), collectSnapshot, renderSnapshot, and updateRow() when making the edits for consistency with other optional metrics.
263-276: Consider caching thermal zone paths.
discoverThermalZoneTempPaths()performsFile.listFiles()and reads multipletypefiles every second. Since thermal zones don't change at runtime, these paths could be discovered once and cached.♻️ Suggested optimization
// Add as class properties private var cpuTempPaths: List<String>? = null private var gpuTempPaths: List<String>? = null private fun readCpuTempC(): Int? { val paths = cpuTempPaths ?: discoverThermalZoneTempPaths { type -> type.contains("cpu") || type.contains("tsens") }.also { cpuTempPaths = it } return readTemperatureC(paths) } private fun readGpuTempC(): Int? { val paths = gpuTempPaths ?: ( listOf("/sys/class/kgsl/kgsl-3d0/temp") + discoverThermalZoneTempPaths { type -> type.contains("gpu") || type.contains("kgsl") } ).also { gpuTempPaths = it } return readTemperatureC(paths) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/app/gamenative/ui/widget/PerformanceHudView.kt` around lines 263 - 276, discoverThermalZoneTempPaths is called every second and re-reads filesystem; cache the discovered temp paths in class properties (e.g., add private var cpuTempPaths: List<String>? = null and private var gpuTempPaths: List<String>? = null) and update readCpuTempC and readGpuTempC to use the cached lists if present, otherwise call discoverThermalZoneTempPaths(...) once and store the result; for GPU also prepend the known "/sys/class/kgsl/kgsl-3d0/temp" path before caching; keep discoverThermalZoneTempPaths unchanged except for being used as the one-time discovery function.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@app/src/main/java/app/gamenative/ui/widget/PerformanceHudView.kt`:
- Around line 37-38: The SupervisorJob-backed CoroutineScope (scope) in
PerformanceHudView is never cancelled, risking resource leaks; update
onDetachedFromWindow to cancel the scope's job (call scope.cancel()) when the
view is detached, and also ensure you still call stopUpdates() (which cancels
updateJob) before or after cancelling the scope so all coroutines launched on
scope are cleaned up; locate the scope declaration and the
onDetachedFromWindow/stopUpdates methods to add the cancellation.
- Around line 210-222: The RAM row should be hidden when RAM is unavailable:
change readUsedRamText() to return null instead of the placeholder "—" when
ActivityManager or memory info can't be obtained, then adjust collectSnapshot to
accept the nullable result and pass it through, and update renderSnapshot to
call updateRow("ram", value) (or similar row key) so the row is removed when the
value is null; reference readUsedRamText(), collectSnapshot, renderSnapshot, and
updateRow() when making the edits for consistency with other optional metrics.
- Around line 263-276: discoverThermalZoneTempPaths is called every second and
re-reads filesystem; cache the discovered temp paths in class properties (e.g.,
add private var cpuTempPaths: List<String>? = null and private var gpuTempPaths:
List<String>? = null) and update readCpuTempC and readGpuTempC to use the cached
lists if present, otherwise call discoverThermalZoneTempPaths(...) once and
store the result; for GPU also prepend the known "/sys/class/kgsl/kgsl-3d0/temp"
path before caching; keep discoverThermalZoneTempPaths unchanged except for
being used as the one-time discovery function.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 649c29e9-814e-4245-87ba-c60e5c9ead7d
📒 Files selected for processing (1)
app/src/main/java/app/gamenative/ui/widget/PerformanceHudView.kt
|
I think it would also be great to pop these settings in the EditContainerSettings |
|
@phobos665 since they're hot toggles, what's the benefit there, like to have it default to on or off on container init as the setting? |
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
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/java/app/gamenative/utils/ContainerUtils.kt">
<violation number="1" location="app/src/main/java/app/gamenative/utils/ContainerUtils.kt:443">
P1: `showFPS` is being hardcoded to `false` everywhere instead of properly round-tripping the user's existing preference, causing silent data loss when containers are saved/recreated.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
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/java/app/gamenative/ui/component/QuickMenu.kt">
<violation number="1" location="app/src/main/java/app/gamenative/ui/component/QuickMenu.kt:106">
P1: The `INPUT_CONTROLS` quick-menu action was removed during the tab refactor, making the 'Toggle Input Controls' feature inaccessible to users, even though the handler code in `XServerScreen.kt` (line 560) still expects this action</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Summary by cubic
Adds a Performance HUD toggle in the Quick Menu to show live FPS and system stats in-game, and introduces a tabbed Quick Menu (General, Controller) with clearer controller focus and gradient outlines.
New Features
FrameRating.getCurrentFPS()), CPU/GPU %, RAM, battery, power (W), and CPU/GPU temps; hides rows when unavailable.performance_hud_toggled.Refactors
showFPSis now always false. Updated the boot tip to point to the new HUD.Written for commit da6b396. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes