Skip to content

Fix Training Crop Box Pruning#1365

Open
kmeirlaen wants to merge 2 commits into
MrNeRF:masterfrom
kmeirlaen:fix-cropboxtraining
Open

Fix Training Crop Box Pruning#1365
kmeirlaen wants to merge 2 commits into
MrNeRF:masterfrom
kmeirlaen:fix-cropboxtraining

Conversation

@kmeirlaen

@kmeirlaen kmeirlaen commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR fixes live training crop-box pruning for transformed crop boxes and prevents intermittent Vulkan viewport failures while crop pruning changes model topology.

The crop-box mask logic has been extracted from trainer.cpp into a shared training helper so the trainer and regression tests exercise the same implementation.

Problem

Live training supports an enabled crop box attached to the training model and continuously prunes gaussians that fall on the removed side of that box. That pruning path was using the crop box node's world transform directly as the transform for the inside/outside test.

Scene node transforms are box-to-world transforms. The crop test needs the opposite direction: world-to-box. When the crop box was translated, rotated, or scaled, training evaluated gaussian positions in the wrong coordinate frame. The practical result was that valid splats inside the visible crop box could be removed while splats outside the crop boundary survived. In one test with a fitted/trimmed crop box, this could remove all initial gaussians and fail training with n_primitives is 0 - model has no gaussians.

Fixing the crop transform exposed a second issue. Once the crop mask was correct, live training could prune small numbers of gaussians repeatedly during ordinary non-refinement iterations. remove_gaussians() compacts model tensors, and the GUI/Vulkan training path then repairs those tensors back into Vulkan-external storage. The trainer only excluded the renderer during strategy refinement, so a viewport frame could sometimes observe the model after compaction but before Vulkan-external storage was restored.

When that happened, VkSplat intentionally refused the unsafe full input-copy fallback and logged errors such as:

VkSplat refusing full input-copy fallback; model tensors must use Vulkan-external storage

The user-facing symptom was an intermittent black viewport during training. Moving the camera or otherwise triggering a later redraw could make the splats appear again after the storage repair completed.

Reproduction Steps

Wrong crop pruning

  1. Load a dataset or checkpoint with a training model and initial point cloud.
  2. Add or enable a crop box on the training model.
  3. Move, rotate, scale, or use Fit to Scene (Trimmed) so the crop box transform is not identitcal.
  4. Start training with the crop box enabled.
  5. Observe that pruning can affect the wrong side of the crop box:
    • splats visibly inside the crop box may be removed
    • splats outside the crop box perimeter may survive
    • in severe cases training can fail immediately with n_primitives is 0 - model has no gaussians

Vulkan black viewport during crop pruning

Note: this could only be replicated if first problem is resolved, so without the PR this is not possible to simulate, and with PR this is resolved)

  1. Use a crop box that removes only a small number of gaussians during live training.

  2. Start training with the Vulkan viewport active.

  3. Watch the training log while crop pruning runs repeatedly.

  4. Before this fix, logs could show MRNF::remove_gaussians followed by:

    VkSplat refusing full input-copy fallback; model tensors must use Vulkan-external storage
    
  5. The viewport could temporarily go black even though training continued.

  6. Moving the camera or otherwise forcing a later redraw could make the splats appear again.

What Changed

  • Fixed transformed crop-box pruning during live training.
    • The old training path passed the crop box world transform as if it were a world-to-box transform.
    • The new helper uses glm::inverse(scene.getWorldTransform(cropbox_id)), matching the world-to-box convention used by crop-box filtering.
  • Preserved crop semantics.
    • Normal crop boxes remove splats outside the box.
    • Inverse crop boxes remove splats inside the box.
    • Already soft-deleted splats are not returned for removal again.
  • Extracted crop mask computation into src/training/training_cropbox_mask.*.
    • compute_training_cropbox_remove_mask(...) is still production code used by the trainer.
    • compute_cropbox_remove_mask(...) covers the pure tensor/math portion for direct testing.
  • Fixed a Vulkan render race during crop pruning.
    • Crop pruning can compact/reallocate model tensors even on non-refinement iterations.
    • The trainer now takes the renderer exclusion lock before crop-prune topology changes and holds it through scene topology sync and Vulkan-external tensor repair.
    • This prevents the viewport from seeing an intermediate non-Vulkan-external tensor layout.
  • Added focused C++ regression coverage.
    • Translated crop boxes.
    • Normal and inverse crop masks.
    • Soft-deleted splats.
    • Nullopt/no-op cases.
    • Both return contracts and observable side-effect contracts are checked.

Files

  • src/training/trainer.cpp
  • src/training/training_cropbox_mask.hpp
  • src/training/training_cropbox_mask.cpp
  • tests/test_training_cropbox_mask.cpp
  • src/training/CMakeLists.txt
  • tests/CMakeLists.txt

Testing

Manual Validation

  • Start training with an enabled translated or fitted crop box.
  • Confirm normal crop pruning keeps splats inside the crop box and prunes outside splats.
  • Confirm inverse crop pruning flips the mask.
  • Confirm the viewport does not intermittently go black during repeated small crop-prune topology changes.
  • Confirm logs no longer show VkSplat refusing full input-copy fallback during crop-prune iterations.

Risk

  • The crop-box helper is now shared production code instead of an anonymous helper inside trainer.cpp.
  • The trainer takes the renderer exclusion lock when crop pruning actually removes splats, which may briefly block rendering during topology repair, but avoids exposing inconsistent tensor storage to Vulkan.

Copilot AI review requested due to automatic review settings July 3, 2026 07:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes live training crop-box pruning when the crop box is transformed (by using a world-to-box transform) and prevents intermittent Vulkan viewport failures during crop-driven topology compaction by extending renderer exclusion to cover crop-prune tensor reallocation and subsequent storage repair.

Changes:

  • Extracted crop-box removal mask computation into src/training/training_cropbox_mask.* and updated the trainer to use the shared helper.
  • Corrected crop-box transform usage for pruning by applying glm::inverse(scene.getWorldTransform(cropbox_id)) (world-to-cropbox).
  • Added focused regression tests for translated crop boxes, inverse masks, soft-deleted splats, and nullopt/no-op cases; wired the new test into the tests CMake list.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/training/trainer.cpp Uses shared crop-box mask helper and expands renderer exclusion locking to cover crop-prune topology changes and Vulkan-external storage repair.
src/training/training_cropbox_mask.hpp Declares shared crop-box remove-mask helpers for production and test use.
src/training/training_cropbox_mask.cpp Implements the crop-box remove-mask logic, including correct world-to-cropbox transform handling.
tests/test_training_cropbox_mask.cpp Adds regression coverage for transformed crop pruning semantics and scene/model non-mutation contracts.
src/training/CMakeLists.txt Adds the new helper implementation file to the training sources.
tests/CMakeLists.txt Adds the new regression test to the test target sources.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/training/training_cropbox_mask.cpp
@kmeirlaen kmeirlaen marked this pull request as ready for review July 5, 2026 14:01
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.

2 participants