From 4fd90ad5df8439ec9b45803dd031b6e7d604d01e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Mondaini=20Calv=C3=A3o?= Date: Wed, 30 Sep 2026 21:47:46 -0300 Subject: [PATCH 1/2] feat: make auto (Vulkan on Vulkan 1.1 devices) the default renderer (#30) A providers/android.json without a `renderer` key now means `auto` (was `gles`), so the default manifest carries the optional Vulkan 1.1 uses-feature (D7). The runtime default for an APK without the `labelle.renderer` meta-data is `auto` too (source `auto; default`); an unreadable or too-long meta-data still means gles. Explicit `gles` is the opt-out and produces the old manifest. Resolution order and the crash guard are unchanged: a default `auto` that resolves to Vulkan is marked before init, cleared when stable and falls back to GLES after a crashed start, like an explicit `vulkan`. Bumps the version to 0.5.0. Claude-Session: https://claude.ai/code/session_01LpszrcSrLLijQcxxvgWyUQ --- README.md | 4 +- build.zig.zon | 2 +- docs/emulator-testing.md | 2 +- docs/renderer.md | 35 +++++---- src/crash_guard.zig | 123 ++++++++++++++++++++++++++++--- src/renderer.zig | 145 ++++++++++++++++++++++++++----------- tests/provider/e2e.py | 5 ++ tools/package/manifest.zig | 57 +++++++++++++-- tools/settings.zig | 13 +++- 9 files changed, 305 insertions(+), 81 deletions(-) diff --git a/README.md b/README.md index ff0ab07..c378e4a 100644 --- a/README.md +++ b/README.md @@ -131,7 +131,7 @@ The project's plugin pin therefore decides which labelle-android runs, for bgfx "signing": { "keystore": "keys/release.jks", "store_password": "env:FP_KS_PASS", "key_alias": "labelle-release", "key_password": "env:FP_KEY_PASS" }, "deploy": { "repo": "owner/name", "channel": "stable" }, - "renderer": "gles" + "renderer": "auto" } ``` @@ -148,7 +148,7 @@ The project's plugin pin therefore decides which labelle-android runs, for bgfx | `abis` | no | `["arm64-v8a"]` | exactly `["arm64-v8a"]` in v1 | | `signing` | no | debug keystore | `store_password`/`key_password` must be `env:VAR` or `file:PATH` (apksigner's forms); a `pass:` literal is rejected so no secret is committed | | `deploy` | no | | `repo` is `owner/name`; `channel` is `stable`, `staging`, `preview` or `internal` | -| `renderer` | no | `"gles"` | `gles`, `vulkan` or `auto` (Vulkan when the device reports Vulkan ≥ 1.1, else GLES). Always stamped into the manifest as `` on ``; `vulkan` and `auto` also add the optional `android.hardware.vulkan.version` `0x401000` (Vulkan 1.1) feature. GLES 3.0 stays a required feature as the fallback ([labelle-bgfx#172](https://github.com/labelle-toolkit/labelle-bgfx/issues/172) D2/D7). See [Choosing a renderer](docs/renderer.md) | +| `renderer` | no | `"auto"` (since 0.5.0; was `"gles"`) | `gles`, `vulkan` or `auto` (Vulkan when the device reports Vulkan ≥ 1.1, else GLES). `"gles"` opts out of Vulkan. Always stamped into the manifest as `` on ``; `vulkan` and `auto` also add the optional `android.hardware.vulkan.version` `0x401000` (Vulkan 1.1) feature. GLES 3.0 stays a required feature as the fallback ([labelle-bgfx#172](https://github.com/labelle-toolkit/labelle-bgfx/issues/172) D2/D7). See [Choosing a renderer](docs/renderer.md) | The parse is strict: unknown keys, duplicate keys and wrong types are errors, and the file is validated before the provider does anything. `immersive_mode` and `load_assets_from_apk` are rejected here: the assembler reads them at generate time, so they stay in `project.labelle .android`. There is no `studio` block: `labelle android studio` is not part of this release. `version_name` names the bundle and stamps `android:versionName`; `versionCode` is `labelle bundle --build-number` (1 for `labelle build`). diff --git a/build.zig.zon b/build.zig.zon index 2281055..f7a730f 100644 --- a/build.zig.zon +++ b/build.zig.zon @@ -1,7 +1,7 @@ .{ .fingerprint = 0x1ba3596142e04ab0, .name = .labelle_android, - .version = "0.4.0", + .version = "0.5.0", .minimum_zig_version = "0.16.0", .dependencies = .{}, .paths = .{ diff --git a/docs/emulator-testing.md b/docs/emulator-testing.md index 5cef37f..b364c23 100644 --- a/docs/emulator-testing.md +++ b/docs/emulator-testing.md @@ -77,7 +77,7 @@ On an M1 Max host the device is `Apple M1 Max` (gfxstream over MoltenVK) and the On a macOS host, the emulator's GLES runs on the host's desktop OpenGL 4.1, which has no ASTC. An ASTC-only Android build can't load its atlases there under GLES. -Under Vulkan, ASTC works natively: bgfx reports every ASTC format as `TEXTURE_2D`, and the atlases sample correctly. **Use the Vulkan renderer for ASTC builds on the emulator.** Set `"renderer": "vulkan"` in a local `providers/android.json`, or use the launch override below. +Under Vulkan, ASTC works natively: bgfx reports every ASTC format as `TEXTURE_2D`, and the atlases sample correctly. **Use the Vulkan renderer for ASTC builds on the emulator.** From labelle-android 0.5.0 the default `auto` setting already picks Vulkan here (the guest reports Vulkan 1.3); with an explicit `"renderer": "gles"`, or on an older release, set `"renderer": "vulkan"` in a local `providers/android.json`, or use the launch override below. ## 5. Launch with a renderer override diff --git a/docs/renderer.md b/docs/renderer.md index 4a3f6d0..cb07ceb 100644 --- a/docs/renderer.md +++ b/docs/renderer.md @@ -2,7 +2,7 @@ On Android, labelle-bgfx can render with **GLES** or **Vulkan**. labelle-android decides which one at launch and passes the choice to bgfx through the `LABELLE_BGFX_RENDERER` environment variable ([labelle-bgfx#172](https://github.com/labelle-toolkit/labelle-bgfx/issues/172), D1–D4 and D11). -This page describes labelle-android ≥ 0.4.0. Earlier releases don't read the setting at launch; at most they stamp it into the manifest. +This page describes labelle-android ≥ 0.5.0, where the default is `auto`. In 0.4.0 the default was `gles`. Earlier releases don't read the setting at launch; at most they stamp it into the manifest. It also needs a labelle-bgfx that reads `LABELLE_BGFX_RENDERER` on every platform ([labelle-bgfx#176](https://github.com/labelle-toolkit/labelle-bgfx/issues/176)). The crash guard's frame-based stable rule needs `labelle_bgfx_frames_presented` ([labelle-bgfx#182](https://github.com/labelle-toolkit/labelle-bgfx/issues/182)); without it the guard uses a time-only rule. @@ -14,24 +14,26 @@ Set it in `providers/android.json`: { "schema_version": 1, "package_name": "com.example.game", - "renderer": "vulkan" + "renderer": "gles" } ``` | Value | Meaning | |---|---| -| `"gles"` (default) | GLES 3.0. | +| `"auto"` (default) | Vulkan when the device reports Vulkan ≥ 1.1 (the `android.hardware.vulkan.version` system feature ≥ `0x401000`), else GLES. | +| `"gles"` | GLES 3.0. The opt-out from Vulkan. | | `"vulkan"` | Vulkan. bgfx falls back to GLES if Vulkan init fails. | -| `"auto"` | Vulkan when the device reports Vulkan ≥ 1.1 (the `android.hardware.vulkan.version` system feature ≥ `0x401000`), else GLES. | + +Leaving the key out means `auto`. To stay on GLES, set `"renderer": "gles"`, as in the example above. Any other value, or a non-string value, fails the build with `renderer must be one of "gles", "vulkan", "auto"`. The packager stamps the setting into `AndroidManifest.xml`: - **Always**, `gles` included: `` as the first child of ``. This is what the runtime reads. -- **For `vulkan` and `auto` only:** ``. It's optional, so Play still offers the app to devices without Vulkan. The required GLES 3.0 feature stays, as the fallback. +- **For `vulkan` and `auto` (so also with no `renderer` key):** ``. It's optional, so Play still offers the app to devices without Vulkan. The required GLES 3.0 feature stays, as the fallback. -With `gles`, the manifest is otherwise unchanged. To check a packaged APK (from the generated target directory, e.g. `.labelle/bgfx_android/`): +With an explicit `gles`, the manifest is otherwise unchanged from the pre-Vulkan one, with no Vulkan feature. To check a packaged APK (from the generated target directory, e.g. `.labelle/bgfx_android/`): ```sh SDK="${ANDROID_HOME:-${ANDROID_SDK_ROOT:-$HOME/Library/Android/sdk}}" @@ -51,8 +53,8 @@ This uses the newest installed build-tools revision **that actually contains `aa |---|---|---| | 1 | `intent` | The `LABELLE_BGFX_RENDERER` launch extra, `vulkan` or `gles` (exact, lower-case). **Debuggable builds only.** In a non-debuggable build it's ignored with an info line. Any other value is warned about and ignored. An empty extra counts as absent. | | 2 | `crash-guard` | A previous Vulkan start didn't complete, so start on `gles` (see [Crash guard](#crash-guard)). | -| 3 | `setting` | The `labelle.renderer` meta-data: `gles` or `vulkan`. Missing or unreadable → `gles`. An invalid value → `gles`, with a warning. | -| 4 | `auto` | The setting is `auto`: `vulkan` if the device has the Vulkan 1.1 feature, else `gles`. If the query fails, it counts as no Vulkan. | +| 3 | `setting` | The `labelle.renderer` meta-data: `gles` or `vulkan`. Missing → the default, `auto` (rule 4). Unreadable or too long → `gles`. An invalid value → `gles`, with a warning. | +| 4 | `auto` | The setting is `auto`, or the meta-data is missing (the default; logged with `; default`): `vulkan` if the device has the Vulkan 1.1 feature, else `gles`. If the query fails, it counts as no Vulkan. | The result is exported with `setenv("LABELLE_BGFX_RENDERER", "vulkan"|"gles")`. It always replaces any value already in the process environment. @@ -88,14 +90,15 @@ adb logcat -d -s labelle | grep -E 'renderer: (gles|vulkan)|bgfx: renderer|crash | Line | Meaning | |---|---| -| `renderer: vulkan (source: setting)` | labelle-android's choice and the rule that made it: `intent`, `crash-guard`, `setting` or `auto`. Logged once per launch. | +| `renderer: vulkan (source: auto)` | labelle-android's choice and the rule that made it: `intent`, `crash-guard`, `setting` or `auto`. Logged once per launch. A default project's APK stamps `auto`, so this is the usual line on a Vulkan 1.1 device. | +| `renderer: vulkan (source: auto; default)` | The APK has no `labelle.renderer` meta-data (packaged before the setting existed), so the default `auto` applied. | | `renderer: gles (source: crash-guard; …)` | The crash guard decided; see [Crash guard log lines](#crash-guard-log-lines). | | `android: LABELLE_BGFX_RENDERER=vulkan (launch intent extra)` | The override was applied (debuggable build). | | `android: LABELLE_BGFX_RENDERER cleared (set by a previous launch's intent)` / `… restored to (set by a previous launch's intent)` | A launch without the extra undid the previous launch's override. | | `android: ignoring intent extra LABELLE_BGFX_RENDERER: the apk is not debuggable` | The override was sent to a release build. | | `android: ignoring intent extra LABELLE_BGFX_RENDERER='' (expected 'vulkan' or 'gles')` | Bad override value. The next rule decides. | | `android: invalid labelle.renderer meta-data '' (expected 'gles', 'vulkan' or 'auto'); using gles` | The APK carries a bad setting. The strict setting parser should make this impossible. | -| `android: no labelle.renderer meta-data; using gles` | The APK has no setting (built before the setting existed). | +| `android: no labelle.renderer meta-data; using the default (auto)` | The APK has no setting (built before the setting existed). | | `android: labelle.renderer meta-data too long; using gles` | The value doesn't fit the reader's buffer. | | `android: could not read the labelle.renderer meta-data; using gles (crash-guard marks kept)` | The setting couldn't be read, and the launch uses GLES. The unreadable setting can't itself reset a marker; a marker is still reset if the `versionCode` was read and changed. | | `android: could not set LABELLE_BGFX_RENDERER=` | `setenv` failed; bgfx uses its platform default. | @@ -110,11 +113,13 @@ If the game crashes, hangs or is killed while starting on Vulkan, the next launc ### Markers -Two marker files, each holding ` `. The setting is the effective `renderer` setting; an absent or invalid one counts as `gles`. +Two marker files, each holding ` `. The setting is the effective `renderer` setting; an absent one counts as `auto` (the default), an invalid one as `gles`. + +The guard covers every Vulkan start, whichever rule chose it: an explicit `vulkan`, an explicit `auto`, the default `auto`, or the intent override. A default `auto` that resolves to Vulkan is marked before init, cleared once stable, and sends the next launch to GLES after a crashed start, exactly like an explicit `vulkan`. | File | Written | Removed | |---|---|---| -| `.labelle_vulkan_start` | Whenever the resolved renderer is `vulkan` (intent override included), before `setenv`. | When this Vulkan start becomes **stable** (below); or at the next launch in a new process, which then writes `.labelle_vulkan_disabled`. | +| `.labelle_vulkan_start` | Whenever the resolved renderer is `vulkan` (default `auto` and intent override included), before `setenv`. | When this Vulkan start becomes **stable** (below); or at the next launch in a new process, which then writes `.labelle_vulkan_disabled`. | | `.labelle_vulkan_disabled` | At launch, when a `.labelle_vulkan_start` left by a process that's gone has a matching stamp. | When its stamp is proven to have changed (see Reset). | **Where they live.** The directory is chosen once per process: @@ -180,8 +185,10 @@ These are logged under the game's `labelle` tag: The JNI helpers log under the `labelle-android` tag. When a JNI local frame can't be pushed, they log `renderer meta-data: PushLocalFrame failed; exception cleared`, or the same line with `system feature:`, `no-backup dir:` or `version code:` in front, and the query counts as failed. -## Vulkan will become the default +## Vulkan is the default + +Since labelle-android 0.5.0 the default is `"auto"` ([labelle-android#30](https://github.com/labelle-toolkit/labelle-android/issues/30), [labelle-bgfx#172](https://github.com/labelle-toolkit/labelle-bgfx/issues/172) D12). This replaces the earlier plan to switch to `"vulkan"` only after the production gate: the owner chose `auto` without waiting for the gate. -The default is `"gles"` for now. Once Vulkan passes the production gate in [labelle-bgfx#172](https://github.com/labelle-toolkit/labelle-bgfx/issues/172) (D8), a minor labelle-android release changes the default to `"vulkan"` (D12). Devices without working Vulkan still start: bgfx's fallback covers a failed init, and the crash guard covers a crashed one. A project that needs GLES should set `"renderer": "gles"` explicitly. +Devices without Vulkan 1.1 use GLES (the device check). Devices where Vulkan fails still start: bgfx's fallback covers a failed init, and the crash guard covers a crashed or hung one. To stay on GLES, set `"renderer": "gles"`. For testing Vulkan on the emulator, see [Android emulator testing](emulator-testing.md). diff --git a/src/crash_guard.zig b/src/crash_guard.zig index 893835f..1d6e545 100644 --- a/src/crash_guard.zig +++ b/src/crash_guard.zig @@ -14,7 +14,9 @@ //! and a marker found in either is effective (`checkDirs`). //! //! * `.labelle_vulkan_start`: written by `beginVulkanStart`, just before a -//! Vulkan start (the resolved renderer is `vulkan`). +//! Vulkan start (the resolved renderer is `vulkan`, whichever rule chose +//! it: an explicit `vulkan`, an explicit `auto`, the DEFAULT `auto` of a +//! missing meta-data, or the intent). //! * `.labelle_vulkan_disabled`: written when a launch finds a //! `.labelle_vulkan_start` left by a process that is gone, i.e. a Vulkan //! start that never reached "stable". @@ -125,7 +127,8 @@ const stamp_cap = 64; /// READ FAILURE (JNI error), not a value: it can never count as a change, /// so it never resets the guard (Codex review of labelle-android#33, /// labelle-bgfx#172 comment 5904915798). An ABSENT `labelle.renderer` key is -/// a successful read and arrives here as the default `gles`. +/// a successful read and arrives here as the default `auto` +/// (labelle-android#30). pub const Observed = struct { version_code: ?i64, setting: ?[]const u8, @@ -1106,8 +1109,8 @@ test "an intent override beats the guard (renderer.decide asks the intent first) self.guard_calls += 1; return checkAtLaunch(self.fs, v1_vulkan); } - pub fn metaData(_: *@This()) ?[]const u8 { - return "vulkan"; + pub fn metaData(_: *@This()) renderer.MetaRead { + return .{ .value = "vulkan" }; } pub fn hasVulkan(_: *@This()) bool { return true; @@ -1163,6 +1166,8 @@ const LaunchQuery = struct { fs: *FakeFs, meta: renderer.MetaRead, version: ?i64, + /// The device's Vulkan 1.1 feature (`auto`'s check). + vulkan: bool = true, pub fn intentExtra(_: *@This()) ?[]const u8 { return null; @@ -1173,17 +1178,36 @@ const LaunchQuery = struct { pub fn vulkanDisabled(self: *@This()) bool { return checkNew(self.fs, .{ .version_code = self.version, .setting = self.meta.guardSetting() }); } - pub fn metaData(self: *@This()) ?[]const u8 { - return self.meta.metaData(); + pub fn metaData(self: *@This()) renderer.MetaRead { + return self.meta; } - pub fn hasVulkan(_: *@This()) bool { - return true; + pub fn hasVulkan(self: *@This()) bool { + return self.vulkan; } fn run(fs: *FakeFs, meta: renderer.MetaRead, version: ?i64) renderer.Decision { var q: LaunchQuery = .{ .fs = fs, .meta = meta, .version = version }; return renderer.decide(&q); } + + /// A whole launch in a NEW process, the way `renderer.resolve` wires it: + /// decide (guard first), then for `vulkan` the start mark + stable + /// thread (`beginStart`, keyed on the same `guardSetting`) and + /// `afterStart`. The stable thread runs on `clock` at 60 fps; a + /// `clock.dies_at_ns` before stable is a crash. + fn launch(fs: *FakeFs, clock: *FakeClock, meta: renderer.MetaRead, version: i64, vulkan: bool) renderer.Decision { + var q: LaunchQuery = .{ .fs = fs, .meta = meta, .version = version, .vulkan = vulkan }; + var d = renderer.decide(&q); + if (d.renderer == .vulkan) { + var state: StartState = .{}; + var got: ?u32 = null; + const stamp: Stamp = .{ .version_code = version, .setting = meta.guardSetting().? }; + d = renderer.afterStart(d, beginStart(fs, stamp, &state, NoFrames{}, RecordingSpawner{ .got = &got })); + var frames: FakeFrames = .{ .clock = clock }; + if (got) |mine| settle(fs, clock, &frames, &state, mine, null); + } + return d; + } }; test "a failed meta-data read preserves the marks; the next good read is still guarded" { @@ -1255,17 +1279,94 @@ test "an observed change still resets even when the OTHER half is unreadable" { try testing.expect(fs.disabled == null); } -test "an absent key is a successful read of the default gles: a real change vs a vulkan stamp" { +test "an absent key is a successful read of the default auto: a real change vs a vulkan stamp" { const R = LaunchQuery.renderer; var fs: FakeFs = .{}; fs.set(disabled_file, "1 vulkan"); fs.set(start_file, "1 vulkan"); const d = LaunchQuery.run(&fs, .absent, 1); - try testing.expectEqual(R.Renderer.gles, d.renderer); - try testing.expectEqual(R.Source.setting, d.source); // not the guard + try testing.expectEqual(R.Renderer.vulkan, d.renderer); + try testing.expectEqual(R.Source.auto_default, d.source); // not the guard try testing.expect(fs.disabled == null and fs.start == null); // reset } +// ── labelle-android#30: the default `auto` is guarded like an explicit `vulkan` ── + +test "default auto: Vulkan start is marked before init and the mark is stamped 'auto'" { + const R = LaunchQuery.renderer; + var fs: FakeFs = .{}; + // The process dies right after the mark (before any stable poll). + var clock: FakeClock = .{ .dies_at_ns = 1 }; + const d = LaunchQuery.launch(&fs, &clock, .absent, 1, true); + try testing.expectEqual(R.Renderer.vulkan, d.renderer); + try testing.expectEqual(R.Source.auto_default, d.source); + try testing.expectEqualStrings("1 auto", fs.start.?); +} + +test "default auto: a crashed Vulkan start flips the next launch to gles (crash-guard)" { + const R = LaunchQuery.renderer; + var fs: FakeFs = .{}; + var clock: FakeClock = .{ .dies_at_ns = stable_after_ns - 1 }; + _ = LaunchQuery.launch(&fs, &clock, .absent, 1, true); + try testing.expectEqualStrings("1 auto", fs.start.?); // never stable + + var clock2: FakeClock = .{}; + const d = LaunchQuery.launch(&fs, &clock2, .absent, 1, true); + try testing.expectEqual(R.Renderer.gles, d.renderer); + try testing.expectEqual(R.Source.crash_guard, d.source); + try testing.expectEqualStrings("1 auto", fs.disabled.?); + try testing.expect(fs.start == null); + // ...and stays there, exactly as for an explicit `vulkan`. + const again = LaunchQuery.launch(&fs, &clock2, .absent, 1, true); + try testing.expectEqual(R.Source.crash_guard, again.source); + + // The packaged default and the runtime default are the same setting: an + // APK that stamps `auto` explicitly is guarded by the same marker. + const explicit = LaunchQuery.launch(&fs, &clock2, .{ .value = "auto" }, 1, true); + try testing.expectEqual(R.Source.crash_guard, explicit.source); + + // Opting out to gles is a changed setting: the marker is reset. + const opt_out = LaunchQuery.launch(&fs, &clock2, .{ .value = "gles" }, 1, true); + try testing.expectEqual(R.Renderer.gles, opt_out.renderer); + try testing.expectEqual(R.Source.setting, opt_out.source); + try testing.expect(fs.disabled == null and fs.start == null); +} + +test "default auto: a stable Vulkan start clears its mark; the next launch is Vulkan again" { + const R = LaunchQuery.renderer; + var fs: FakeFs = .{}; + var clock: FakeClock = .{}; + const d1 = LaunchQuery.launch(&fs, &clock, .absent, 1, true); + try testing.expectEqual(R.Source.auto_default, d1.source); + try testing.expectEqual(stable_after_ns, clock.now_ns); // cleared at the threshold + try testing.expect(fs.start == null and fs.disabled == null); + + var clock2: FakeClock = .{}; + const d2 = LaunchQuery.launch(&fs, &clock2, .absent, 1, true); + try testing.expectEqual(R.Renderer.vulkan, d2.renderer); + try testing.expectEqual(R.Source.auto_default, d2.source); +} + +test "default auto: an unrecordable start falls back to gles like an explicit vulkan" { + const R = LaunchQuery.renderer; + var fs: FakeFs = .{}; + fs.partial_writes = true; + var clock: FakeClock = .{}; + const d = LaunchQuery.launch(&fs, &clock, .absent, 1, true); + try testing.expectEqual(R.Renderer.gles, d.renderer); + try testing.expectEqual(R.Source.crash_guard_unrecorded, d.source); +} + +test "default auto on a device without Vulkan 1.1: gles, no start mark" { + const R = LaunchQuery.renderer; + var fs: FakeFs = .{}; + var clock: FakeClock = .{}; + const d = LaunchQuery.launch(&fs, &clock, .absent, 1, false); + try testing.expectEqual(R.Renderer.gles, d.renderer); + try testing.expectEqual(R.Source.auto_default, d.source); + try testing.expect(fs.start == null and fs.disabled == null); +} + // ── Codex bot findings on labelle-android#33 (final round) ───────────── test "same-process relaunch before stable supersedes its own mark (no disable)" { diff --git a/src/renderer.zig b/src/renderer.zig index 8fd9836..75c0a7e 100644 --- a/src/renderer.zig +++ b/src/renderer.zig @@ -15,10 +15,13 @@ //! new version or a changed setting tries Vulkan again. //! 3. `setting`: the provider setting, stamped into the manifest as //! `` -//! (labelle-android#26). Missing or unreadable → `gles`; an invalid -//! value → `gles` with a warning. -//! 4. `auto` (the setting's third value): Vulkan when the device reports -//! `android.hardware.vulkan.version` >= 1.1 (0x401000), else `gles`. +//! (labelle-android#26). Missing → the default, `auto` (rule 4, source +//! `auto` with `; default`; labelle-android#30). Unreadable (a JNI +//! error) or too long → `gles`; an invalid value → `gles` with a +//! warning. +//! 4. `auto` (the setting's third value, and the default since 0.5.0): +//! Vulkan when the device reports `android.hardware.vulkan.version` +//! >= 1.1 (0x401000), else `gles`. //! //! Then, when the result is `vulkan`, `crash_guard.beginVulkanStart` (the //! start mark + the stable thread: 120 frames and 10 s). If the start mark @@ -27,7 +30,9 @@ //! `setenv("LABELLE_BGFX_RENDERER", "vulkan"|"gles", 1)` and one log line: //! `renderer: (source: intent|crash-guard|setting|auto)`, with //! `; previous Vulkan start did not complete` (or `; could not record the -//! Vulkan start`) after `crash-guard`. +//! Vulkan start`) after `crash-guard`, and `; default` after an `auto` that +//! came from a missing meta-data. A default `auto` that picks Vulkan goes +//! through the crash guard exactly like an explicit `vulkan` or `auto`. //! //! ## Where it runs //! @@ -73,13 +78,16 @@ pub const Source = enum { crash_guard_unrecorded, setting, auto, + /// `auto` because the meta-data is missing: the default setting + /// (labelle-android#30). Logged as `auto` with `; default`. + auto_default, pub fn label(self: Source) []const u8 { return switch (self) { .intent => "intent", .crash_guard, .crash_guard_unrecorded => "crash-guard", .setting => "setting", - .auto => "auto", + .auto, .auto_default => "auto", }; } @@ -88,6 +96,7 @@ pub const Source = enum { return switch (self) { .crash_guard => "; previous Vulkan start did not complete", .crash_guard_unrecorded => "; could not record the Vulkan start", + .auto_default => "; default", else => "", }; } @@ -111,6 +120,12 @@ pub fn afterStart(d: Decision, recorded: bool) Decision { /// The provider setting's values. pub const Setting = enum { gles, vulkan, auto }; +/// What a missing `labelle.renderer` meta-data means (labelle-android#30): +/// the same default as a `providers/android.json` without a `renderer` key, +/// so an APK packaged without the meta-data behaves like a new default one. +/// `gles` is the explicit opt-out. +pub const default_setting: Setting = .auto; + /// The three (four) outcomes of the `labelle.renderer` meta-data read. The /// JNI helper's return code carries them across the C boundary: /// `>= 0` = value length, `-1` = absent, `-3` = too long, `-2` (or any @@ -118,7 +133,8 @@ pub const Setting = enum { gles, vulkan, auto }; pub const MetaRead = union(enum) { /// Read successfully (may still be an invalid value; `decide` warns). value: []const u8, - /// Read successfully; the key is not there → the default, `gles`. + /// Read successfully; the key is not there → the default + /// (`default_setting`, `auto`). absent, /// Read successfully; the value does not fit the buffer (so it is not a /// valid setting) → `gles`, like any invalid value. @@ -135,31 +151,25 @@ pub const MetaRead = union(enum) { }; } - /// What the setting rule sees: the value, or null (→ `gles`) otherwise. - /// For the RENDERER, a read error still means gles, as before. - pub fn metaData(self: MetaRead) ?[]const u8 { - return switch (self) { - .value => |v| v, - else => null, - }; - } - /// The setting the crash guard compares/records: the effective setting - /// for every SUCCESSFUL read (absent/invalid/too long → `gles`), and null - /// for a read error, which the guard treats as unknown (marks kept). + /// for every SUCCESSFUL read (absent → the default `auto`; invalid/too + /// long → `gles`), and null for a read error, which the guard treats as + /// unknown (marks kept). pub fn guardSetting(self: MetaRead) ?[]const u8 { return switch (self) { .value => |v| settingLabel(v), - .absent, .too_long => "gles", + .absent => settingLabel(null), + .too_long => "gles", .read_error => null, }; } }; /// The effective setting the crash guard's stamp records: the parsed -/// meta-data, and `gles` for missing or invalid (which is what they mean). +/// meta-data, `default_setting` (`auto`) for missing, and `gles` for +/// invalid (which is what they mean). pub fn settingLabel(meta: ?[]const u8) []const u8 { - const raw = meta orelse return "gles"; + const raw = meta orelse return @tagName(default_setting); return @tagName(parseSetting(raw) orelse .gles); } @@ -186,9 +196,9 @@ pub fn parseSetting(value: []const u8) ?Setting { /// * `intentExtra() ?[]const u8` — the launch extra (null = absent) /// * `debuggable() bool` — asked only when the extra is present /// * `vulkanDisabled() bool` — the D11 crash guard -/// * `metaData() ?[]const u8` — the `labelle.renderer` meta-data (null = -/// missing or unreadable) -/// * `hasVulkan() bool` — asked only for `auto` +/// * `metaData() MetaRead` — the `labelle.renderer` meta-data read +/// (absent → `default_setting`; read error or too long → `gles`) +/// * `hasVulkan() bool` — asked only for `auto` (explicit or default) pub fn decide(q: anytype) Decision { if (q.intentExtra()) |v| { // An empty extra counts as absent, as it does in `intent_env`. @@ -205,17 +215,23 @@ pub fn decide(q: anytype) Decision { } } if (q.vulkanDisabled()) return .{ .renderer = .gles, .source = .crash_guard }; - const setting: Setting = if (q.metaData()) |raw| - parseSetting(raw) orelse blk: { + const meta: MetaRead = q.metaData(); + const setting: Setting = switch (meta) { + .value => |raw| parseSetting(raw) orelse blk: { std.log.warn("android: invalid {s} meta-data '{s}' (expected 'gles', 'vulkan' or 'auto'); using gles", .{ meta_data_name, raw }); break :blk .gles; - } - else - .gles; + }, + .absent => default_setting, + // Not a usable setting: stay on the safe renderer. + .too_long, .read_error => .gles, + }; return switch (setting) { .gles => .{ .renderer = .gles, .source = .setting }, .vulkan => .{ .renderer = .vulkan, .source = .setting }, - .auto => .{ .renderer = if (q.hasVulkan()) .vulkan else .gles, .source = .auto }, + .auto => .{ + .renderer = if (q.hasVulkan()) .vulkan else .gles, + .source = if (meta == .absent) .auto_default else .auto, + }, }; } @@ -244,8 +260,8 @@ const JniQuery = struct { pub fn vulkanDisabled(self: *JniQuery) bool { return crash_guard.vulkanDisabled(self.activity, self.metaRead().guardSetting()); } - pub fn metaData(self: *JniQuery) ?[]const u8 { - return self.metaRead().metaData(); + pub fn metaData(self: *JniQuery) MetaRead { + return self.metaRead(); } fn metaRead(self: *JniQuery) MetaRead { if (self.meta) |m| return m; @@ -253,7 +269,7 @@ const JniQuery = struct { const m = MetaRead.fromCode(n, &self.meta_buf); switch (m) { .value => {}, - .absent => std.log.info("android: no {s} meta-data; using gles", .{meta_data_name}), + .absent => std.log.info("android: no {s} meta-data; using the default ({s})", .{ meta_data_name, @tagName(default_setting) }), .too_long => std.log.warn("android: {s} meta-data too long; using gles", .{meta_data_name}), .read_error => std.log.warn("android: could not read the {s} meta-data; using gles (crash-guard marks kept)", .{meta_data_name}), } @@ -295,7 +311,10 @@ const Fake = struct { extra: ?[]const u8 = null, is_debuggable: bool = false, disabled: bool = false, + /// The meta-data value; null = the key is absent. meta: ?[]const u8 = null, + /// Overrides `meta` with another read outcome (too long, read error). + read: ?MetaRead = null, vulkan: bool = false, debuggable_calls: usize = 0, @@ -314,9 +333,10 @@ const Fake = struct { self.guard_calls += 1; return self.disabled; } - pub fn metaData(self: *Fake) ?[]const u8 { + pub fn metaData(self: *Fake) MetaRead { self.meta_calls += 1; - return self.meta; + if (self.read) |r| return r; + return if (self.meta) |v| .{ .value = v } else .absent; } pub fn hasVulkan(self: *Fake) bool { self.vulkan_calls += 1; @@ -385,13 +405,52 @@ test "the setting applies when there is no override and the guard is clear" { try expectDecision(.{ .renderer = .gles, .source = .setting }, decide(&g)); } -test "missing meta-data means gles" { +test "the default setting is auto (labelle-android#30)" { + try testing.expectEqual(Setting.auto, default_setting); + try testing.expectEqualStrings("auto", settingLabel(null)); +} + +test "missing meta-data means the default auto: Vulkan on a Vulkan 1.1 device" { var f: Fake = .{ .meta = null, .vulkan = true }; - try expectDecision(.{ .renderer = .gles, .source = .setting }, decide(&f)); + // The default-auto path ran (not `setting`, not an explicit `auto`). + try expectDecision(.{ .renderer = .vulkan, .source = .auto_default }, decide(&f)); + try testing.expectEqual(@as(usize, 1), f.guard_calls); try testing.expectEqual(@as(usize, 1), f.meta_calls); + try testing.expectEqual(@as(usize, 1), f.vulkan_calls); // the device check ran +} + +test "missing meta-data on a device without Vulkan 1.1: gles via the default auto's device check" { + var f: Fake = .{ .meta = null, .vulkan = false }; + try expectDecision(.{ .renderer = .gles, .source = .auto_default }, decide(&f)); + try testing.expectEqual(@as(usize, 1), f.vulkan_calls); +} + +test "explicit gles is the opt-out: no device check, even on a Vulkan device" { + var f: Fake = .{ .meta = "gles", .vulkan = true }; + try expectDecision(.{ .renderer = .gles, .source = .setting }, decide(&f)); try testing.expectEqual(@as(usize, 0), f.vulkan_calls); } +test "an unreadable or too-long meta-data stays gles (not the default)" { + inline for (.{ MetaRead.read_error, MetaRead.too_long }) |r| { + var f: Fake = .{ .read = r, .vulkan = true }; + try expectDecision(.{ .renderer = .gles, .source = .setting }, decide(&f)); + try testing.expectEqual(@as(usize, 0), f.vulkan_calls); + } +} + +test "the crash guard still beats the default auto; the intent still beats both" { + var f: Fake = .{ .meta = null, .disabled = true, .vulkan = true }; + try expectDecision(.{ .renderer = .gles, .source = .crash_guard }, decide(&f)); + try testing.expectEqual(@as(usize, 0), f.meta_calls); + try testing.expectEqual(@as(usize, 0), f.vulkan_calls); + + var g: Fake = .{ .extra = "gles", .is_debuggable = true, .meta = null, .vulkan = true }; + try expectDecision(.{ .renderer = .gles, .source = .intent }, decide(&g)); + try testing.expectEqual(@as(usize, 0), g.guard_calls); + try testing.expectEqual(@as(usize, 0), g.meta_calls); +} + test "an invalid meta-data value means gles" { inline for (.{ "metal", "VULKAN", "Auto", "", "vulkan " }) |bad| { var f: Fake = .{ .meta = bad, .vulkan = true }; @@ -416,6 +475,9 @@ test "env values and log labels" { try testing.expectEqualStrings("intent", Source.intent.label()); try testing.expectEqualStrings("setting", Source.setting.label()); try testing.expectEqualStrings("auto", Source.auto.label()); + try testing.expectEqualStrings("auto", Source.auto_default.label()); + try testing.expectEqualStrings("; default", Source.auto_default.detail()); + try testing.expectEqualStrings("", Source.auto.detail()); try testing.expectEqualStrings("; previous Vulkan start did not complete", Source.crash_guard.detail()); try testing.expectEqualStrings("crash-guard", Source.crash_guard_unrecorded.label()); try testing.expectEqualStrings("; could not record the Vulkan start", Source.crash_guard_unrecorded.detail()); @@ -426,14 +488,12 @@ test "MetaRead: the JNI return code's outcomes, and what each side sees" { const buf = "vulkanXXXX"; const v = MetaRead.fromCode(6, buf); try testing.expectEqualStrings("vulkan", v.value); - try testing.expectEqualStrings("vulkan", v.metaData().?); try testing.expectEqualStrings("vulkan", v.guardSetting().?); - // Absent: a successful read → default gles for both sides. + // Absent: a successful read → the default auto for both sides. const a = MetaRead.fromCode(-1, buf); try testing.expect(a == .absent); - try testing.expect(a.metaData() == null); - try testing.expectEqualStrings("gles", a.guardSetting().?); + try testing.expectEqualStrings("auto", a.guardSetting().?); // Too long: an (invalid) value → gles. const t = MetaRead.fromCode(-3, buf); @@ -444,7 +504,6 @@ test "MetaRead: the JNI return code's outcomes, and what each side sees" { inline for (.{ -2, -7 }) |code| { const e = MetaRead.fromCode(code, buf); try testing.expect(e == .read_error); - try testing.expect(e.metaData() == null); try testing.expect(e.guardSetting() == null); } @@ -456,7 +515,7 @@ test "settingLabel: the effective setting for the crash guard's stamp" { try testing.expectEqualStrings("vulkan", settingLabel("vulkan")); try testing.expectEqualStrings("auto", settingLabel("auto")); try testing.expectEqualStrings("gles", settingLabel("gles")); - try testing.expectEqualStrings("gles", settingLabel(null)); + try testing.expectEqualStrings("auto", settingLabel(null)); // missing = default try testing.expectEqualStrings("gles", settingLabel("metal")); } diff --git a/tests/provider/e2e.py b/tests/provider/e2e.py index a94d8c6..170af7f 100644 --- a/tests/provider/e2e.py +++ b/tests/provider/e2e.py @@ -375,6 +375,11 @@ def check_layout(path): assert 'package="com.labelle.fixture"' in manifest and 'android:versionCode="1"' in manifest, manifest assert 'android:versionName="1.0"' in manifest and 'android:label="Fixture Game"' in manifest, manifest assert 'android:icon="@mipmap/ic_launcher"' in manifest and 'debuggable' not in manifest, manifest + # No `renderer` key: the default `auto` (labelle-android#30) is stamped, + # with the optional Vulkan 1.1 feature (D7). + assert '' in manifest, manifest + assert ('') in manifest, manifest record = json.loads((apk_dir / 'package.json').read_text()) assert record['optimize'] == 'Debug' and record['version_code'] == 1, record assert record['package_name'] == 'com.labelle.fixture', record diff --git a/tools/package/manifest.zig b/tools/package/manifest.zig index 8fd91d2..bcf891c 100644 --- a/tools/package/manifest.zig +++ b/tools/package/manifest.zig @@ -29,8 +29,9 @@ pub const Inputs = struct { version_name: []const u8 = "1.0", /// Whether `icon.stage` wrote `res/mipmap-*/ic_launcher.png`. has_launcher_icon: bool = false, - /// `providers/android.json` `renderer` (labelle-bgfx#172 D2/D7). - renderer: settings_mod.Renderer = .gles, + /// `providers/android.json` `renderer` (labelle-bgfx#172 D2/D7); the + /// settings default, `auto`, when the key is absent. + renderer: settings_mod.Renderer = settings_mod.default_renderer, }; /// The optional Vulkan 1.1 feature (D7). `0x401000` is @@ -41,7 +42,8 @@ pub const vulkan_feature_line = /// The `` meta-data name the runtime reads at launch to pick a /// renderer (labelle-android#27). It is written for every value, `gles` -/// included, so the runtime never has to guess. +/// included, so the runtime never has to guess (an APK without it gets the +/// runtime's default, `auto`, the same as the settings default). pub const renderer_meta_name = "labelle.renderer"; /// The Android `android:screenOrientation` value for `o`. @@ -236,13 +238,12 @@ fn fpInputs(renderer: settings_mod.Renderer) Inputs { }; } -test "defaults reproduce the CLI's manifest plus only the gles renderer meta-data" { +test "explicit gles reproduces the CLI's manifest plus only the gles renderer meta-data" { var arena = std.heap.ArenaAllocator.init(std.testing.allocator); defer arena.deinit(); const a = arena.allocator(); const xml = try generate(a, fpInputs(.gles)); - // The default is gles (D2), and it is written: the runtime never guesses. - try std.testing.expectEqual(settings_mod.Renderer.gles, (Inputs{ .package_name = "", .app_name = "" }).renderer); + // gles is written too: the runtime never guesses. try std.testing.expectEqual(@as(usize, 1), std.mem.count(u8, xml, gles_meta_line)); // The meta-data is the first child of , not in . const app_open_end = std.mem.indexOfPos(u8, xml, std.mem.indexOf(u8, xml, "\n").?; @@ -276,6 +277,50 @@ test "vulkan and auto add the optional Vulkan 1.1 feature and keep GLES 3.0 requ } } +/// Settings → manifest inputs the way `package.zig` maps them. +fn inputsFromSettings(s: settings_mod.Settings) Inputs { + var in = fpInputs(s.renderer); + in.package_name = s.package_name; + return in; +} + +test "default (no renderer key) is auto: the manifest has the optional Vulkan feature and auto meta-data (#30)" { + var arena = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena.deinit(); + const a = arena.allocator(); + var diag: settings_mod.Diagnostic = .{}; + const s = try settings_mod.parse(a, "{\"schema_version\": 1, \"package_name\": \"com.labelle.flying_platform\"}", &diag); + // The mechanism: the key is absent, so the field default applied, and + // that default is `auto`, for both the settings and the bare Inputs. + try std.testing.expectEqual(settings_mod.Renderer.auto, settings_mod.default_renderer); + try std.testing.expectEqual(settings_mod.default_renderer, s.renderer); + try std.testing.expectEqual(settings_mod.default_renderer, (Inputs{ .package_name = "", .app_name = "" }).renderer); + + const xml = try generate(a, inputsFromSettings(s)); + const auto_meta = " \n"; + try std.testing.expectEqual(@as(usize, 1), std.mem.count(u8, xml, auto_meta)); + try std.testing.expectEqual(@as(usize, 1), std.mem.count(u8, xml, vulkan_feature_line)); + // Exactly the explicit-auto manifest. + try std.testing.expectEqualStrings(try generate(a, fpInputs(.auto)), xml); + // And, minus those two lines, the CLI's (gles-era) manifest. + const no_feature = try std.mem.replaceOwned(u8, a, xml, vulkan_feature_line, ""); + const stripped = try std.mem.replaceOwned(u8, a, no_feature, auto_meta, ""); + try std.testing.expectEqualStrings(fp_manifest, stripped); +} + +test "explicit gles in providers/android.json: the old manifest, no Vulkan feature (#30 opt-out)" { + var arena = std.heap.ArenaAllocator.init(std.testing.allocator); + defer arena.deinit(); + const a = arena.allocator(); + var diag: settings_mod.Diagnostic = .{}; + const s = try settings_mod.parse(a, "{\"schema_version\": 1, \"package_name\": \"com.labelle.flying_platform\", \"renderer\": \"gles\"}", &diag); + try std.testing.expectEqual(settings_mod.Renderer.gles, s.renderer); + const xml = try generate(a, inputsFromSettings(s)); + try std.testing.expect(std.mem.indexOf(u8, xml, "vulkan") == null); + const stripped = try std.mem.replaceOwned(u8, a, xml, gles_meta_line, ""); + try std.testing.expectEqualStrings(fp_manifest, stripped); +} + test "omits android:debuggable unless the settings opt in (#737)" { var arena = std.heap.ArenaAllocator.init(std.testing.allocator); defer arena.deinit(); diff --git a/tools/settings.zig b/tools/settings.zig index d5d2bcb..be19c54 100644 --- a/tools/settings.zig +++ b/tools/settings.zig @@ -69,6 +69,13 @@ pub const Renderer = enum { } }; +/// The renderer when `providers/android.json` has no `renderer` key +/// (labelle-android#30; labelle-bgfx#172 D12): `auto`, i.e. Vulkan on +/// devices that report Vulkan >= 1.1, else GLES. The runtime uses the same +/// default for an APK without the meta-data (`src/renderer.zig` +/// `default_setting`). `"renderer": "gles"` opts out. +pub const default_renderer: Renderer = .auto; + /// `"gles", "vulkan", "auto"`, for the rejection message. const renderer_values = blk: { var text: []const u8 = ""; @@ -91,8 +98,8 @@ pub const Settings = struct { abis: []const []const u8 = &.{supported_abi}, signing: ?Signing = null, deploy: ?Deploy = null, - /// `gles` until the Vulkan production gate passes (D12 flips it). - renderer: Renderer = .gles, + /// `default_renderer` (`auto`) when the key is absent. + renderer: Renderer = default_renderer, }; pub const Error = error{ @@ -301,7 +308,7 @@ test "a minimal file gets today's defaults" { try std.testing.expectEqual(@as(usize, 1), s.abis.len); try std.testing.expectEqualStrings("arm64-v8a", s.abis[0]); try std.testing.expect(s.signing == null and s.deploy == null); - try std.testing.expectEqual(Renderer.gles, s.renderer); + try std.testing.expectEqual(Renderer.auto, s.renderer); // app_name falls back to the project title. try std.testing.expectEqualStrings("Project Title", appName(s, "Project Title")); } From 09f537379f2fd68f9dfd753fab30e5680c012c60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Mondaini=20Calv=C3=A3o?= Date: Wed, 30 Sep 2026 22:13:50 -0300 Subject: [PATCH 2/2] fix: a present non-string labelle.renderer meta-data is invalid, not absent (#30) The JNI read returned -1 for both a missing key and a present non-string value (Bundle.getString returns null for both), and MetaRead mapped -1 to the new default `auto`, so a malformed meta-data could enable Vulkan and stamp `auto` in the crash-guard marker. renderer_query.c now uses Bundle.containsKey + get + instanceof String and returns -4 for a present-but-not-a-String/null value. MetaRead.wrong_type maps it to gles via a new `setting_invalid` source (logged as `setting; invalid meta-data`, also used for invalid and too-long strings), never runs the device check, and stamps `gles` for the crash guard. Only -1 (absent) reaches the default auto. Claude-Session: https://claude.ai/code/session_01LpszrcSrLLijQcxxvgWyUQ --- docs/renderer.md | 6 +- src/jni/renderer_query.c | 67 +++++++++++++++------- src/renderer.zig | 121 ++++++++++++++++++++++++++++++++------- 3 files changed, 148 insertions(+), 46 deletions(-) diff --git a/docs/renderer.md b/docs/renderer.md index cb07ceb..a9e2727 100644 --- a/docs/renderer.md +++ b/docs/renderer.md @@ -53,7 +53,7 @@ This uses the newest installed build-tools revision **that actually contains `aa |---|---|---| | 1 | `intent` | The `LABELLE_BGFX_RENDERER` launch extra, `vulkan` or `gles` (exact, lower-case). **Debuggable builds only.** In a non-debuggable build it's ignored with an info line. Any other value is warned about and ignored. An empty extra counts as absent. | | 2 | `crash-guard` | A previous Vulkan start didn't complete, so start on `gles` (see [Crash guard](#crash-guard)). | -| 3 | `setting` | The `labelle.renderer` meta-data: `gles` or `vulkan`. Missing → the default, `auto` (rule 4). Unreadable or too long → `gles`. An invalid value → `gles`, with a warning. | +| 3 | `setting` | The `labelle.renderer` meta-data: `gles` or `vulkan`. Missing (the key isn't in the manifest's meta-data) → the default, `auto` (rule 4). Unreadable → `gles`. Invalid → `gles`, with a warning, logged with `; invalid meta-data`: an unknown string, a too-long one, or a key that's present but isn't a string (for example `android:value="true"`, a number, or `android:resource`). An invalid meta-data never becomes the default and never enables Vulkan. | | 4 | `auto` | The setting is `auto`, or the meta-data is missing (the default; logged with `; default`): `vulkan` if the device has the Vulkan 1.1 feature, else `gles`. If the query fails, it counts as no Vulkan. | The result is exported with `setenv("LABELLE_BGFX_RENDERER", "vulkan"|"gles")`. It always replaces any value already in the process environment. @@ -92,6 +92,7 @@ adb logcat -d -s labelle | grep -E 'renderer: (gles|vulkan)|bgfx: renderer|crash |---|---| | `renderer: vulkan (source: auto)` | labelle-android's choice and the rule that made it: `intent`, `crash-guard`, `setting` or `auto`. Logged once per launch. A default project's APK stamps `auto`, so this is the usual line on a Vulkan 1.1 device. | | `renderer: vulkan (source: auto; default)` | The APK has no `labelle.renderer` meta-data (packaged before the setting existed), so the default `auto` applied. | +| `renderer: gles (source: setting; invalid meta-data)` | The `labelle.renderer` meta-data is present but invalid; see the warning before it. | | `renderer: gles (source: crash-guard; …)` | The crash guard decided; see [Crash guard log lines](#crash-guard-log-lines). | | `android: LABELLE_BGFX_RENDERER=vulkan (launch intent extra)` | The override was applied (debuggable build). | | `android: LABELLE_BGFX_RENDERER cleared (set by a previous launch's intent)` / `… restored to (set by a previous launch's intent)` | A launch without the extra undid the previous launch's override. | @@ -100,6 +101,7 @@ adb logcat -d -s labelle | grep -E 'renderer: (gles|vulkan)|bgfx: renderer|crash | `android: invalid labelle.renderer meta-data '' (expected 'gles', 'vulkan' or 'auto'); using gles` | The APK carries a bad setting. The strict setting parser should make this impossible. | | `android: no labelle.renderer meta-data; using the default (auto)` | The APK has no setting (built before the setting existed). | | `android: labelle.renderer meta-data too long; using gles` | The value doesn't fit the reader's buffer. | +| `android: labelle.renderer meta-data is not a string (use android:value="gles\|vulkan\|auto"); using gles` | The key is present but holds a boolean, number, resource id or null. Treated as invalid, not as missing. | | `android: could not read the labelle.renderer meta-data; using gles (crash-guard marks kept)` | The setting couldn't be read, and the launch uses GLES. The unreadable setting can't itself reset a marker; a marker is still reset if the `versionCode` was read and changed. | | `android: could not set LABELLE_BGFX_RENDERER=` | `setenv` failed; bgfx uses its platform default. | | `bgfx: renderer requested=Vulkan actual=Vulkan` | bgfx started what was asked for. Logged on every init and resume. | @@ -113,7 +115,7 @@ If the game crashes, hangs or is killed while starting on Vulkan, the next launc ### Markers -Two marker files, each holding ` `. The setting is the effective `renderer` setting; an absent one counts as `auto` (the default), an invalid one as `gles`. +Two marker files, each holding ` `. The setting is the effective `renderer` setting; an absent one counts as `auto` (the default), an invalid one (including a non-string value) as `gles`. The guard covers every Vulkan start, whichever rule chose it: an explicit `vulkan`, an explicit `auto`, the default `auto`, or the intent override. A default `auto` that resolves to Vulkan is marked before init, cleared once stable, and sends the next launch to GLES after a crashed start, exactly like an explicit `vulkan`. diff --git a/src/jni/renderer_query.c b/src/jni/renderer_query.c index ce6f542..c2ea538 100644 --- a/src/jni/renderer_query.c +++ b/src/jni/renderer_query.c @@ -7,7 +7,7 @@ // labelle-android#26): // getPackageManager() // .getApplicationInfo(getPackageName(), GET_META_DATA) -// .metaData.getString("labelle.renderer") +// .metaData .containsKey / .get("labelle.renderer") instanceof String // * `auto`'s device check: // getPackageManager().hasSystemFeature("android.hardware.vulkan.version", // 0x401000) // Vulkan 1.1 @@ -46,12 +46,21 @@ static jobject package_manager(JNIEnv *env, jobject activity) { } // Copy the `labelle.renderer` meta-data string into `buf` (NUL-terminated). -// Returns its length (>= 0), -1 when the key is absent (or not a string), -// -2 on any JNI failure, -3 when the value does not fit in `buf_cap`. -// -1 (absent) and -2 (failure) MUST stay distinct: `renderer.MetaRead` maps -// -1 to the default `gles` but -2 to "unknown", and the crash guard keeps its -// marks on unknown rather than reading a transient failure as a setting -// change (labelle-android#33 review). +// The return-code contract (mirrored by `renderer.MetaRead.fromCode`, whose +// host tests pin the Zig side; this TU only builds for Android, so it has no +// host test of its own): +// >= 0 the value is a String; its length (the bytes in `buf`) +// -1 ABSENT: no meta-data bundle at all, or `containsKey` is false. +// The ONLY outcome that means the default `auto`. +// -2 READ FAILURE: any JNI failure or pending exception → `gles`, and +// the crash guard keeps its marks (an unknown setting must not read +// as a setting change; labelle-android#33 review) +// -3 TOO LONG: a String that does not fit in `buf_cap` → `gles` +// -4 WRONG TYPE: the key is present but its value is not a String +// (`android:value="true"` / `"1"` arrive as Boolean / Integer, +// `android:resource` as an Integer id) or is null → `gles`. +// MUST stay distinct from -1, or a malformed meta-data would become +// the default `auto` and enable Vulkan (labelle-android#39 review). // Never leaves a Java exception pending and never leaks a local ref. // Callable from any thread (attached or not). int labelle_android_read_renderer_meta(const void *activity_ptr, char *buf, size_t buf_cap) { @@ -86,25 +95,39 @@ int labelle_android_read_renderer_meta(const void *activity_ptr, char *buf, size result = -1; } else { jclass bundle_cls = (*env)->GetObjectClass(env, bundle); - jmethodID get_string = bundle_cls ? (*env)->GetMethodID(env, bundle_cls, "getString", "(Ljava/lang/String;)Ljava/lang/String;") : NULL; - jstring jkey = (get_string && !(*env)->ExceptionCheck(env)) ? (*env)->NewStringUTF(env, "labelle.renderer") : NULL; - jstring jval = (jkey && !(*env)->ExceptionCheck(env)) ? (jstring)(*env)->CallObjectMethod(env, bundle, get_string, jkey) : NULL; + // `containsKey` separates "absent" from "present but not a + // String"; `get` + `instanceof String` then reads it + // (`getString` returns null for both, so it can't). + jmethodID contains_key = bundle_cls ? (*env)->GetMethodID(env, bundle_cls, "containsKey", "(Ljava/lang/String;)Z") : NULL; + jmethodID get_obj = (contains_key && !(*env)->ExceptionCheck(env)) ? (*env)->GetMethodID(env, bundle_cls, "get", "(Ljava/lang/String;)Ljava/lang/Object;") : NULL; + jclass string_cls = (get_obj && !(*env)->ExceptionCheck(env)) ? (*env)->FindClass(env, "java/lang/String") : NULL; + jstring jkey = (string_cls && !(*env)->ExceptionCheck(env)) ? (*env)->NewStringUTF(env, "labelle.renderer") : NULL; if (jkey != NULL && !(*env)->ExceptionCheck(env)) { - if (jval == NULL) { - // Missing key, or a non-string value. + jboolean present = (*env)->CallBooleanMethod(env, bundle, contains_key, jkey); + if ((*env)->ExceptionCheck(env)) { + // read failure; cleared below + } else if (!present) { result = -1; } else { - // The accepted values are ASCII, so modified - // UTF-8 is byte-identical for every valid one. - jsize chars = (*env)->GetStringLength(env, jval); - jsize bytes = (*env)->GetStringUTFLength(env, jval); - if (bytes < 0 || (size_t)bytes + 1 > buf_cap) { - result = -3; + jobject jval = (*env)->CallObjectMethod(env, bundle, get_obj, jkey); + if ((*env)->ExceptionCheck(env)) { + // read failure; cleared below + } else if (jval == NULL || !(*env)->IsInstanceOf(env, jval, string_cls)) { + // Present, but null or not a String. + result = -4; } else { - (*env)->GetStringUTFRegion(env, jval, 0, chars, buf); - if (!(*env)->ExceptionCheck(env)) { - buf[bytes] = 0; - result = (int)bytes; + // The accepted values are ASCII, so modified + // UTF-8 is byte-identical for every valid one. + jsize chars = (*env)->GetStringLength(env, (jstring)jval); + jsize bytes = (*env)->GetStringUTFLength(env, (jstring)jval); + if (bytes < 0 || (size_t)bytes + 1 > buf_cap) { + result = -3; + } else { + (*env)->GetStringUTFRegion(env, (jstring)jval, 0, chars, buf); + if (!(*env)->ExceptionCheck(env)) { + buf[bytes] = 0; + result = (int)bytes; + } } } } diff --git a/src/renderer.zig b/src/renderer.zig index 75c0a7e..c498915 100644 --- a/src/renderer.zig +++ b/src/renderer.zig @@ -15,10 +15,12 @@ //! new version or a changed setting tries Vulkan again. //! 3. `setting`: the provider setting, stamped into the manifest as //! `` -//! (labelle-android#26). Missing → the default, `auto` (rule 4, source -//! `auto` with `; default`; labelle-android#30). Unreadable (a JNI -//! error) or too long → `gles`; an invalid value → `gles` with a -//! warning. +//! (labelle-android#26). Missing (the key is not in the bundle) → the +//! default, `auto` (rule 4, source `auto` with `; default`; +//! labelle-android#30). Unreadable (a JNI error) → `gles`. Invalid — +//! an unknown string, a too-long one, or a present non-string/null +//! value — → `gles` with a warning (source `setting` with +//! `; invalid meta-data`); never the default, never Vulkan. //! 4. `auto` (the setting's third value, and the default since 0.5.0): //! Vulkan when the device reports `android.hardware.vulkan.version` //! >= 1.1 (0x401000), else `gles`. @@ -30,8 +32,9 @@ //! `setenv("LABELLE_BGFX_RENDERER", "vulkan"|"gles", 1)` and one log line: //! `renderer: (source: intent|crash-guard|setting|auto)`, with //! `; previous Vulkan start did not complete` (or `; could not record the -//! Vulkan start`) after `crash-guard`, and `; default` after an `auto` that -//! came from a missing meta-data. A default `auto` that picks Vulkan goes +//! Vulkan start`) after `crash-guard`, `; default` after an `auto` that +//! came from a missing meta-data, and `; invalid meta-data` after a +//! `setting` that was present but invalid. A default `auto` that picks Vulkan goes //! through the crash guard exactly like an explicit `vulkan` or `auto`. //! //! ## Where it runs @@ -77,6 +80,10 @@ pub const Source = enum { /// launch stays off Vulkan (`afterStart`). crash_guard_unrecorded, setting, + /// `gles` because the meta-data is present but invalid (an unknown or + /// too-long string, or not a string at all). Logged as `setting` with + /// `; invalid meta-data`. + setting_invalid, auto, /// `auto` because the meta-data is missing: the default setting /// (labelle-android#30). Logged as `auto` with `; default`. @@ -86,7 +93,7 @@ pub const Source = enum { return switch (self) { .intent => "intent", .crash_guard, .crash_guard_unrecorded => "crash-guard", - .setting => "setting", + .setting, .setting_invalid => "setting", .auto, .auto_default => "auto", }; } @@ -97,6 +104,7 @@ pub const Source = enum { .crash_guard => "; previous Vulkan start did not complete", .crash_guard_unrecorded => "; could not record the Vulkan start", .auto_default => "; default", + .setting_invalid => "; invalid meta-data", else => "", }; } @@ -126,9 +134,11 @@ pub const Setting = enum { gles, vulkan, auto }; /// `gles` is the explicit opt-out. pub const default_setting: Setting = .auto; -/// The three (four) outcomes of the `labelle.renderer` meta-data read. The -/// JNI helper's return code carries them across the C boundary: -/// `>= 0` = value length, `-1` = absent, `-3` = too long, `-2` (or any +/// The outcomes of the `labelle.renderer` meta-data read. The JNI helper's +/// return code carries them across the C boundary (the contract is spelled +/// out on `labelle_android_read_renderer_meta` in `jni/renderer_query.c`): +/// `>= 0` = value length, `-1` = absent (`containsKey` false, or no bundle), +/// `-3` = too long, `-4` = present but not a String (or null), `-2` (or any /// other negative) = read error. pub const MetaRead = union(enum) { /// Read successfully (may still be an invalid value; `decide` warns). @@ -139,6 +149,10 @@ pub const MetaRead = union(enum) { /// Read successfully; the value does not fit the buffer (so it is not a /// valid setting) → `gles`, like any invalid value. too_long, + /// Read successfully; the key is present but its value is not a String + /// (a Boolean, an Integer, a resource id) or is null. Invalid → `gles`, + /// NOT the default: a malformed meta-data must never enable Vulkan. + wrong_type, /// The JNI walk failed: the setting is UNKNOWN. read_error, @@ -147,19 +161,20 @@ pub const MetaRead = union(enum) { return switch (n) { -1 => .absent, -3 => .too_long, + -4 => .wrong_type, else => .read_error, }; } /// The setting the crash guard compares/records: the effective setting - /// for every SUCCESSFUL read (absent → the default `auto`; invalid/too - /// long → `gles`), and null for a read error, which the guard treats as + /// for every SUCCESSFUL read (absent → the default `auto`; invalid, too + /// long or wrong type → `gles`), and null for a read error, which the guard treats as /// unknown (marks kept). pub fn guardSetting(self: MetaRead) ?[]const u8 { return switch (self) { .value => |v| settingLabel(v), .absent => settingLabel(null), - .too_long => "gles", + .too_long, .wrong_type => "gles", .read_error => null, }; } @@ -197,7 +212,8 @@ pub fn parseSetting(value: []const u8) ?Setting { /// * `debuggable() bool` — asked only when the extra is present /// * `vulkanDisabled() bool` — the D11 crash guard /// * `metaData() MetaRead` — the `labelle.renderer` meta-data read -/// (absent → `default_setting`; read error or too long → `gles`) +/// (absent → `default_setting`; read error, invalid, too long or wrong +/// type → `gles`) /// * `hasVulkan() bool` — asked only for `auto` (explicit or default) pub fn decide(q: anytype) Decision { if (q.intentExtra()) |v| { @@ -217,13 +233,16 @@ pub fn decide(q: anytype) Decision { if (q.vulkanDisabled()) return .{ .renderer = .gles, .source = .crash_guard }; const meta: MetaRead = q.metaData(); const setting: Setting = switch (meta) { - .value => |raw| parseSetting(raw) orelse blk: { + .value => |raw| parseSetting(raw) orelse { std.log.warn("android: invalid {s} meta-data '{s}' (expected 'gles', 'vulkan' or 'auto'); using gles", .{ meta_data_name, raw }); - break :blk .gles; + return .{ .renderer = .gles, .source = .setting_invalid }; }, .absent => default_setting, - // Not a usable setting: stay on the safe renderer. - .too_long, .read_error => .gles, + // Present but unusable: the safe renderer, never the default. + // (Warned once where the read happens, `JniQuery.metaRead`.) + .too_long, .wrong_type => return .{ .renderer = .gles, .source = .setting_invalid }, + // Unknown: stay on the safe renderer. + .read_error => .gles, }; return switch (setting) { .gles => .{ .renderer = .gles, .source = .setting }, @@ -271,6 +290,7 @@ const JniQuery = struct { .value => {}, .absent => std.log.info("android: no {s} meta-data; using the default ({s})", .{ meta_data_name, @tagName(default_setting) }), .too_long => std.log.warn("android: {s} meta-data too long; using gles", .{meta_data_name}), + .wrong_type => std.log.warn("android: {s} meta-data is not a string (use android:value=\"gles|vulkan|auto\"); using gles", .{meta_data_name}), .read_error => std.log.warn("android: could not read the {s} meta-data; using gles (crash-guard marks kept)", .{meta_data_name}), } self.meta = m; @@ -315,6 +335,10 @@ const Fake = struct { meta: ?[]const u8 = null, /// Overrides `meta` with another read outcome (too long, read error). read: ?MetaRead = null, + /// Overrides both with a raw JNI return code, decoded by + /// `MetaRead.fromCode` exactly as `JniQuery` does (with `code_buf`). + code: ?c_int = null, + code_buf: []const u8 = "", vulkan: bool = false, debuggable_calls: usize = 0, @@ -335,6 +359,7 @@ const Fake = struct { } pub fn metaData(self: *Fake) MetaRead { self.meta_calls += 1; + if (self.code) |n| return MetaRead.fromCode(n, self.code_buf); if (self.read) |r| return r; return if (self.meta) |v| .{ .value = v } else .absent; } @@ -431,14 +456,59 @@ test "explicit gles is the opt-out: no device check, even on a Vulkan device" { try testing.expectEqual(@as(usize, 0), f.vulkan_calls); } -test "an unreadable or too-long meta-data stays gles (not the default)" { - inline for (.{ MetaRead.read_error, MetaRead.too_long }) |r| { +test "an unreadable meta-data stays gles (not the default)" { + var f: Fake = .{ .read = .read_error, .vulkan = true }; + try expectDecision(.{ .renderer = .gles, .source = .setting }, decide(&f)); + try testing.expectEqual(@as(usize, 0), f.vulkan_calls); +} + +test "a too-long or non-string meta-data is invalid: gles, never the default" { + inline for (.{ MetaRead.too_long, MetaRead.wrong_type }) |r| { var f: Fake = .{ .read = r, .vulkan = true }; - try expectDecision(.{ .renderer = .gles, .source = .setting }, decide(&f)); + try expectDecision(.{ .renderer = .gles, .source = .setting_invalid }, decide(&f)); try testing.expectEqual(@as(usize, 0), f.vulkan_calls); } } +test "JNI codes end to end: only -1 (absent) reaches the default auto" { + // -1: the key is not in the bundle → the default auto; the device + // check RUNS, and the guard stamp is `auto`. + var absent: Fake = .{ .code = -1, .vulkan = true }; + try expectDecision(.{ .renderer = .vulkan, .source = .auto_default }, decide(&absent)); + try testing.expectEqual(@as(usize, 1), absent.vulkan_calls); + try testing.expectEqualStrings("auto", absent.metaData().guardSetting().?); + + // -4: present but a Boolean / Integer / resource id / null (labelle- + // android#39 review) → gles via the INVALID path, the device check + // never runs, and the guard stamp is `gles`, not `auto`. + var wrong: Fake = .{ .code = -4, .vulkan = true }; + try expectDecision(.{ .renderer = .gles, .source = .setting_invalid }, decide(&wrong)); + try testing.expectEqual(@as(usize, 0), wrong.vulkan_calls); + try testing.expectEqual(@as(usize, 1), wrong.guard_calls); + const wrong_stamp = wrong.metaData().guardSetting().?; + try testing.expectEqualStrings("gles", wrong_stamp); + try testing.expect(!std.mem.eql(u8, wrong_stamp, "auto")); + + // -2 (and any unknown negative): a read failure → gles, no device + // check, and an UNKNOWN guard setting (marks kept). + inline for (.{ -2, -9 }) |n| { + var err: Fake = .{ .code = n, .vulkan = true }; + try expectDecision(.{ .renderer = .gles, .source = .setting }, decide(&err)); + try testing.expectEqual(@as(usize, 0), err.vulkan_calls); + try testing.expect(err.metaData().guardSetting() == null); + } + + // -3: too long → invalid, gles. + var long: Fake = .{ .code = -3, .vulkan = true }; + try expectDecision(.{ .renderer = .gles, .source = .setting_invalid }, decide(&long)); + try testing.expectEqual(@as(usize, 0), long.vulkan_calls); + + // >= 0: a String, parsed as before. + var ok: Fake = .{ .code = 4, .code_buf = "autoXX", .vulkan = true }; + try expectDecision(.{ .renderer = .vulkan, .source = .auto }, decide(&ok)); + try testing.expectEqual(@as(usize, 1), ok.vulkan_calls); +} + test "the crash guard still beats the default auto; the intent still beats both" { var f: Fake = .{ .meta = null, .disabled = true, .vulkan = true }; try expectDecision(.{ .renderer = .gles, .source = .crash_guard }, decide(&f)); @@ -454,7 +524,7 @@ test "the crash guard still beats the default auto; the intent still beats both" test "an invalid meta-data value means gles" { inline for (.{ "metal", "VULKAN", "Auto", "", "vulkan " }) |bad| { var f: Fake = .{ .meta = bad, .vulkan = true }; - try expectDecision(.{ .renderer = .gles, .source = .setting }, decide(&f)); + try expectDecision(.{ .renderer = .gles, .source = .setting_invalid }, decide(&f)); try testing.expectEqual(@as(usize, 0), f.vulkan_calls); } } @@ -482,6 +552,8 @@ test "env values and log labels" { try testing.expectEqualStrings("crash-guard", Source.crash_guard_unrecorded.label()); try testing.expectEqualStrings("; could not record the Vulkan start", Source.crash_guard_unrecorded.detail()); try testing.expectEqualStrings("", Source.setting.detail()); + try testing.expectEqualStrings("setting", Source.setting_invalid.label()); + try testing.expectEqualStrings("; invalid meta-data", Source.setting_invalid.detail()); } test "MetaRead: the JNI return code's outcomes, and what each side sees" { @@ -500,6 +572,11 @@ test "MetaRead: the JNI return code's outcomes, and what each side sees" { try testing.expect(t == .too_long); try testing.expectEqualStrings("gles", t.guardSetting().?); + // Wrong type (present, not a String): invalid → gles, NOT absent. + const w = MetaRead.fromCode(-4, buf); + try testing.expect(w == .wrong_type); + try testing.expectEqualStrings("gles", w.guardSetting().?); + // Read error: the renderer still gets gles, the guard gets UNKNOWN. inline for (.{ -2, -7 }) |code| { const e = MetaRead.fromCode(code, buf);