Skip to content

Fix the Android review leftovers from the layout restructure - #42

Merged
Xget7 merged 4 commits into
mainfrom
followup-android
Oct 20, 2026
Merged

Xget7 merged 4 commits into
mainfrom
followup-android

Conversation

@Xget7

@Xget7 Xget7 commented Oct 20, 2026

Copy link
Copy Markdown
Owner

Summary

Review leftovers from #39 and #40, Android only.

packages/splatkit-android/
├── README.md                    # alignment lines back above "Native tests on a device";
│                                # BUILD= set, network note, loop stops at the first failing test
└── src/main/cpp/rendering/vulkan/
    ├── cull/VulkanVisibilityPass.cpp   # kSetFamilies, kPoolSizeCount replace the bare 2s
    ├── frame/VulkanFrameCompute.h      # submission_ is one line, comment above it
    ├── layout/VulkanFrameSlots.h       # first sentence only
    └── VulkanSplatRenderer.cpp         # comment names the wait adoptWorld relies on
CONTRIBUTING.md, scripts/lint-cpp.sh    # the link step adds undefined references, not source paths
tests/.../VulkanRadixSortTest.cpp       # keyBitCount(input.keyBits)

World upload waits, from the code:

render(): uploadFinishedLoads()        # render thread, start of every frame, before draw
  uploadWorld / uploadLodWorld / createSlab
    waitIdle()                         # entry: the old world has no work in flight
    VulkanFrameCompute::create         # no queue submit of its own
    VulkanWorld::upload                # VulkanBuffer::upload: submit + vkWaitForFences
    waitIdle()                         # uploadWorld and createSlab only
    adoptWorld                         # replaces world_ and compute_

Nothing draws between the entry wait and adoptWorld (same thread), and the uploads in between are fenced, so the entry wait covers the replaced world.
The second wait in uploadWorld and createSlab is redundant, and uploadLodWorld already runs without it.
GPU synchronisation is unchanged here; the comment now says which wait is relied on, and the "may still be in flight" remarks on the second waits are gone because they said otherwise.

Evidence

  • Before: the findings in final-fix-1 and final-fix-2 reviews (Minors 1-4 and 1-5).
    After: each is fixed; scripts/lint-cpp.sh exit 0, the Gradle release build and unit tests exit 0.
  • Where it ran: the Mac host only, NDK 27.1.12297006 arm64 builds. No physical device was attached and no device run has happened; nothing here changes GPU commands, so device runs stay with the end session.
  • Checks:
    • scripts/lint-cpp.sh: clang-format and clang-tidy clean, and all seven splatkit_vulkan_*_test link for arm64 through splatkit_android_tests.
    • ./gradlew :splatkit:assembleRelease :app:assembleRelease :splatkit:testDebugUnitTest --no-daemon: BUILD SUCCESSFUL.
    • Host WorldFrameCompletionTest compiled with the workflow's flags: exit 0.
    • The nine generated/shaders/*.spv hashes equal the recorded baseline.
    • The README loop checked with a stub adb that fails the second test: it stops there and exits 1.
    • Skipped: npm run check, npm run lint and sdk_harness.py (no TypeScript or engine change); the validation layer (no device).

Merge Danger

Door: two-way.

Comments, constant names, docs and a test printout only.

Blast Radius: negligible

The two new constants keep the values 2 and 2 the pool already used.

@Xget7
Xget7 merged commit ab16afb into main Oct 20, 2026
22 checks passed
@Xget7
Xget7 deleted the followup-android branch October 20, 2026 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant