Skip to content

Add quick menu performance HUD overlay#777

Merged
utkarshdalal merged 12 commits intoutkarshdalal:masterfrom
xXJSONDeruloXx:feature/quickmenu-performance-hud
Mar 12, 2026
Merged

Add quick menu performance HUD overlay#777
utkarshdalal merged 12 commits intoutkarshdalal:masterfrom
xXJSONDeruloXx:feature/quickmenu-performance-hud

Conversation

@xXJSONDeruloXx
Copy link
Copy Markdown
Contributor

@xXJSONDeruloXx xXJSONDeruloXx commented Mar 9, 2026


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

    • Performance HUD overlay: toggle from the General tab (first item); shows FPS (via FrameRating.getCurrentFPS()), CPU/GPU %, RAM, battery, power (W), and CPU/GPU temps; hides rows when unavailable.
    • Reliable lifecycle: updates every second off the main thread; anchors top-left; cleans up on toggle off, on screen dispose, and on game view release; emits performance_hud_toggled.
    • Tabbed Quick Menu (General, Controller) with localized labels and a gradient focus outline across tabs, rows, and the close button.
    • Controller tab: restored Input Controls option.
  • Refactors

    • Removed the legacy “Show FPS” (Mesa overlay) setting, related env vars, and tests; showFPS is 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

    • Performance HUD: floating overlay showing FPS, CPU/GPU usage, RAM, battery, power draw, and temperatures.
    • Quick Menu toggle: enable/disable the Performance HUD directly from the quick menu.
    • UI text: added translatable "Performance HUD" label for the new menu item.
  • Bug Fixes

    • Improved HUD lifecycle handling to avoid duplicate overlays and ensure proper cleanup.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 9, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds 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

Cohort / File(s) Summary
Quick Menu Integration
app/src/main/java/app/gamenative/ui/component/QuickMenu.kt
Added QuickMenuAction.PERFORMANCE_HUD = 6 and new QuickMenu item (icon Icons.Default.QueryStats, label R.string.performance_hud, accent color).
Screen HUD Management
app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
Added nullable performanceHudView/performanceHudHost, updatePerformanceHud(show: Boolean) to attach/detach HUD to nearest FrameLayout (TOP-START) with margins, removePerformanceHud() cleanup, QuickMenu action handling to toggle HUD, and lifecycle/disposal hooks to remove HUD on view teardown.
Performance HUD Widget
app/src/main/java/app/gamenative/ui/widget/PerformanceHudView.kt
New PerformanceHudView (extends FrameLayout) that displays FPS, CPU/GPU/RAM/Battery/Power and temperatures, runs a coroutine update loop, reads system metrics (e.g., /proc/stat, kgsl, ActivityManager, BatteryManager, thermal zones), updates rows, and starts/stops on attach/detach.
FPS Accessor
app/src/main/java/com/winlator/widget/FrameRating.java
Added public float getCurrentFPS() to expose the latest FPS value.
Resources
app/src/main/res/values/strings.xml
Added performance_hud string resource ("Performance HUD").

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
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 I hop in code with metrics bright,

A floating HUD that glows at night,
Tap the menu — numbers bloom,
I count your frames and chase the gloom,
🥕📊 quick hops, clear sight!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 3.85% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title directly and accurately describes the main change: adding a performance HUD overlay feature accessible from the quick menu.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

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.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 8bf5a27 and bffdb18.

📒 Files selected for processing (5)
  • app/src/main/java/app/gamenative/ui/component/QuickMenu.kt
  • app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
  • app/src/main/java/app/gamenative/ui/widget/PerformanceHudView.kt
  • app/src/main/java/com/winlator/widget/FrameRating.java
  • app/src/main/res/values/strings.xml

Comment thread app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt Outdated
Comment thread app/src/main/java/app/gamenative/ui/widget/PerformanceHudView.kt Outdated
@xXJSONDeruloXx xXJSONDeruloXx force-pushed the feature/quickmenu-performance-hud branch from bffdb18 to 9a8be9b Compare March 9, 2026 22:33
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

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.

Comment thread app/src/main/java/app/gamenative/ui/widget/PerformanceHudView.kt
Comment thread app/src/main/java/app/gamenative/ui/widget/PerformanceHudView.kt Outdated
Comment thread app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt Outdated
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.

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 the type file 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

📥 Commits

Reviewing files that changed from the base of the PR and between bffdb18 and 9a8be9b.

📒 Files selected for processing (5)
  • app/src/main/java/app/gamenative/ui/component/QuickMenu.kt
  • app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
  • app/src/main/java/app/gamenative/ui/widget/PerformanceHudView.kt
  • app/src/main/java/com/winlator/widget/FrameRating.java
  • app/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

Comment thread app/src/main/java/app/gamenative/ui/widget/PerformanceHudView.kt Outdated
Comment thread app/src/main/java/app/gamenative/ui/widget/PerformanceHudView.kt Outdated
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

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.

Comment thread app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
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.

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/thermal and rereads every zone type file 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

📥 Commits

Reviewing files that changed from the base of the PR and between 9a8be9b and aee1126.

📒 Files selected for processing (3)
  • app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
  • app/src/main/java/app/gamenative/ui/widget/PerformanceHudView.kt
  • app/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

Comment thread app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
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 (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 for scope is never cancelled. While updateJob is cancelled in stopUpdates(), the parent job remains active. If this view is rapidly attached/detached or if additional coroutines are launched on scope in 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? = null

Then 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 returning null and using updateRow() 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 null

Then 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() performs File.listFiles() and reads multiple type files 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

📥 Commits

Reviewing files that changed from the base of the PR and between aee1126 and be1f89f.

📒 Files selected for processing (1)
  • app/src/main/java/app/gamenative/ui/widget/PerformanceHudView.kt

@phobos665
Copy link
Copy Markdown
Contributor

I think it would also be great to pop these settings in the EditContainerSettings

@xXJSONDeruloXx
Copy link
Copy Markdown
Contributor Author

@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?

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

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.

Comment thread app/src/main/java/app/gamenative/utils/ContainerUtils.kt
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

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.

Comment thread app/src/main/java/app/gamenative/ui/component/QuickMenu.kt
Comment thread app/src/main/res/values/strings.xml
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.

3 participants