Skip to content

feat: add per-game backdrop images for letterboxing#1002

Open
xXJSONDeruloXx wants to merge 9 commits intoutkarshdalal:masterfrom
xXJSONDeruloXx:feat/per-game-backdrop
Open

feat: add per-game backdrop images for letterboxing#1002
xXJSONDeruloXx wants to merge 9 commits intoutkarshdalal:masterfrom
xXJSONDeruloXx:feat/per-game-backdrop

Conversation

@xXJSONDeruloXx
Copy link
Copy Markdown
Contributor

@xXJSONDeruloXx xXJSONDeruloXx commented Mar 24, 2026

https://discord.com/channels/1378308569287622737/1485707057108881519

Especially for 21:9 devices, allow using hero image or user provided image in background of renderer to mitigate black bars without stretching:
image

This example I'm making the game 4:3 so you can still see the effect in 16:9 example


Summary by cubic

Adds per-game backdrop images behind letterboxed content with hero/cover art fallback and a soft viewport shadow. Opt-in per container.

  • New Features

    • Settings: “Backdrop Images” toggle in General; when enabled, pick a custom image and clear via delete. Hidden for default configs.
    • Backdrop source: uses saved backdropImageUri, else falls back to game art (Steam hero/header, Custom grid hero, GOG image/icon, Epic portrait/cover/square/logo, Amazon hero/art). Images load via coil.
    • Rendering: center-crops the backdrop, draws it behind the viewport, and adds a subtle edge shadow; render order works with the magnifier.
  • Bug Fixes

    • Robust handling of persisted URI permissions when selecting, replacing, clearing, dismissing, or saving; release stale grants.
    • Refresh backdrop texture on surface recreation to avoid stale textures; safely handle canceled image loads; skip store metadata lookups when a custom backdrop image is set.
    • Fix container settings verification error when saving after backdrop changes.

Written for commit 6a707b6. Summary will update on new commits.

Summary by CodeRabbit

  • New Features

    • Choose, persist, and clear a custom backdrop image per container; settings show selection status, picker/clear controls, and are hidden for default configs
    • Renderer shows chosen backdrops with aspect-preserving scaling and subtle edge shadowing around the viewport; falls back to game hero/art when no custom image
    • New UI strings for backdrop controls
  • Bug Fixes

    • Cleaner handling and release of persisted URI permissions when saving, replacing, or dismissing dialogs

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 24, 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 container-level backdrop image feature: dialog picker with persistable-URI handling and cleanup, persistence in ContainerData/ContainerUtils, prioritized image resolution and Coil loading in XServerScreen, and OpenGL backdrop rendering plus viewport shadowing in the renderer.

Changes

Cohort / File(s) Summary
Dialog & UI State
app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt, app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigState.kt, app/src/main/java/app/gamenative/ui/component/dialog/GeneralTab.kt
Added backdrop image picker launcher and clear callback; best-effort take/release of persistable URI permissions; dismissal/save now perform backdrop-permission cleanup; General tab shows conditional SettingsMenuLink and inline clear action.
Persistence / Model
app/src/main/java/com/winlator/container/ContainerData.kt, app/src/main/java/app/gamenative/utils/ContainerUtils.kt
Added backdropImageUri field to ContainerData (Saver persistence) and read/write mapping to Container extras.
Image resolution & loading
app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
New suspend helpers to build ordered backdrop candidate list from container extras and game sources; Coil-based bitmap loading (allowHardware=false) and LaunchedEffect to apply first successful bitmap or clear backdrop.
Renderer & Shaders
app/src/main/java/com/winlator/renderer/GLRenderer.java, app/src/main/java/com/winlator/renderer/material/ShaderMaterial.java, app/src/main/java/com/winlator/renderer/material/ShadowMaterial.java
Added backdrop bitmap lifecycle, GL texture creation/upload/delete, renderBackdrop and viewport shadow rendering, new ShadowMaterial shader class, ShaderMaterial.setUniformVec4 helper; exposed setBackdropBitmap and clearBackdrop.
Localization
app/src/main/res/values/strings.xml
Added strings for backdrop UI (label, description, selected status, clear action).

