Skip to content

Commit 85f9ba0

Browse files
committed
fix: address PR utkarshdalal#1759 review findings (critical, expert, CPU, GPU, KISS)
- Revert gradle.properties to fork version (PR silently downgraded jvmargs 8g->4g, dropped UTF-8/HeapDump flags, added dead config) - Extract resolution helpers to ui/util/ResolutionUtils.kt; test moved to matching package and extended to 8 tests incl. adaptive generation - Fix contains() substring collision in screenSize index resolution: custom '200x540' matched preset '1200x540 (20:9)' - now exact token - Coerce restored screenSizeIndex against rebuilt adaptive list (OOB risk) - Validate custom resolution post-rounding (negative raw could save 0) - deviceNativeResolution uses WindowMetrics on API 30+ (multi-window safe) - Hoist getGPUCards/preset managers into remember{} (config-change jank) - Replace startsWith dedupe with parsed-resolution comparison; dedupe among adaptive scales; guard zero dimensions; aspect map for special ratios; delete dead aspect_ratio string resources
1 parent 941910a commit 85f9ba0

8 files changed

Lines changed: 205 additions & 139 deletions

File tree

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,3 +73,6 @@ app/src/main/cpp/steambootstrap/steam_bootstrap.c
7373

7474
# Vendored SDL2 headers (fetched for evshim build, not pushed)
7575
app/src/main/cpp/third_party/SDL2/
76+
77+
# OpenCode session artifacts
78+
.omo/

app/src/main/java/app/gamenative/ui/component/dialog/ContainerConfigDialog.kt

Lines changed: 28 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,9 @@ import androidx.compose.ui.tooling.preview.Preview
8383
import app.gamenative.BuildConfig
8484
import app.gamenative.R
8585
import app.gamenative.ui.util.SnackbarManager
86+
import app.gamenative.ui.util.deviceNativeResolution
87+
import app.gamenative.ui.util.evenRound
88+
import app.gamenative.ui.util.generateAdaptiveScreenSizes
8689
import app.gamenative.ui.component.dialog.state.MessageDialogState
8790
import app.gamenative.ui.component.settings.SettingsCPUList
8891
import app.gamenative.ui.component.settings.SettingsCenteredLabel
@@ -130,7 +133,6 @@ import kotlinx.coroutines.launch
130133
import kotlinx.coroutines.withContext
131134
import java.io.File
132135
import java.util.Locale
133-
import kotlin.math.roundToInt
134136

135137
/**
136138
* Gets the component title for Win Components settings group.
@@ -152,42 +154,6 @@ internal fun winComponentsItemTitleRes(string: String): Int {
152154
}
153155
}
154156

155-
/**
156-
* Rounds a float value to the nearest even integer.
157-
* This is required by many mobile GPU drivers to avoid rendering artifacts or crashes
158-
* when using resolutions that are not divisible by 2.
159-
*
160-
* @param value The float value to be rounded.
161-
* @return The nearest even integer to the input value.
162-
*/
163-
internal fun evenRound(value: Float): Int = (value / 2.0f).roundToInt() * 2
164-
165-
/**
166-
* Calculates the greatest common divisor (GCD) of two integers using the Euclidean algorithm.
167-
*
168-
* @param a The first integer.
169-
* @param b The second integer.
170-
* @return The greatest common divisor of a and b.
171-
*/
172-
internal fun gcd(a: Int, b: Int): Int = if (b == 0) a else gcd(b, a % b)
173-
174-
/**
175-
* Calculates and formats the aspect ratio for a given resolution.
176-
* Handles special cases for common mobile aspect ratios like 19.5:9 and 21.5:9.
177-
*
178-
* @param width The width of the resolution in pixels.
179-
* @param height The height of the resolution in pixels.
180-
* @return A string representation of the aspect ratio (e.g., "16:9" or "19.5:9").
181-
*/
182-
internal fun calculateAspectRatio(width: Int, height: Int): String {
183-
val common = gcd(width, height)
184-
val w = width / common
185-
val h = height / common
186-
if ((w == 13) && (h == 6)) return "19.5:9"
187-
if ((w == 43) && (h == 18)) return "21.5:9"
188-
return "$w:$h"
189-
}
190-
191157
private data class ContainerConfigDialogStaticData(
192158
val screenSizes: List<String>,
193159
val baseGraphicsDrivers: List<String>,
@@ -252,38 +218,22 @@ private fun rememberContainerConfigDialogStaticData(): ContainerConfigDialogStat
252218
}
253219
}
254220

