perf(geometry): int32 vtkOriginalPointIds + width-agnostic hardware-selection#79
Merged
Conversation
…tic selection
vtkGeometryFilter::PassPointIds now stores the vtkOriginalPointIds passthrough
array in an int32 container when every input point id fits in 0x7FFFFFFF (else
int64), halving its footprint on large extracted surfaces. Values are sacred;
only the container width changes.
This is only safe because the render hardware-selection path reads point/cell id
passthrough arrays width-agnostically: vtkOpenGLPolyDataMapper (and the Batched,
LowMemory, and LowMemoryBatched variants) previously did
vtkArrayDownCast<vtkIdTypeArray>(...) which returns NULL on an int32 array,
silently skipping the picked-id remap. They now fetch the array as vtkDataArray
and read values via GetComponent (exact for ids < 2^53, identical to the former
vtkIdType GetValue), including the UpdateMaximumPointId/CellId id-bit-sizing
paths.
Validation:
* bitexact: op_geometry / op_geometry_ugrid / op_geometry_ugrid_mixed now
enable PassThroughPointIdsOn, so the int32 vtkOriginalPointIds array is
compared against stock's int64 (width-normalized -> values must match).
* NEW selection gate (tests/renderexact/run_select.py + compare_select.py,
wired into ci/run-renderexact.sh): renders the surface of a hex lattice with
the mapper remapping picked point ids through vtkOriginalPointIds, runs a
full-viewport vtkHardwareSelector POINT selection, and asserts fvtk's
selected original-id set equals stock's. The lattice makes original ids a
non-identity map of surface ids, so a dropped int32 remap would change the
result -- the gate is discriminating.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
a34dc2a to
8203d5b
Compare
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
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.
Summary
Two coupled changes that make the width-relaxed int32 rule safe for named public id arrays (not just
vtkCellArrayconnectivity):Render hardware-selection is now width-agnostic. The GPU picking path remaps a picked pixel's raw VTK point/cell id to the value in a passthrough id array. It previously did
vtkArrayDownCast<vtkIdTypeArray>(...), which returns null on an int32 array → the remap was silently skipped → wrong picked ids. All four mappers (vtkOpenGLPolyDataMapper, plus the Batched, LowMemory, and LowMemoryBatched variants) now fetch the array asvtkDataArrayand read values viaGetComponent(exact for ids < 2^53, identical to the formervtkIdType GetValue), including theUpdateMaximumPointId/CellIdid-bit-sizing paths.vtkGeometryFilter::PassPointIdsstoresvtkOriginalPointIdsas int32-when-fits (templatedPassPointIdsFill), halving its footprint on large extracted surfaces. Values are sacred; only the container width narrows.New validation: a hardware-selection (picking) gate
Neither the bitexact nor the pixel-exact gate exercises selection, so this PR adds one (
tests/renderexact/run_select.py+compare_select.py, wired intoci/run-renderexact.shunder the same software-EGL driver). It renders the surface of a hex lattice with the mapper remapping picked point ids throughvtkOriginalPointIds, runs a full-viewportvtkHardwareSelectorPOINT selection, and asserts fvtk's selected original-id set equals stock's. The lattice makes original ids a non-identity map of surface ids, so a dropped int32 remap would change the result — the gate is discriminating.Validation (executor build + manylinux_2_28 container)
op_geometry/op_geometry_ugrid/op_geometry_ugrid_mixednow enablePassThroughPointIdsOn, so the int32vtkOriginalPointIdsarray is compared against stock's int64 (width-normalized → values match).surface_pointpick: 54 selected ids match (stock dtype=int64, fvtk dtype=int32).Dependency / merge ordering
Carries the width-relaxed integer comparison in
tests/bitexact/compare.py(compare integer arrays by value cast to int64). This is identical in intent to the change in thevtkCellArray-int32 PR (#76) — land #76 first and this hunk de-dupes on rebase. Without it, the int32-vs-int64 dtype mismatch reds the suite.Follow-ups (not in this PR)
PassCellIdsint32 (template theCompositeCellIdsOrigIds pointer + add cell-picking to the gate),vtkDataSetSurfaceFilter(10 sites),vtkStripper.🤖 Generated with Claude Code