fix: Meta Quest 3/3s crashing on Turnip driver#1185
Open
lvonasek wants to merge 2 commits intoutkarshdalal:masterfrom
Open
fix: Meta Quest 3/3s crashing on Turnip driver#1185lvonasek wants to merge 2 commits intoutkarshdalal:masterfrom
lvonasek wants to merge 2 commits intoutkarshdalal:masterfrom
Conversation
Contributor
📝 WalkthroughWalkthroughAdds a new GPU blacklist class and uses it to exclude Turnip driver data: GPUBlackist determines device-specific Turnip blacklisting; GPUInformation consults it when reporting renderer and capability; BestConfigService omits Turnip-related config fields when blacklisted. Changes
Sequence Diagram(s)sequenceDiagram
participant BestConfig as BestConfigService
participant GPUInfo as GPUInformation
participant Blacklist as GPUBlackist
participant Result as ResultMap
BestConfig->>GPUInfo: request GPU renderer/info
GPUInfo->>Blacklist: isTurnipBlacklisted()
Blacklist-->>GPUInfo: true/false
GPUInfo-->>BestConfig: renderer, driverVersion, driverConfig
BestConfig->>Blacklist: isTurnipBlacklisted() (if needed)
alt not blacklisted and values don't contain "turnip"
BestConfig->>Result: include graphicsDriverVersion/config
else blacklisted or contains "turnip"
BestConfig->>Result: omit those fields
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![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/com/winlator/core/GPUInformation.java (1)
164-179: NormalizeBuild.PRODUCTbefore the switch for safer allowlist matching.At Lines 167-175, matching is case-sensitive. Normalizing once avoids accidental false blacklisting if product casing varies.
♻️ Proposed hardening
public static boolean isTurnipBlacklisted() { if ((Build.MANUFACTURER.compareToIgnoreCase("OCULUS") == 0) || (Build.MANUFACTURER.compareToIgnoreCase("META") == 0)) { - return switch (Build.PRODUCT) { + String product = Objects.toString(Build.PRODUCT, "").toLowerCase(Locale.ENGLISH); + return switch (product) { //Quest 1 case "monterey", "vr_monterey" -> false; //Quest 2, Quest Pro case "hollywood", "seacliff" -> false; //Quest 3, Quest 3s case "eureka", "stinson", "panther" -> true; //future devices default -> true; }; } return false; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/winlator/core/GPUInformation.java` around lines 164 - 179, The switch in isTurnipBlacklisted currently matches Build.PRODUCT case-sensitively; normalize Build.PRODUCT once (e.g., toLowerCase or toUpperCase) before the switch and use the normalized value in the cases so comparisons like "monterey", "vr_monterey", "hollywood", "seacliff", "eureka", "stinson", "panther" are matched reliably; update the switch expression to use the normalizedProduct variable and keep the same case literals normalized to the same form.
🤖 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/utils/BestConfigService.kt`:
- Around line 733-742: The contains("turnip") checks on filteredJson values are
case-sensitive and can be bypassed by mixed-case payloads; update the checks in
BestConfigService (the branches handling "graphicsDriverVersion" and
"graphicsDriverConfig" from filteredJson) to perform case-insensitive matching
(e.g., use a case-insensitive contains or normalize the string) before
consulting GPUInformation.isTurnipBlacklisted(), then only set
resultMap["graphicsDriverVersion"] and resultMap["graphicsDriverConfig"] when
the case-insensitive check allows it.
---
Nitpick comments:
In `@app/src/main/java/com/winlator/core/GPUInformation.java`:
- Around line 164-179: The switch in isTurnipBlacklisted currently matches
Build.PRODUCT case-sensitively; normalize Build.PRODUCT once (e.g., toLowerCase
or toUpperCase) before the switch and use the normalized value in the cases so
comparisons like "monterey", "vr_monterey", "hollywood", "seacliff", "eureka",
"stinson", "panther" are matched reliably; update the switch expression to use
the normalizedProduct variable and keep the same case literals normalized to the
same form.
🪄 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: 4544a771-87a4-4efd-ad2e-e0ae4a2a59c4
📒 Files selected for processing (2)
app/src/main/java/app/gamenative/utils/BestConfigService.ktapp/src/main/java/com/winlator/core/GPUInformation.java
lvonasek
force-pushed
the
fix-quest3-turnip
branch
from
b4dc6e2 to
9327ae2
Compare
lvonasek
force-pushed
the
fix-quest3-turnip
branch
from
9327ae2 to
bd0f598
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/main/java/com/winlator/core/GPUBlackist.java (1)
5-5: Rename and lock down this utility class for maintainability.
GPUBlackistappears misspelled and is instantiable. Recommend renaming toGPUBlacklistand adding a private constructor (optionallyfinalclass).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/winlator/core/GPUBlackist.java` at line 5, Rename the misspelled utility class GPUBlackist to GPUBlacklist and make it non-instantiable by adding a private constructor; also consider declaring the class final to prevent subclassing. Update the class declaration (change GPUBlackist to GPUBlacklist) and add a private GPUBlacklist() { /* prevent instantiation */ } constructor, and ensure all references/usages of GPUBlackist are updated to the new GPUBlacklist identifier.
🤖 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/core/GPUBlackist.java`:
- Around line 8-10: The manufacturer check in GPUBlackist currently uses exact
equalsIgnoreCase on Build.MANUFACTURER ("OCULUS"/"META") which misses branded
variants; change the condition to a null-safe, case-insensitive substring or
prefix match (e.g., use toUpperCase/Locale.ROOT and contains or startsWith) for
Build.MANUFACTURER so any variant like "Meta_X" or "OculusVR" still triggers the
blacklist switch on Build.PRODUCT; update the condition that surrounds the
switch (the if that references Build.MANUFACTURER) to use this hardened match.
---
Nitpick comments:
In `@app/src/main/java/com/winlator/core/GPUBlackist.java`:
- Line 5: Rename the misspelled utility class GPUBlackist to GPUBlacklist and
make it non-instantiable by adding a private constructor; also consider
declaring the class final to prevent subclassing. Update the class declaration
(change GPUBlackist to GPUBlacklist) and add a private GPUBlacklist() { /*
prevent instantiation */ } constructor, and ensure all references/usages of
GPUBlackist are updated to the new GPUBlacklist identifier.
🪄 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: 915bd2a0-ab25-441c-a8e7-787c5dbae445
📒 Files selected for processing (3)
app/src/main/java/app/gamenative/utils/BestConfigService.ktapp/src/main/java/com/winlator/core/GPUBlackist.javaapp/src/main/java/com/winlator/core/GPUInformation.java
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/main/java/app/gamenative/utils/BestConfigService.kt
- app/src/main/java/com/winlator/core/GPUInformation.java
Owner
|
please attach before and after video of this fix |
Contributor
Author
Videos attached |
Owner
|
makes sense, will review again. Which driver should be used? System? |
Contributor
Author
Yes. System driver works on Quest reliable. |
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
Meta Quest 3/3s has Adreno 740 which is normally Turnip compatible but Meta did there something on the driver level (around VR features like SpaceWarp) which made Turnip incompatible. The users starting games on that device see "GPU compatible" but when they start any 3D game, it just keeps crashing.
This PR blacklists Turnip on Meta Quest 3/3s and newer and report the GPU with manufacturer postfix (considering a company having a breaking change on a driver level usually spreads the change around own device portfolio). That means we generate only one additional GPU "Adreno (TM) 740 - Oculus" which will cover the mentioned devices.
This approach avoids making incorrect compatibility reports and applying Turnip on the blacklisted devices. I tried to write it as generic as possible to be able to cover similar future cases.
Recording
Before the fix: https://github.com/user-attachments/assets/14819fdf-ce00-4d71-8ef0-28a7c7240c7d
With the fix: https://github.com/user-attachments/assets/9101636b-4af8-4bf3-b857-37d681e0a11f
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
Fixes crashes on Meta Quest 3/3s and newer by disabling Turnip and using the system graphics driver. Prevents false “GPU compatible” messages and filters out Turnip configs on blacklisted devices.
GPUBlackistto detect Meta/Oculus devices by product and disable Turnip on Quest 3/3s and future devices; allowed on Quest 1/2/Pro.Build.MANUFACTURERto the GPU renderer (e.g., “Adreno (TM) 740 - Meta/Oculus”) to create a distinct GPU entry.BestConfigService, ignores Turnip ingraphicsDriverVersionandgraphicsDriverConfig; inGPUInformation, gatesisTurnipCapablewith the blacklist.Written for commit bd0f598. Summary will update on new commits.
Summary by CodeRabbit