Skip to content

Fix build sample on pull request#663

Merged
ThomasGorisse merged 2 commits intoSceneView:mainfrom
hannesa2:BuildSample
Mar 20, 2026
Merged

Fix build sample on pull request#663
ThomasGorisse merged 2 commits intoSceneView:mainfrom
hannesa2:BuildSample

Conversation

@hannesa2
Copy link
Contributor

@hannesa2 hannesa2 commented Mar 19, 2026

What

Fix of #662

close #662

Why

Fix of #662

How

Fix build

Checklist

  • /review passed — no threading, Compose API, or style issues
  • /document run — KDoc updated for changed public APIs, llms.txt updated if signatures changed
  • /test run — new tests added for new behaviour
  • ./gradlew :sceneview:assembleDebug :arsceneview:assembleDebug passes
  • Filament materials recompiled (if .mat files changed)
  • Minimal diff — no unrelated reformatting

AI-assisted contributions welcome.
Run /contribute in Claude Code for a guided workflow that handles review, docs, and test generation automatically.
MCP server: npx -y sceneview-mcp — gives Claude full SceneView API context.

@hannesa2
Copy link
Contributor Author

Ahh, I see you event don't build pull requests

I recommend to change

  pull_request:
    branches: [main]

to

  pull_request:
 

@hannesa2
Copy link
Contributor Author

Next, please consider to run this once and commit the output to avoid too much noise in a pull request
image

Copy link
Contributor

@ThomasGorisse ThomasGorisse left a comment

Choose a reason for hiding this comment

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

Thanks @hannesa2 — this addresses issue #662 which was a real blocker. A few notes:

autopilot-demo fix: The engine argument removal is correct — PlaneNode, CubeNode, CylinderNode inside a Scene { } block are composables provided by SceneScope, not constructors, so engine must not be passed. However, this fix was already merged to main in 9f51027 so this part of the PR will conflict.

The unused import removals (CubeNode, CylinderNode, PlaneNode) are a net improvement — those should stay.

CI change: Replacing the two separate build steps with a single ./gradlew assembleDebug is cleaner, but main now has an explicit Build samples step added after Build libraries. The cleanest resolution would be to collapse all three into one assembleDebug call and drop the separate samples step — which is exactly what you did.

Suggested path to merge:

  1. Rebase onto current main (the engine removal will conflict — just accept main's version)
  2. Keep only the import removals and the CI consolidation (assembleDebug replacing both build steps)
  3. Drop the duplicate Build samples step since assembleDebug covers everything

The CI change alone is worth merging once the conflict is resolved. Happy to help rebase if needed.

ThomasGorisse added a commit that referenced this pull request Mar 19, 2026
AR samples cannot be tested locally on Apple Silicon Macs because:
- The Emulator only ships the darwin-aarch64 QEMU binary
- ARCore's emulator APK is x86-only (x86 + x86_64 ABIs)
- ARM64 Google Play system images expose the back camera as ID "1"
  instead of "0", causing ARCore session creation to fail

Fix: run AR emulator tests on GitHub Actions (x86_64 Ubuntu with KVM)
using the official ARCore x86_64 emulator APK + virtualscene camera.
Screenshots are uploaded as CI artifacts after each run.

Also consolidates the two build steps into a single ./gradlew assembleDebug
(adopting the cleaner approach from PR #663 by @hannesa2).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Contributor

@ThomasGorisse ThomasGorisse left a comment

Choose a reason for hiding this comment

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

Looks good — the parameter removal matches the composable node API and the CI build step change correctly builds all modules. Merging after CI passes.

@ThomasGorisse ThomasGorisse added the bug Something isn't working label Mar 20, 2026
Copy link
Contributor

@ThomasGorisse ThomasGorisse left a comment

Choose a reason for hiding this comment

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

Looks good — the engine parameter removal matches the composable node API and the CI build step change correctly builds all modules. Merging after CI passes.

@ThomasGorisse ThomasGorisse merged commit 49da22f into SceneView:main Mar 20, 2026
This was referenced Mar 20, 2026
ThomasGorisse added a commit that referenced this pull request Mar 20, 2026
* qa: add emulator rendering screenshots — 2026-03-20

Captured from Pixel_9 AVD (SwiftShader GPU, API 35) during daily
maintenance sweep. All 4 non-AR samples render correctly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* ci: add AR emulator job using x86_64 Linux runner + ARCore emulator APK

AR samples cannot be tested locally on Apple Silicon Macs because:
- The Emulator only ships the darwin-aarch64 QEMU binary
- ARCore's emulator APK is x86-only (x86 + x86_64 ABIs)
- ARM64 Google Play system images expose the back camera as ID "1"
  instead of "0", causing ARCore session creation to fail

Fix: run AR emulator tests on GitHub Actions (x86_64 Ubuntu with KVM)
using the official ARCore x86_64 emulator APK + virtualscene camera.
Screenshots are uploaded as CI artifacts after each run.

Also consolidates the two build steps into a single ./gradlew assembleDebug
(adopting the cleaner approach from PR #663 by @hannesa2).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* docs(maintain): document Apple Silicon AR emulator limitation

AR samples cannot be tested locally on Apple Silicon:
- darwin-aarch64 QEMU only; ARCore emulator APK is x86-only
- ARM64 system images expose back camera as "1"/"10", not "0"
- ARCore hardcodes camera ID "0" during session creation

AR screenshots come from the ar-emulator CI job (x86_64 Ubuntu + KVM).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* ci: fix AR emulator stability — wait for launcher, dismiss ANR dialogs

The first CI run captured ANR crash dialogs instead of AR samples.
Root cause: emulator launcher wasn't fully settled before launching apps.

Fixes:
- Add 15s settle time + screen unlock after emulator boot
- Dismiss any ANR dialogs before installing/launching
- Increase per-sample wait to 20s for ARCore session init
- Add ar-augmented-image to the screenshot run

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* ci: fix sh syntax error in AR emulator script

The for loop used bash-specific syntax but the runner uses /bin/sh.
Replace with explicit keyevent repetitions and add shebang.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* ci: install sample APKs before launching in AR emulator script

The previous script built but never installed the AR sample APKs,
so am start captured the home screen instead of the running app.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* ci: kill Pixel Launcher before each AR screenshot to prevent ANR overlay

The Pixel Launcher ANR dialog was covering the AR samples.
ARCore itself works correctly — the fix is simply to stop the launcher
process before launching each sample app.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* qa: update AR emulator screenshots — ARCore session confirmed working

All 3 AR samples launch and initialize an ARCore session correctly.
Camera background is white (SwiftShader headless can't render the
virtualscene texture) but "Move your phone slowly to detect a surface"
confirms the AR session is active and plane detection is running.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* deps: bump Filament 1.56.0 → 1.70.0, fix API break, recompile materials

KTX1Loader.createIndirectLight() and createSkybox() now return bundle
objects (IndirectLightBundle / SkyboxBundle) instead of the raw Filament
types directly. Extract .indirectLight and .skybox from the bundles in
SceneFactories and EnvironmentLoader.

All 10 .filamat files recompiled with matc from the v1.70.0 tools
archive (new material version required per Filament release notes).

Changelog: https://github.com/google/filament/releases/tag/v1.70.0

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

samples doesn't build

2 participants