255-
val displayMetrics = context.resources.displayMetrics
256-
val screenWidth = maxOf(displayMetrics.widthPixels, displayMetrics.heightPixels).toFloat()
257-
val screenHeight = minOf(displayMetrics.widthPixels, displayMetrics.heightPixels).toFloat()
258-
259-
val nativeWidth = evenRound(screenWidth)
260-
val nativeHeight = evenRound(screenHeight)
261-
val nativeRes = "${nativeWidth}x$nativeHeight"
262-
val nativeRatio = calculateAspectRatio(nativeWidth, nativeHeight)
263-
264-
val optimizedWidth = evenRound(screenWidth * 0.75f)
265-
val optimizedHeight = evenRound(screenHeight * 0.75f)
266-
val optimizedRes = "${optimizedWidth}x$optimizedHeight"
267-
val optimizedRatio = calculateAspectRatio(optimizedWidth, optimizedHeight)
268-
269-
val halfWidth = evenRound(screenWidth * 0.5f)
270-
val halfHeight = evenRound(screenHeight * 0.5f)
271-
val halfRes = "${halfWidth}x$halfHeight"
272-
val halfRatio = calculateAspectRatio(halfWidth, halfHeight)
221+
// Heavy non-composable loads are remembered so a configuration change (which
222+
// re-runs this whole function via LocalConfiguration) does not repeat them.
223+
val gpuCards = remember { ContainerUtils.getGPUCards(context) }
224+
val box64Presets = remember { Box86_64PresetManager.getPresets("box64", context) }
225+
val fexcorePresets = remember { FEXCorePresetManager.getPresets(context) }
273226

274227
val baseScreenSizes = stringArrayResource(R.array.screen_size_entries).toList()
275-
val adaptiveScreenSizes = mutableListOf<String>()
276-
277-
// Add device specific resolutions if they don't exactly match existing presets
278-
listOf(
279-
Triple(nativeRes, nativeRatio, context.getString(R.string.resolution_native)),
280-
Triple(optimizedRes, optimizedRatio, context.getString(R.string.resolution_optimized)),
281-
Triple(halfRes, halfRatio, context.getString(R.string.resolution_half)),
282-
).forEach { (res, ratio, label) ->
283-
if (baseScreenSizes.none { it.startsWith(res) }) {
284-
adaptiveScreenSizes.add("$res ($ratio, $label)")
285-
}
286-
}
228+
val (deviceWidth, deviceHeight) = deviceNativeResolution(context)
229+
val adaptiveScreenSizes = generateAdaptiveScreenSizes(
230+
deviceWidth = deviceWidth,
231+
deviceHeight = deviceHeight,
232+
baseScreenSizes = baseScreenSizes,
233+
nativeLabel = stringResource(R.string.resolution_native),
234+
optimizedLabel = stringResource(R.string.resolution_optimized),
235+
halfLabel = stringResource(R.string.resolution_half),
236+
)
287237