Sequence Diagram

sequenceDiagram
    participant User
    participant Dialog
    participant State
    participant ContainerData
    participant XServerScreen
    participant Coil
    participant Renderer

    User->>Dialog: open dialog / pick image
    Dialog->>Dialog: takePersistableUriPermission(...)
    Dialog->>State: set config.backdropImageUri
    State->>ContainerData: persist backdropImageUri (ContainerUtils)

    Note over XServerScreen,Coil: LaunchedEffect resolves and loads candidates
    XServerScreen->>XServerScreen: resolveBackdropImageModels()
    XServerScreen->>Coil: request bitmap for candidate URLs
    Coil-->>XServerScreen: bitmap or failure
    alt bitmap loaded
        XServerScreen->>Renderer: setBackdropBitmap(bitmap)
        Renderer->>Renderer: ensureBackdropTexture()/upload
        Renderer->>Renderer: renderBackdrop() and renderViewportShadow()
    else all fail
        XServerScreen->>Renderer: clearBackdrop()
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Suggested reviewers

  • phobos665
  • DanielJVoxSmart

Poem

🐰 I hop and I pick a picture bright,
I hold its URI with gentle care.
Bitmaps bloom, shaders paint the light,
Shadows curl around the game’s lair,
A cozy backdrop for each play-night.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ⚠️ Warning PR description references Discord discussion and includes auto-generated summary, but lacks structured template sections for explanation and recording. Complete the description template: add clear 'Description' section explaining the changes, and provide a recording/GIF as required by the checklist.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding per-game backdrop images for letterboxing. It is directly related to the primary feature being implemented across multiple files.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@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 10 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/screen/xserver/XServerScreen.kt">

<violation number="1" location="app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt:332">
P2: Catching `Throwable` in suspend backdrop loading swallows coroutine cancellation and can allow stale `LaunchedEffect` work to update renderer state.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

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: 6

🤖 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/component/dialog/ContainerConfigDialog.kt`:
- Around line 836-849: The backdropImagePickerLauncher callback currently takes
a new persistable URI permission but never revokes any previous grant; update
the launcher (rememberLauncherForActivityResult) to parse the existing
config.backdropImageUri, and if non-null, call
context.contentResolver.releasePersistableUriPermission(Uri.parse(oldUri),
Intent.FLAG_GRANT_READ_URI_PERMISSION) inside a try/catch for SecurityException
before taking the new permission and setting config =
config.copy(backdropImageUri = uri.toString()); likewise, when the image is
cleared (e.g., in GeneralTab's clear button handler that updates
config.backdropImageUri to null/empty), add similar logic to
releasePersistableUriPermission for the old URI before clearing the config to
avoid accumulating stale grants.

In `@app/src/main/java/app/gamenative/ui/component/dialog/GeneralTab.kt`:
- Around line 327-333: The delete IconButton currently sets the Icon's
contentDescription to null, making the clear action inaccessible; update the
Icon (inside IconButton) to provide an appropriate, localized contentDescription
(e.g., via stringResource like a new R.string.clear_backdrop_image or an
existing descriptive string) so screen readers announce the button's purpose,
while keeping the onClick that calls state.config.value =
config.copy(backdropImageUri = "") unchanged.

In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`:
- Around line 268-270: The Steam branch currently adds steamAppInfo.headerUrl
before steamAppInfo.getHeroUrl(), which makes the hero art fallback unreachable;
change the order so getHeroUrl() is evaluated and added to models first (call
steamAppInfo?.getHeroUrl()?.takeIf { it.isNotEmpty() }?.let(models::add) before
steamAppInfo?.headerUrl?.takeIf { it.isNotEmpty() }?.let(models::add)) so hero
art is prioritized, keeping the existing null/empty guards intact.
- Around line 314-318: The ImageRequest currently decodes images at full
resolution (ImageRequest.Builder with .data(imageModel)), which can cause
OOM/GL_MAX_TEXTURE_SIZE issues; update the request to cap decoding to the
renderer surface dimensions by passing the surfaceWidth and surfaceHeight into
the request (e.g., ImageRequest.Builder(...).data(imageModel).size(surfaceWidth,
surfaceHeight) or use scale/resolution APIs available) so the decoded bitmap is
bounded to the target surface before upload; keep allowHardware(false) as-is and
apply this change where ImageLoader and ImageRequest.Builder are used in
XServerScreen (reference: imageModel, ImageLoader, ImageRequest.Builder,
surfaceWidth, surfaceHeight).
- Around line 332-334: In XServerScreen.kt replace the broad "catch (e:
Throwable)" used when loading the backdrop image model with a handler that
preserves coroutine cancellation: catch only Exception and explicitly re-throw
CancellationException (e.g., if (e is CancellationException) throw e) before
logging with Timber, so the LaunchedEffect/ coroutine scope for the backdrop
loader is allowed to cancel promptly when appId changes; update the catch in the
backdrop image-loading block accordingly.

