diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 97a31c1..f4cd09b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -76,7 +76,7 @@ scripts/lint-cpp.sh --fix # rewrite the formatting and apply the fixes clang- Both tools come from the NDK the project pins, so a machine that builds the library can lint it; `.clang-format` and `.clang-tidy` at the root hold the rules. The script configures `splat-core`, `splatkit-engine` and `splatkit-android` for the Android target and lints tests and tools too. -It also links the Android native tests, so a wrong source path or an undefined reference fails CI; [running them on a device](packages/splatkit-android/README.md#native-tests-on-a-device) stays manual. +It also links the Android native tests, so an undefined reference fails CI; [running them on a device](packages/splatkit-android/README.md#native-tests-on-a-device) stays manual. `git config core.hooksPath .githooks` installs a pre-push hook that runs `scripts/lint-cpp.sh --format-only`, the seconds-long formatting half of that check. VS Code's C/C++ extension reads the compile databases it leaves in `build/lint`, as [.vscode/settings.json](.vscode/settings.json) sets up. clang-tidy holds every function to its default cognitive complexity of 25, counting the source as written, so test assertion macros add nothing. diff --git a/packages/splatkit-android/README.md b/packages/splatkit-android/README.md index e57c0f6..238ffa3 100644 --- a/packages/splatkit-android/README.md +++ b/packages/splatkit-android/README.md @@ -113,25 +113,29 @@ cd apps/android-dev ./gradlew :splatkit:assembleRelease :splatkit:testDebugUnitTest ``` +Tests and emulator evidence do not replace physical Adreno/Mali validation or reference-image acceptance. +Source builds enable 16 KB ELF alignment; CI checks every packaged shared library and APK ZIP alignment with `scripts/check_android_alignment.py`. +Artifact alignment checks do not replace execution on a 16 KB device. + ## Native tests on a device The `splatkit_vulkan_*_test` executables need a real Vulkan device and run without a surface. `scripts/lint-cpp.sh` links all of them in CI; running them is a manual device step. Build them outside the checkout with the NDK toolchain flags from that script plus `-DSPLATKIT_ANDROID_BUILD_TESTS=ON`. +The first configure fetches dependencies over the network. ```sh +BUILD=~/.rn-build-external/android-tests cmake -S packages/splatkit-android/src/main/cpp -B "$BUILD" \ -DCMAKE_TOOLCHAIN_FILE="$ANDROID_NDK_HOME/build/cmake/android.toolchain.cmake" \ -DANDROID_ABI=arm64-v8a -DANDROID_PLATFORM=android-29 -DANDROID_STL=c++_static \ -DSPLATKIT_ANDROID_BUILD_TESTS=ON cmake --build "$BUILD" --target splatkit_android_tests -for t in "$BUILD"/tests/splatkit_vulkan_*_test; do - adb push "$t" /data/local/tmp/ && adb shell "/data/local/tmp/$(basename "$t")" -done +(set -e; for t in "$BUILD"/tests/splatkit_vulkan_*_test; do + adb push "$t" /data/local/tmp/ + adb shell "/data/local/tmp/$(basename "$t")" +done) ``` Each test exits 0 on success; `splatkit_vulkan_buffer_test` and `splatkit_vulkan_upload_pressure_test` accept `--small` for a device with little memory, which skips their large cases. -Tests and emulator evidence do not replace physical Adreno/Mali validation or reference-image acceptance. -Source builds enable 16 KB ELF alignment; CI checks every packaged shared library and APK ZIP alignment with `scripts/check_android_alignment.py`. -Artifact alignment checks do not replace execution on a 16 KB device. diff --git a/packages/splatkit-android/src/main/cpp/rendering/vulkan/VulkanSplatRenderer.cpp b/packages/splatkit-android/src/main/cpp/rendering/vulkan/VulkanSplatRenderer.cpp index 4c44216..215020d 100644 --- a/packages/splatkit-android/src/main/cpp/rendering/vulkan/VulkanSplatRenderer.cpp +++ b/packages/splatkit-android/src/main/cpp/rendering/vulkan/VulkanSplatRenderer.cpp @@ -117,7 +117,9 @@ std::optional VulkanSplatRenderer::world() const { return GpuWorldInfo{world_->count, world_->shDegree}; } -// Callers have waited for the GPU to finish with the world being replaced. +// Relies on the waitIdle each caller makes on entry: uploads run on the render thread between +// draws, and the copies they make are fenced, so nothing is in flight on the world replaced here. +// The second waitIdle in uploadWorld and createSlab is not needed for that. void VulkanSplatRenderer::adoptWorld(std::unique_ptr world, std::unique_ptr compute) { world_ = std::move(world); @@ -139,7 +141,7 @@ bool VulkanSplatRenderer::uploadWorld(const splat::SplatCloud& cloud, int maxShD } auto world = VulkanWorld::upload(ctx_, cloud, maxShDegree, false); if (!world) return false; - ctx_.waitIdle(); // the previous world may still be in flight + ctx_.waitIdle(); adoptWorld(std::move(world), std::move(compute.value())); return true; } @@ -180,7 +182,7 @@ bool VulkanSplatRenderer::createSlab(uint32_t capacity, int shDegree) { } auto world = VulkanWorld::createSlab(ctx_, capacity, shDegree, false); if (!world) return false; - ctx_.waitIdle(); // the previous world may still be in flight + ctx_.waitIdle(); adoptWorld(std::move(world), std::move(compute.value())); return true; } diff --git a/packages/splatkit-android/src/main/cpp/rendering/vulkan/cull/VulkanVisibilityPass.cpp b/packages/splatkit-android/src/main/cpp/rendering/vulkan/cull/VulkanVisibilityPass.cpp index 5a946c9..ff5b9e1 100644 --- a/packages/splatkit-android/src/main/cpp/rendering/vulkan/cull/VulkanVisibilityPass.cpp +++ b/packages/splatkit-android/src/main/cpp/rendering/vulkan/cull/VulkanVisibilityPass.cpp @@ -28,6 +28,10 @@ constexpr VkSubgroupFeatureFlags kRequiredSubgroupOperations = VK_SUBGROUP_FEATU constexpr uint32_t kVisibilityStorageBindings = 7; constexpr uint32_t kVisibilityBindings = 1 + kVisibilityStorageBindings; constexpr uint32_t kPrepareBindings = 3; +// Each frame slot owns one visibility set and one prepare set. +constexpr uint32_t kSetFamilies = 2; +// The descriptor pool holds uniform buffers and storage buffers. +constexpr uint32_t kPoolSizeCount = 2; // The bits of the shader's keyMode push constant. constexpr uint32_t kKeyLow16 = 1; constexpr uint32_t kKeyDescending = 2; @@ -254,13 +258,13 @@ bool VulkanVisibilityPass::createDescriptors() { if (vkCreateDescriptorSetLayout(device, &prepareInfo, nullptr, &prepareSetLayout_) != VK_SUCCESS) return false; - VkDescriptorPoolSize sizes[2]{}; + VkDescriptorPoolSize sizes[kPoolSizeCount]{}; sizes[0] = {VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER, kFramesInFlight}; sizes[1] = {VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, (kVisibilityStorageBindings + kPrepareBindings) * kFramesInFlight}; VkDescriptorPoolCreateInfo poolInfo{VK_STRUCTURE_TYPE_DESCRIPTOR_POOL_CREATE_INFO}; - poolInfo.maxSets = 2 * kFramesInFlight; - poolInfo.poolSizeCount = 2; + poolInfo.maxSets = kSetFamilies * kFramesInFlight; + poolInfo.poolSizeCount = kPoolSizeCount; poolInfo.pPoolSizes = sizes; if (vkCreateDescriptorPool(device, &poolInfo, nullptr, &descriptorPool_) != VK_SUCCESS) return false; diff --git a/packages/splatkit-android/src/main/cpp/rendering/vulkan/frame/VulkanFrameCompute.h b/packages/splatkit-android/src/main/cpp/rendering/vulkan/frame/VulkanFrameCompute.h index ec4781f..ae75940 100644 --- a/packages/splatkit-android/src/main/cpp/rendering/vulkan/frame/VulkanFrameCompute.h +++ b/packages/splatkit-android/src/main/cpp/rendering/vulkan/frame/VulkanFrameCompute.h @@ -89,8 +89,8 @@ class VulkanFrameCompute { std::array, kFramesInFlight> ranges_, readback_; std::array queries_{}; std::array pending_{}; - std::array - submission_{}; // of the pending encode; 0 when not submitted + // Per slot, the submission of the pending encode; 0 when not submitted. + std::array submission_{}; uint64_t collectedSubmission_ = 0; std::vector rangeRecords_; Stats stats_; diff --git a/packages/splatkit-android/src/main/cpp/rendering/vulkan/layout/VulkanFrameSlots.h b/packages/splatkit-android/src/main/cpp/rendering/vulkan/layout/VulkanFrameSlots.h index 1e05fb4..88cdfd1 100644 --- a/packages/splatkit-android/src/main/cpp/rendering/vulkan/layout/VulkanFrameSlots.h +++ b/packages/splatkit-android/src/main/cpp/rendering/vulkan/layout/VulkanFrameSlots.h @@ -4,8 +4,7 @@ namespace splatkit { -// Frames the CPU records while the GPU still draws earlier ones. Every per-frame slot array -// and descriptor pool in the Vulkan renderer is sized from this one count. +// Frames the CPU records while the GPU still draws earlier ones. inline constexpr uint32_t kFramesInFlight = 2; } // namespace splatkit diff --git a/packages/splatkit-android/tests/rendering/vulkan/sort/VulkanRadixSortTest.cpp b/packages/splatkit-android/tests/rendering/vulkan/sort/VulkanRadixSortTest.cpp index a6d2ac9..7958587 100644 --- a/packages/splatkit-android/tests/rendering/vulkan/sort/VulkanRadixSortTest.cpp +++ b/packages/splatkit-android/tests/rendering/vulkan/sort/VulkanRadixSortTest.cpp @@ -9,6 +9,7 @@ #include "rendering/vulkan/sort/VulkanRadixSort.h" #include "support/VulkanTestContext.h" +using splatkit::keyBitCount; using splatkit::KeyBits; using splatkit::kFramesInFlight; using splatkit::VulkanBuffer; @@ -159,8 +160,8 @@ void runCase(VulkanTestContext& gpu, const VulkanRadixSort& sort, const InputBuf const VulkanRadixSort::Input& input, uint32_t n, uint32_t pattern, std::mt19937& random, uint32_t& cases) { const bool low16 = input.keyBits == KeyBits::low16; - std::printf("radix case=%u bits=%u n=%u pattern=%u stage=upload\n", cases, low16 ? 16u : 32u, n, - pattern); + std::printf("radix case=%u bits=%u n=%u pattern=%u stage=upload\n", cases, + keyBitCount(input.keyBits), n, pattern); const auto inKeys = makeKeys(n, pattern, low16, random); std::vector inValues(n); std::vector expected(n); diff --git a/scripts/lint-cpp.sh b/scripts/lint-cpp.sh index 540bb40..2583742 100755 --- a/scripts/lint-cpp.sh +++ b/scripts/lint-cpp.sh @@ -91,8 +91,8 @@ cmake -S "$android" -B "$build/android" "${toolchain[@]}" \ -DSPLATKIT_ANDROID_BUILD_TESTS=ON > /dev/null # The splat pipeline includes the generated shader headers. cmake --build "$build/android" --target splatkit_shaders_generate > /dev/null -# Linking the device tests catches a wrong source path or an undefined reference that -# clang-tidy, which only parses, would let through to a phone. +# Linking the device tests catches an undefined reference that clang-tidy, which only parses, +# would let through to a phone. echo "link splatkit-android tests" if ! tests_log="$(cmake --build "$build/android" --target splatkit_android_tests 2>&1)"; then printf '%s\n' "$tests_log" >&2