Consistent constructor naming of Asset3D across C++ and Rust

[abhishek47kashyap]

Related

Addresses #6881.

What

Deprecates existing constructor naming of Asset3D in favor of new names.

• C++: from_file() to from_file_path() and from_bytes() to from_file_contents()
• Rust: from_file() to from_file_path()

Passes

• pixi run -e cpp cpp-test

Fails:

• cargo test --all-targets --all-features: stems from re_chunk/tests/memory_test.rs and fails on the main branch as well

failures:
    concat_single_is_noop
    filter_does_allocate
    filter_empty_or_full_is_noop
    take_does_not_allocate
    take_empty_or_full_is_noop

• pixi run fast-lint: There are some warnings but the main error is from asset3d.hpp : out of sync which I'm not entirely sure how to fix

[abey79]

Thanks for the contribution!

Let's start with the more obvious/simple stuff:

• If you haven't done so, you'll need to install pixi and set it to version 0.41.4 using pixi self-update --version 0.41.4
• When changing things in the SDKs, you always need to run the codegen to update auto-generated files. This is done with pixi run codegen.
• You need to auto-format C++ code using pixi run cpp-fmt (when asked, use the cpp environment).
• Please add a note in migration-0-23.md about the deprecated methods.

[abhishek47kashyap]

Thanks for the feedback.

• My pixi version is 0.41.4.
• On running pixi run codegen, it said Returning early: no changes detected (and --force wasn't set), so I decided to set the force flag to observe the difference and found that my changes to asset3d.hpp (which this PR introduces) were reversed. Does this mean I modified the generated code but not the generating/"source" code?
• I ran pixi run cpp-fmt and the changes are in commit 5242f67. However pixi run fast-lint still fails with asset3d.hpp : out of sync.
• Have updated migration-0-23.md (had to resolve a merge conflict because my fork had fallen behind).

[abhishek47kashyap]

@abey79 thought I'd ping on how best to move this PR forward 🙂

[abey79]

Sorry for the delay. Your latest changes look good, thanks! I've updated to latest main and run codger (make sure to pull these commits!).

Could you also update all snippets (in docs/snippets/all) and examples (in examples/cpp and examples/rust) to use the new API?

[abey79]

Also, apparently there are build errors. You might want to run pixi run -e cpp cpp-build-all and fix any error.

[abhishek47kashyap]

Thanks for getting back to me. Yeah sure I'll get to the snippets and examples.

I'm a little unsure why is codegen undoing the following change to C++ API (from the PR description at the very top):

C++: from_file() to from_file_path() and from_bytes() to from_file_contents()

The build errors stem from the absence of from_file_path() which is getting called inside from_file(), the latter being the constructor we want to deprecate through this PR. Since codegen removes from_file_path(), the implementation of from_file() is no longer correct and pixi run -e cpp cpp-build-all fails.

Perhaps I'm making an incorrect assumption about the purpose/effect of codegen? My understanding is that I need to modify both C++ and Rust constructors of Asset3D, but codegen undoes my changes to the C++ constructor, so I'm confused how to run codegen and keep the changes to C++.

[abey79]

The way to get things in asset3d.hpp is via asset3d_ext.cpp and the CODEGEN_COPY_TO_HEADER markers. You add stuff there and then run pixi run codegen so it shows up in the actual .hpp file.

[abhishek47kashyap]

Thanks for the explanation, I hadn't made all the changes to asset3d_ext.cpp which was causing asset3d.hpp : out of sync. That's resolved now and there are no build errors from pixi run -e cpp cpp-build-all.

docs/snippets was already updated in ctor renaming and I think I got them all. Search returns empty for Asset3D in examples so didn't see any changes to be made there.

[emilk]

Nice; thank you!

[abey79]

thanks!

[emilk]

CI failures are unrelated.