288238
return ContainerConfigDialogStaticData(
289239
screenSizes = baseScreenSizes + adaptiveScreenSizes,
@@ -293,7 +243,7 @@ private fun rememberContainerConfigDialogStaticData(): ContainerConfigDialogStat
293243
dxvkVersionsBase = stringArrayResource(R.array.dxvk_version_entries).toList(),
294244
vkd3dVersionsBase = stringArrayResource(R.array.vkd3d_version_entries).toList(),
295245
audioDrivers = stringArrayResource(R.array.audio_driver_entries).toList(),
296-
gpuCards = ContainerUtils.getGPUCards(context),
246+
gpuCards = gpuCards,
297247
presentModes = stringArrayResource(R.array.present_mode_entries).toList(),
298248
rendererPresentModes = listOf("fifo", "mailbox"),
299249
resourceTypes = stringArrayResource(R.array.resource_type_entries).toList(),
@@ -314,9 +264,9 @@ private fun rememberContainerConfigDialogStaticData(): ContainerConfigDialogStat
314264
box64Versions = stringArrayResource(R.array.box64_version_entries).toList(),
315265
wowBox64VersionsBase = stringArrayResource(R.array.wowbox64_version_entries).toList(),
316266
box64BionicVersionsBase = stringArrayResource(R.array.box64_bionic_version_entries).toList(),
317-
box64Presets = Box86_64PresetManager.getPresets("box64", context),
267+
box64Presets = box64Presets,
318268
fexcoreVersionsBase = stringArrayResource(R.array.fexcore_version_entries).toList(),
319-
fexcorePresets = FEXCorePresetManager.getPresets(context),
269+
fexcorePresets = fexcorePresets,
320270
fexcoreTSOPresets = stringArrayResource(R.array.fexcore_preset_entries).toList(),
321271
fexcoreX87Presets = stringArrayResource(R.array.x87mode_preset_entries).toList(),
322272
fexcoreMultiblockValues = stringArrayResource(R.array.multiblock_values).toList(),
@@ -838,19 +788,19 @@ fun ContainerConfigDialog(
838788
}
839789

840790
val screenSizeIndexRef = rememberSaveable {
841-
val searchIndex = screenSizes.indexOfFirst { it.contains(config.screenSize) }
791+
val searchIndex = screenSizes.indexOfFirst { it.substringBefore(' ') == config.screenSize }
842792
mutableIntStateOf(if (searchIndex > 0) searchIndex else 0)
843793
}
844794
var screenSizeIndex by screenSizeIndexRef
845795
val customScreenWidthRef = rememberSaveable {
846-
val searchIndex = screenSizes.indexOfFirst { it.contains(config.screenSize) }
796+
val searchIndex = screenSizes.indexOfFirst { it.substringBefore(' ') == config.screenSize }
847797
mutableStateOf(
848798
if (searchIndex <= 0) config.screenSize.split("x").getOrElse(0) { "1280" } else "1280"
849799
)
850800
}
851801
var customScreenWidth by customScreenWidthRef
852802
val customScreenHeightRef = rememberSaveable {
853-
val searchIndex = screenSizes.indexOfFirst { it.contains(config.screenSize) }
803+
val searchIndex = screenSizes.indexOfFirst { it.substringBefore(' ') == config.screenSize }
854804
mutableStateOf(
855805
if (searchIndex <= 0) config.screenSize.split("x").getOrElse(1) { "720" } else "720"
856806
)
@@ -1112,15 +1062,15 @@ fun ContainerConfigDialog(
11121062

11131063
val applyScreenSizeToConfig: () -> Unit = {
11141064
val screenSize = if (screenSizeIndex == 0) {
1115-
val widthInt = customScreenWidth.toIntOrNull() ?: 0
1116-
val heightInt = customScreenHeight.toIntOrNull() ?: 0
1117-
if (widthInt != 0 && heightInt != 0) {
1118-
"${evenRound(widthInt.toFloat())}x${evenRound(heightInt.toFloat())}"
1065+
val width = evenRound((customScreenWidth.toIntOrNull() ?: 0).toFloat())
1066+
val height = evenRound((customScreenHeight.toIntOrNull() ?: 0).toFloat())
1067+
if (width > 0 && height > 0) {
1068+
"${width}x$height"
11191069
} else {
11201070
config.screenSize
11211071
}
11221072
} else {
1123-
screenSizes[screenSizeIndex].split(" ")[0]
1073+
screenSizes[screenSizeIndex.coerceIn(1, screenSizes.lastIndex)].split(" ")[0]
11241074
}
11251075
config = config.copy(screenSize = screenSize)
11261076
}

app/src/main/java/app/gamenative/ui/component/dialog/GeneralTab.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import androidx.compose.ui.unit.sp
3131
import app.gamenative.R
3232
import app.gamenative.ui.component.NoExtractOutlinedTextField
3333
import app.gamenative.ui.component.settings.SettingsListDropdown
34+
import app.gamenative.ui.util.evenRound
3435
import com.alorma.compose.settings.ui.SettingsSwitch
3536
import app.gamenative.ui.theme.settingsTileColors
3637
import app.gamenative.ui.theme.settingsTileColorsAlt
@@ -307,7 +308,7 @@ fun GeneralTabContent(
307308
SettingsListDropdown(
308309
colors = settingsTileColors(),
309310
title = { Text(text = stringResource(R.string.screen_size)) },
310-
value = state.screenSizeIndex.value,
311+
value = state.screenSizeIndex.value.coerceIn(0, state.screenSizes.lastIndex),
311312
items = state.screenSizes,
312313
onItemSelected = {
313314
state.screenSizeIndex.value = it
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
package app.gamenative.ui.util
2+
3+
import android.content.Context
4+
import android.os.Build
5+
import android.view.WindowManager
6+
import kotlin.math.roundToInt
7+
8+
/**
9+
* Rounds a float value to the nearest even integer.
10+
* This is required by many mobile GPU drivers to avoid rendering artifacts or crashes
11+
* when using resolutions that are not divisible by 2.
12+
*/
13+
internal fun evenRound(value: Float): Int = (value / 2.0f).roundToInt() * 2
14+
15+
internal fun gcd(a: Int, b: Int): Int = if (b == 0) a else gcd(b, a % b)
16+
17+
private val wellKnownAspectRatios = mapOf(
18+
(13 to 6) to "19.5:9",
19+
(43 to 18) to "21.5:9",
20+
)
21+
22+
internal fun calculateAspectRatio(width: Int, height: Int): String {
23+
if (width <= 0 || height <= 0) return "$width:$height"
24+
val common = gcd(width, height)
25+
val reduced = (width / common) to (height / common)
26+
return wellKnownAspectRatios[reduced] ?: "${reduced.first}:${reduced.second}"
27+
}
28+
29+
internal fun parseResolution(entry: String): Pair<Int, Int>? {
30+
val dims = entry.substringBefore(' ').split('x')
31+
val width = dims.getOrNull(0)?.toIntOrNull() ?: return null
32+
val height = dims.getOrNull(1)?.toIntOrNull() ?: return null
33+
return width to height
34+
}
35+
36+
/**
37+
* Physical panel resolution as (long side, short side). Uses WindowMetrics on API 30+
38+
* so multi-window and foldable states still report the real display size instead of
39+
* the app window bounds that resources.displayMetrics would give.
40+
*/
41+
internal fun deviceNativeResolution(context: Context): Pair<Int, Int> {
42+
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) {
43+
val bounds = context.getSystemService(WindowManager::class.java)?.currentWindowMetrics?.bounds
44+
if (bounds != null && bounds.width() > 0 && bounds.height() > 0) {
45+
return maxOf(bounds.width(), bounds.height()) to minOf(bounds.width(), bounds.height())
46+
}
47+
}
48+
val metrics = context.resources.displayMetrics
49+
return maxOf(metrics.widthPixels, metrics.heightPixels) to minOf(metrics.widthPixels, metrics.heightPixels)
50+
}
51+
52+
/**
53+
* Builds the device-specific screen-size entries (native, 75%, 50%) that are not
54+
* already covered by [baseScreenSizes]. Entries are formatted as "WxH (ratio, label)".
55+
*/
56+
internal fun generateAdaptiveScreenSizes(
57+
deviceWidth: Int,
58+
deviceHeight: Int,
59+
baseScreenSizes: List<String>,
60+
nativeLabel: String,
61+
optimizedLabel: String,
62+
halfLabel: String,
63+
): List<String> {
64+
val baseResolutions = baseScreenSizes.mapNotNull { parseResolution(it) }
65+
val seen = mutableSetOf<Pair<Int, Int>>()
66+
return listOf(1.0f to nativeLabel, 0.75f to optimizedLabel, 0.5f to halfLabel)
67+
.mapNotNull { (scale, label) ->
68+
val resolution = evenRound(deviceWidth * scale) to evenRound(deviceHeight * scale)
69+
if (resolution.first <= 0 || resolution.second <= 0) return@mapNotNull null
70+
if (resolution in baseResolutions || !seen.add(resolution)) return@mapNotNull null
71+
"${resolution.first}x${resolution.second} (${calculateAspectRatio(resolution.first, resolution.second)}, $label)"
72+
}
73+
}

app/src/main/res/values/strings.xml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2163,8 +2163,6 @@
21632163
<string name="local_mod_copied_progress">Copied %1$s of %2$s</string>
21642164
<string name="nexus_invalid_source_metadata">This managed mod has incomplete source information and cannot be retried.</string>
21652165
<string name="nexus_archive_memory_error">Archive needs more memory than Android allows. Retry after updating GameNative or choose a smaller file.</string>
2166-
<string name="aspect_ratio_19_5_9">19.5:9</string>
2167-
<string name="aspect_ratio_21_5_9">21.5:9</string>
21682166
<string name="language_schinese">Simplified Chinese</string>
21692167
<string name="language_tchinese">Traditional Chinese</string>
21702168
<string name="language_koreana">Korean</string>

app/src/test/java/app/gamenative/ui/component/dialog/ResolutionUtilsTest.kt

Lines changed: 0 additions & 51 deletions
This file was deleted.

0 commit comments

Comments
 (0)