Skip to content

NanoVDB: default Mask's copy constructor - #2310

Merged
kmuseth merged 1 commit into
AcademySoftwareFoundation:masterfrom
sifakis:fix/mask-defaulted-copy-ctor
Sep 3, 2026
Merged

NanoVDB: default Mask's copy constructor#2310
kmuseth merged 1 commit into
AcademySoftwareFoundation:masterfrom
sifakis:fix/mask-defaulted-copy-ctor

Conversation

@sifakis

@sifakis sifakis commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Mask's only data member is uint64_t mWords[WORD_COUNT], and the hand-written copy
constructor's loop copies exactly that range — so it duplicates the implicit memberwise copy.
Defaulting it leaves the semantics unchanged and makes Mask trivially copyable. sizeof and
standard-layout are unaffected.

Note:

  • The copy assignment operator immediately below is already = default; this just finishes
    the pair.
  • nanovdb::cuda::Buffer<T, R> requires std::is_trivially_copyable<T>, so Mask currently
    cannot be a buffer element type. With this change, mask sidecars can be held in a typed
    Buffer<Mask<N>> rather than a Buffer<std::byte> plus a cast.

Verification

  • is_trivially_copyable becomes true for Mask<3>/Mask<4>/Mask<5>; sizeof (64/512/4096)
    and is_standard_layout are unchanged.
  • Copy-construction and assignment still work in device code (the defaulted special member is
    implicitly __host__ __device__, as the already-defaulted operator= beside it relies on).

Mask's only data member is uint64_t mWords[WORD_COUNT], and the
hand-written copy constructor's loop copies exactly that range, so it
duplicates the implicit memberwise copy. Defaulting it leaves the
semantics unchanged and makes Mask trivially copyable, matching the copy
assignment operator defaulted immediately below it.

Signed-off-by: Efty Sifakis <esifakis@nvidia.com>
@sifakis
sifakis marked this pull request as ready for review September 2, 2026 22:26
@sifakis
sifakis requested a review from kmuseth as a code owner September 2, 2026 22:26

@harrism harrism 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.

This is a nice simplification, and it enables replacing some Buffer<std::byte> + cast with Buffer<Mask>. After #2301 merges, I will do a cleanup PR to make those changes.

@kmuseth kmuseth 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.

looks good

@kmuseth
kmuseth merged commit 74ea76a into AcademySoftwareFoundation:master Sep 3, 2026
20 checks passed
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.

3 participants