Fix iOS camera native build break + gate new Camera API on actual usage#5177
Merged
Conversation
The new com.codename1.camera native bridge (CN1Camera.m, #5126) failed to compile in cloud builds: "call to undeclared function 'fromNSData'". Three related problems: 1. fromNSData() was never defined anywhere. The real NSData->byte[] helper is nsDataToByteArr(NSData*) in IOSNative.m (it pulls thread state internally, no thread-state arg). Fixed both call sites + added the extern declaration. 2. The new natives were gated on INCLUDE_CAMERA_USAGE, which IPhoneBuilder flips whenever an app declares an NSCameraUsageDescription. So any app that set a camera permission for the OLD modal Capture API dragged in the new, never-compiled AVFoundation natives -- exactly how this surfaced on a customer build that doesn't use the new API. Introduce a dedicated INCLUDE_CN1_CAMERA define that IPhoneBuilder flips only when the bytecode scan sees com.codename1.camera.* (mirrors usesNfc / CN1_INCLUDE_NFC). The old Capture natives in IOSNative.m keep INCLUDE_CAMERA_USAGE. 3. CI never compiled this code because no test app referenced the API, so the bug shipped unseen. scripts/hellocodenameone now calls Camera.isSupported()/getCameras() at startup (no session, no permission prompt) and declares the usage description, so the natives compile on every iOS/Mac CI run and CameraX is pulled into the Android build. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
Android screenshot updatesCompared 124 screenshots: 123 matched, 1 updated.
Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
Contributor
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
The native iOS/Mac CI builds (which now actually compile CN1Camera.m thanks to the hellocodenameone camera usage) failed with "error: expected ')'" on four fromNSString(...) calls. fromNSString takes (threadState, NSString*), so it needs CN1_THREAD_GET_STATE_PASS_ARG (expands to "getThreadLocalData(),") -- the code used CN1_THREAD_GET_STATE_PASS_SINGLE_ARG (no trailing comma), which is only valid when thread state is the sole argument. These four were present in the original error.txt alongside the fromNSData errors; the earlier commit fixed fromNSData but missed the fromNSString calls because that code path had never been compiled until CI exercised it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Compared 95 screenshots: 95 matched. |
Collaborator
Author
|
Compared 124 screenshots: 124 matched. Benchmark Results
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 124 screenshots: 124 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
Collaborator
Author
|
Compared 124 screenshots: 124 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
DesktopModeScreenshotTest was added in #5170 ("Deepen desktop integration") but no reference screenshots were committed, so every CI screenshot run reports DesktopMode as "missing reference" (non-fatal, but it means the test has no baseline). This seeds the Android baseline from the full-resolution emulator render produced by the scripts-android CI job, so the Android instrumentation screenshot suite now has a golden to compare against. The captured screen is the desktop-mode list (title bar + colored rows), 320x640, matching the other Android goldens. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… hang) The strict screenshot guard only counted "missing_actual" entries from the comparison JSON. Those entries exist only for tests the runner *registered* an actual for. When a suite hangs and the app is killed mid-run (SIGTERM), every remaining test is never registered at all -- so it is invisible to that check. That is exactly how the iOS Metal suite reported "72/72 matched" and passed while 51 of its 123 stored references never produced an image: it hung on ChartRotatedScreenshotTest, the app was terminated, and the ~51 tests after it were silently dropped from the comparison set rather than flagged. Add a reference-coverage guard: every PNG in the reference dir must have a corresponding produced actual. Uncaptured references beyond CN1SS_ALLOWED_MISSING fail with exit 17 (the same code/intent the surrounding comment already described as "fewer screenshots than stored references"). Tolerance reuses CN1SS_ALLOWED_MISSING; bypass with CN1SS_SKIP_COUNT_CHECK=1 when seeding a new baseline set. Implemented bash-3.2-safe for the macOS runners. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…topMode
ChatView_{dark,light} and ToolbarTheme_{dark,light} drifted when #5174
regenerated Themes/iOSModernTheme.res (the Mac Catalyst native theme), changing
the toolbar/chat chrome. The Mac native screenshot goldens were never
regenerated because Themes/*.res changes don't trigger scripts-mac-native
(it keys on native-themes/ios-modern/** and scripts/**). Updated from the
full-resolution Mac Catalyst CI render. Also seeds the missing DesktopMode
baseline (DesktopModeScreenshotTest, added in #5170).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…DesktopMode Same drift as the Mac goldens: #5174 regenerated Themes/iOSModernTheme.res, changing ChatView_{dark,light} and ToolbarTheme_{dark,light} chrome, but the iOS screenshot goldens (both the GL set in scripts/ios/screenshots and the Metal set in scripts/ios/screenshots-metal) were never regenerated because Themes/*.res changes don't trigger scripts-ios. Updated from the full-res CI renders (GL from the build-ios job, Metal from build-ios-metal). Also seeds the missing DesktopMode baseline for both backends. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…5170) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Compared 11 screenshots: 11 matched. |
…wait
The iOS Metal screenshot suite intermittently hung during readback (observed
on ChartRotatedScreenshotTest): the app stalls with no crash and no Metal
validation error, the runner SIGTERMs it, and every test after the stall is
silently dropped (the suite reported 72/123 "matched" and still passed before
the cn1ss reference-coverage guard was added).
Root cause class: [cb waitUntilCompleted] in the mutable-image readback path
blocks the calling thread forever when the command buffer was created (a
mutable image's Begin) but never committed (readback racing ahead of End), and
blocks indefinitely on a genuinely stuck GPU buffer. Either mode is an
unrecoverable hang.
Replace the three unbounded waits (CN1MetalFlushMutableImageSync + the two
blit-to-shared readbacks) with cn1MetalWaitCommandBufferBounded():
- returns immediately for an uncommitted buffer (NotEnqueued/Enqueued) --
it will never be submitted, so there is nothing to await;
- polls status with an 8s deadline for committed/scheduled buffers as a
backstop against a stuck GPU.
On a non-completed buffer the caller reads back whatever is in the texture, so
the affected screenshot fails visibly against its golden instead of hanging the
whole suite. Verified the file compiles clean (clang -fsyntax-only, modules,
iphonesimulator26.2 SDK).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… suite The reference-coverage guard flagged scripts/javase/screenshots as having 12 references but the JavaSE simulator-integration suite only produces 11 (window modes, inspector, network monitor, test recorder, 4 native themes). graphics-inscribed-triangle-grid.png is a device-runner graphics-test golden (it lives in every platform's screenshots dir) that was accidentally committed into the javase simulator-integration dir in #4939. That suite never emits a graphics-* screenshot and is the only consumer of scripts/javase/screenshots, so the file is a true orphan. Removing it makes the dir match what the suite produces. (This is exactly the kind of silent cruft the new guard surfaces.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The reference-coverage guard failed build-ios: scripts/ios/screenshots had 128 references but the suite produces 124. The 4 extras are stale CamelCase goldens -- GraphicsPipeline, GraphicsShapesAndGradients, GraphicsStateAndText, GraphicsTransformations -- left over from before the graphics tests were renamed to kebab-case (graphics-*). They exist only in the iOS GL dir (no other platform), no test produces them, and the current kebab-case graphics-* goldens are produced and matched. The old missing_actual check never saw these (no test registers an actual for them), so they shipped silently; the new guard surfaces them. Removing makes the GL dir match what the suite emits. The TimeApiTest / LocalNotificationOverrideTest CN1SS:ERR failures visible in the same job are pre-existing on master (master's build-ios is green with them) and unrelated to this branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ed tests) The reference-coverage guard added earlier compares every stored golden against the produced actuals and fails when any golden has no actual. That conflates two different things: a hung/killed suite (the metal case it was meant to catch) AND tests that a platform intentionally skips but still keeps goldens for. The latter false-failed the JavaScript suite -- HTML5 parks ~14 tests (ChatView, ChatInput, Sheet, chart-rotated-pie, chart-transform, css-gradients, ...) via Cn1ssDeviceRunner.shouldForceTimeoutInHtml5 yet keeps their goldens, so the guard reported them as "uncaptured" even though the JS comparison itself was green (95/95 matched). Reverting it. The metal screenshot hang it was meant to backstop is already prevented at the source by the bounded command-buffer wait in CN1Metalcompat.m (this PR). A precise, skip-safe hang detector (require CN1SS:SUITE:FINISHED in the device log) is the correct replacement and can be added separately rather than blocking this fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Problem
The new
com.codename1.cameranative bridge (CN1Camera.m, #5126) failed to compile in cloud builds:A customer build (using the old modal Capture API) hit this in code it never uses, and no CI build ever caught it. Three related root causes:
1.
fromNSDatanever existedThe code assumed a helper symmetric to
fromNSString. The real NSData→byte[]helper isnsDataToByteArr(NSData*)inIOSNative.m(it pulls thread state viagetThreadLocalData()internally, so it takes no thread-state arg). Fixed both call sites + added theexterndeclaration.2. The native gate was keyed to a permission string, not API usage
CN1Camera.mwas wrapped in#ifdef INCLUDE_CAMERA_USAGE, whichIPhoneBuilderflips whenever an app declares anNSCameraUsageDescription. So any app that set a camera permission for the oldCaptureAPI dragged in the new, never-compiled AVFoundation natives — exactly how the break surfaced.Introduced a dedicated
INCLUDE_CN1_CAMERAdefine thatIPhoneBuilderflips only when the bytecode scan seescom.codename1.camera.*(mirrors the existingusesNfc/CN1_INCLUDE_NFCpattern, and stays in lockstep with the AVFoundation framework injection already keyed on the same scan viaAiDependencyTable). The oldCapturenatives inIOSNative.mkeepINCLUDE_CAMERA_USAGE.3. CI never compiled the camera natives
Because no test app referenced the API, the
#ifdefcompiled it out of every CI iOS/Mac build.scripts/hellocodenameonenow callsCamera.isSupported()/getCameras()at startup (no session opened, no permission prompt) and declares the usage description — so the natives compile on every iOS/Mac CI run and CameraX is pulled into the Android build.Companion PR
The cloud build server runs its own fork of this builder — see the matching BuildDaemon PR for the
INCLUDE_CN1_CAMERAflip there (without it, the new camera code would never compile on cloud builds).Testing
mvn -pl codenameone-maven-plugin compile→ BUILD SUCCESS.scripts-ios/scripts-mac-native) will now actually compileCN1Camera.m.🤖 Generated with Claude Code