In `@app/src/main/java/com/winlator/renderer/GLRenderer.java`:
- Around line 410-426: setBackdropBitmap and clearBackdrop are mutating GL state
and shared fields (backdropBitmap, backdropTextureId, backdropTextureDirty) off
the render thread and calling GLES20.glDeleteTextures via
deleteBackdropTexture(), which causes GL context violations and data races; fix
by moving all GL calls and mutations of backdropTextureId/backdropTextureDirty
into the GL render thread: have setBackdropBitmap/clearBackdrop only update the
Java-side bitmap reference (backdropBitmap) and post a runnable to the renderer
thread (e.g., via xServerView.queueEvent or the renderer's task queue) that
calls deleteBackdropTexture(), sets backdropTextureDirty, and updates
backdropTextureId as needed, or alternatively enqueue a small command like
renderer.markBackdropForUpdate(bitmap) that the render thread will consume
inside ensureBackdropTexture()/renderBackdrop(); ensure deleteBackdropTexture()
is only ever invoked on the render thread.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c2d0ce32-e9b0-44bf-9a46-fe58697afd7f

📥 Commits

Reviewing files that changed from the base of the PR and between 556af27 and 1de0c00.

📒 Files selected for processing (10)
  • app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigState.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/GeneralTab.kt
  • app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
  • app/src/main/java/app/gamenative/utils/ContainerUtils.kt
  • app/src/main/java/com/winlator/container/ContainerData.kt
  • app/src/main/java/com/winlator/renderer/GLRenderer.java
  • app/src/main/java/com/winlator/renderer/material/ShaderMaterial.java
  • app/src/main/java/com/winlator/renderer/material/ShadowMaterial.java
  • app/src/main/res/values/strings.xml

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 6 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/dialog/ContainerConfigDialog.kt">

<violation number="1" location="app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt:938">
P2: Old backdrop URI permission is revoked before save completion is confirmed, which can break existing backdrop access if save fails/asynchronously errors.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

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/screen/xserver/XServerScreen.kt (1)

449-465: Consider deferring the decode until the surface has a real size.

This effect usually starts as soon as xServerView is assigned, which is earlier than the first non-zero renderer/view dimensions. Falling back to displayMetrics is fine as a fallback, but waiting for the measured surface would keep the decode target aligned with post-attach and resize paths too.

🤖 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/screen/xserver/XServerScreen.kt` around
lines 449 - 465, The effect should wait for a real surface size before decoding:
in the LaunchedEffect that uses xServerView and appId, replace the immediate
decode logic with a size-wait — capture currentXServerView and renderer, then
suspend until renderer.surfaceWidth and renderer.surfaceHeight (or
currentXServerView.width/height) become > 0 (for example using snapshotFlow {
Pair(renderer.surfaceWidth, renderer.surfaceHeight) } and filter/first {
it.first > 0 && it.second > 0 }), then compute targetWidth/targetHeight and call
loadBackdropBitmap, and finally call renderer.setBackdropBitmap or
renderer.clearBackdrop; keep the same function names (LaunchedEffect,
xServerView, renderer, loadBackdropBitmap, setBackdropBitmap, clearBackdrop) so
the decode happens only after a real measured surface exists.
🤖 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/com/winlator/renderer/GLRenderer.java`:
- Around line 410-429: setBackdropBitmap and clearBackdrop currently recycle
incoming Bitmap instances even though loadBackdropBitmap can return either
cached (borrowed) bitmaps from Coil or newly created (owned) bitmaps, which
causes cache corruption or leaks. Fix by choosing one of two consistent
ownership strategies: (A) Make loadBackdropBitmap always return an owned copy
(use Bitmap.createBitmap on the drawable.bitmap) so the renderer can safely
recycle in setBackdropBitmap and clearBackdrop (update loadBackdropBitmap, then
keep recycle calls in setBackdropBitmap/clearBackdrop); or (B) Treat bitmaps as
borrowed everywhere: remove the recycle calls from setBackdropBitmap and
clearBackdrop, and ensure all callers that create owned bitmaps (e.g., any
Bitmap.createBitmap usage) are responsible for recycling them and document
ownership in loadBackdropBitmap and the renderer APIs. Apply one strategy
consistently and update comments/docs accordingly for loadBackdropBitmap,
setBackdropBitmap, and clearBackdrop.

---

Nitpick comments:
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`:
- Around line 449-465: The effect should wait for a real surface size before
decoding: in the LaunchedEffect that uses xServerView and appId, replace the
immediate decode logic with a size-wait — capture currentXServerView and
renderer, then suspend until renderer.surfaceWidth and renderer.surfaceHeight
(or currentXServerView.width/height) become > 0 (for example using snapshotFlow
{ Pair(renderer.surfaceWidth, renderer.surfaceHeight) } and filter/first {
it.first > 0 && it.second > 0 }), then compute targetWidth/targetHeight and call
loadBackdropBitmap, and finally call renderer.setBackdropBitmap or
renderer.clearBackdrop; keep the same function names (LaunchedEffect,
xServerView, renderer, loadBackdropBitmap, setBackdropBitmap, clearBackdrop) so
the decode happens only after a real measured surface exists.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4efe209f-3321-464a-b82d-0131904bdd5d

📥 Commits

Reviewing files that changed from the base of the PR and between 1de0c00 and ad80aee.

📒 Files selected for processing (6)
  • app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigState.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/GeneralTab.kt
  • app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
  • app/src/main/java/com/winlator/renderer/GLRenderer.java
  • app/src/main/res/values/strings.xml

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 252-308: resolveBackdropImageModels currently always runs store
metadata fallbacks even when a user-selected backdrop
(container.getExtra("backdropImageUri", "")) is present, delaying the chosen
image; change the logic to add the saved URI to models first and attempt to
validate/decode it (e.g., a quick local/file/URL reachability or ImageDecoder
check) and if the validation succeeds return that as the result immediately,
otherwise proceed to the GameSource switch (Steam/GOG/EPIC/AMAZON/CUSTOM_GAME)
to collect fallback URLs into models; update resolveBackdropImageModels to
short-circuit to models when the saved backdrop is valid and only perform the
store lookups on decode/fetch failure.
- Around line 450-455: LaunchedEffect is decoding using fallback displayMetrics
before the real render size is available; update the effect that references
xServerView/renderer to wait for a real surface/view size before resolving
images: inside the LaunchedEffect (the block that computes
targetWidth/targetHeight and calls resolveBackdropImageModels), if
renderer.surfaceWidth/renderer.surfaceHeight and
currentXServerView.width/currentXServerView.height are all zero (i.e. you would
fall back to context.resources.displayMetrics), suspend until a non-zero size is
observed (e.g., use snapshotFlow watching
renderer.surfaceWidth/renderer.surfaceHeight/currentXServerView.width/currentXServerView.height
and awaitFirst non-zero) and only then compute targetWidth/targetHeight and call
resolveBackdropImageModels so the backdrop is decoded at the real surface size.
🪄 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: 7d640add-9b6a-42d4-98df-810954a64a2c

📥 Commits

Reviewing files that changed from the base of the PR and between ad80aee and d46820c.

📒 Files selected for processing (5)
  • app/src/main/java/app/gamenative/ui/component/dialog/GeneralTab.kt
  • app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt
  • app/src/main/java/app/gamenative/utils/ContainerUtils.kt
  • app/src/main/java/com/winlator/container/ContainerData.kt
  • app/src/main/res/values/strings.xml
✅ Files skipped from review due to trivial changes (1)
  • app/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (3)
  • app/src/main/java/app/gamenative/utils/ContainerUtils.kt
  • app/src/main/java/com/winlator/container/ContainerData.kt
  • app/src/main/java/app/gamenative/ui/component/dialog/GeneralTab.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

♻️ Duplicate comments (2)
app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt (2)

261-264: ⚠️ Potential issue | 🟠 Major

Keep store-art fallbacks available when the saved backdrop URI is stale.

Lines 261-264 return as soon as backdropImageUri exists, so a deleted file or lost SAF grant leaves the game with no backdrop instead of falling back to hero/cover art. Try the saved URI first, but only short-circuit after it decodes successfully; otherwise continue to the source-specific candidates.

🤖 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/screen/xserver/XServerScreen.kt` around
lines 261 - 264, The current logic short-circuits and returns models as soon as
container.getExtra("backdropImageUri", "") yields a non-empty string, which
prevents falling back to store art when the saved URI is stale; update the block
around container.getExtra/backdropImageUri inside the withContext so you attempt
to resolve/decode/validate the saved userUri (e.g., try opening/decoding the URI
or checking SAF permission) and only return models.toList() if that validation
succeeds, otherwise do not return early and allow the subsequent source-specific
candidates (hero/cover art) to be added to models and returned at the end.

453-469: ⚠️ Potential issue | 🟡 Minor

Wait for a real surface size before decoding.

Lines 456-457 fall back to context.resources.displayMetrics when the renderer/view is still 0x0, and this effect is keyed only by xServerView / appId. In external-display or swap flows that can decode once at the wrong size and never refresh. Suspend until a non-zero renderer/view size is available, or include size changes in the effect key.

🤖 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/screen/xserver/XServerScreen.kt` around
lines 453 - 469, The LaunchedEffect currently decodes/backdrops using fallback
display metrics when renderer or view sizes are zero, causing one-time
wrong-size decoding; update the effect to wait for a real non-zero surface size
or to re-run on size changes by including the renderer/view size in the effect
key (e.g., add renderer.surfaceWidth, renderer.surfaceHeight,
currentXServerView.width/height) or suspend/collect until those sizes are >0
(using snapshotFlow or a small retry loop) before calling
resolveBackdropImageModels/loadBackdropBitmap and
renderer.setBackdropBitmap/clearBackdrop so decoding only happens once a valid
targetWidth/targetHeight are available.
🤖 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 328-330: The loaded BitmapDrawable.bitmap is being returned
directly from the image load (via loader.execute(request) and SuccessResult) and
later recycled by GLRenderer.setBackdropBitmap()/clearBackdrop(), which may
recycle a cached Coil bitmap; change the loader result handling to detect
BitmapDrawable and return a copy of drawable.bitmap (e.g.,
bitmap.copy(bitmap.config ?: Bitmap.Config.ARGB_8888, true)) so the renderer
owns and can recycle the copy without poisoning Coil's cache; keep the rest of
the flow (loader.execute, SuccessResult) unchanged.

---

Duplicate comments:
In `@app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.kt`:
- Around line 261-264: The current logic short-circuits and returns models as
soon as container.getExtra("backdropImageUri", "") yields a non-empty string,
which prevents falling back to store art when the saved URI is stale; update the
block around container.getExtra/backdropImageUri inside the withContext so you
attempt to resolve/decode/validate the saved userUri (e.g., try opening/decoding
the URI or checking SAF permission) and only return models.toList() if that
validation succeeds, otherwise do not return early and allow the subsequent
source-specific candidates (hero/cover art) to be added to models and returned
at the end.
- Around line 453-469: The LaunchedEffect currently decodes/backdrops using
fallback display metrics when renderer or view sizes are zero, causing one-time
wrong-size decoding; update the effect to wait for a real non-zero surface size
or to re-run on size changes by including the renderer/view size in the effect
key (e.g., add renderer.surfaceWidth, renderer.surfaceHeight,
currentXServerView.width/height) or suspend/collect until those sizes are >0
(using snapshotFlow or a small retry loop) before calling
resolveBackdropImageModels/loadBackdropBitmap and
renderer.setBackdropBitmap/clearBackdrop so decoding only happens once a valid
targetWidth/targetHeight are available.
🪄 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: 4a2a5deb-7817-416c-94ca-f4ab5123891d

📥 Commits

Reviewing files that changed from the base of the PR and between 719f632 and 41f7886.

📒 Files selected for processing (1)
  • app/src/main/java/app/gamenative/ui/screen/xserver/XServerScreen.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 6 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/com/winlator/container/ContainerData.kt">

<violation number="1" location="app/src/main/java/com/winlator/container/ContainerData.kt:223">
P2: Restoring missing `enableBackdropImage` as false breaks older saved configs that already have a backdrop URI, causing backdrops to stop rendering after upgrade until the user manually re-enables them.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

unpackFiles = (savedMap["unpackFiles"] as? Boolean) ?: false,
suspendPolicy = (savedMap["suspendPolicy"] as? String) ?: Container.SUSPEND_POLICY_MANUAL,
portraitMode = (savedMap["portraitMode"] as? Boolean) ?: false,
enableBackdropImage = (savedMap["enableBackdropImage"] as? Boolean) ?: false,
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Apr 13, 2026

Choose a reason for hiding this comment

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

P2: Restoring missing enableBackdropImage as false breaks older saved configs that already have a backdrop URI, causing backdrops to stop rendering after upgrade until the user manually re-enables them.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/src/main/java/com/winlator/container/ContainerData.kt, line 223:

<comment>Restoring missing `enableBackdropImage` as false breaks older saved configs that already have a backdrop URI, causing backdrops to stop rendering after upgrade until the user manually re-enables them.</comment>

<file context>
@@ -218,6 +220,7 @@ data class ContainerData(
                     unpackFiles = (savedMap["unpackFiles"] as? Boolean) ?: false,
                     suspendPolicy = (savedMap["suspendPolicy"] as? String) ?: Container.SUSPEND_POLICY_MANUAL,
                     portraitMode = (savedMap["portraitMode"] as? Boolean) ?: false,
+                    enableBackdropImage = (savedMap["enableBackdropImage"] as? Boolean) ?: false,
                     backdropImageUri = (savedMap["backdropImageUri"] as? String) ?: "",
                     sharpnessEffect = (savedMap["sharpnessEffect"] as? String) ?: "None",
</file context>
Suggested change
enableBackdropImage = (savedMap["enableBackdropImage"] as? Boolean) ?: false,
enableBackdropImage = (savedMap["enableBackdropImage"] as? Boolean)
?: ((savedMap["backdropImageUri"] as? String)?.isNotBlank() == true),
Fix with Cubic

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