Skip to content

feat: Add support for no_std - #166

Merged
svenstaro merged 11 commits into
svenstaro:masterfrom
oschijns:no_std
Nov 16, 2025
Merged

feat: Add support for no_std#166
svenstaro merged 11 commits into
svenstaro:masterfrom
oschijns:no_std

Conversation

@oschijns

@oschijns oschijns commented Aug 3, 2025

Copy link
Copy Markdown
Contributor

Hello, I made a first draft for adding no_std support for this crate.
I needed to add a few dependencies to replace some std types with no_std equivalents.
I tried to find a way to replace the thread_local!{BUCKETS} with something but I am unsure if what I did makes sense...

Comment thread Cargo.toml Outdated
Comment thread src/bvh/bvh_node.rs Outdated
Comment thread src/bvh/bvh_node.rs Outdated

@finnbear finnbear left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the PR! I think no_std would be great for bvh :)

@oschijns
oschijns marked this pull request as draft August 4, 2025 07:54
Comment thread src/bvh/bvh_node.rs Outdated
@codecov

codecov Bot commented Sep 4, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 46.15385% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.69%. Comparing base (8005e04) to head (87848f0).
⚠️ Report is 12 commits behind head on master.

Files with missing lines Patch % Lines
src/flat_bvh.rs 0.00% 5 Missing ⚠️
src/bvh/bucket.rs 71.42% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #166      +/-   ##
==========================================
- Coverage   49.89%   49.69%   -0.21%     
==========================================
  Files          17       18       +1     
  Lines        1471     1479       +8     
==========================================
+ Hits          734      735       +1     
- Misses        737      744       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@oschijns

oschijns commented Sep 5, 2025

Copy link
Copy Markdown
Contributor Author

I didn't check the CI, I will submit fixes this evening.

@oschijns
oschijns marked this pull request as ready for review September 5, 2025 19:03

@finnbear finnbear left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you! Apologies for taking longer than I said I would take for review.

Comment thread Cargo.toml Outdated
Comment thread src/aabb/aabb_impl.rs
Comment thread src/bounding_hierarchy.rs
Comment thread src/bvh/bvh_impl.rs
Comment thread src/lib.rs Outdated
Comment thread src/ray/ray_impl.rs
Comment thread src/testbase.rs
@finnbear

finnbear commented Sep 21, 2025

Copy link
Copy Markdown
Collaborator

Code changes look great! Here's what remains, in my opinion:

  • Fixing whatever clippy found ;) (but don't worry about code coverage)
  • Adding a CI step to test with no_std, possibly like the simd feature's CI step
  • Rebasing into a few, meaningful, logical commits. There should be a bias towards avoiding commits that undo previous commits (e.g. the temporary inclusion of hashbrown shouldn't be reflected in the final commits). It seems easiest, for this PR, to rebase all code changes into a single commit. Once done, request svenstaro's review.
  • Writing a changelog entry, which can be a separate commit.

Thanks!

@oschijns

Copy link
Copy Markdown
Contributor Author

Hello, I added a CI for testing builds without the std feature.
I would suspect that I don't need the matrix.os list since no-std imply I don't use OS's features.

@oschijns

oschijns commented Nov 2, 2025

Copy link
Copy Markdown
Contributor Author

Since Rust 1.80 has been released more than a year ago, I guess it is fine to bump the version? It fixes the dependency resolution.

@svenstaro

Copy link
Copy Markdown
Owner

I'd be fine with that.

@svenstaro

Copy link
Copy Markdown
Owner

@finnbear do you wanna merge this?

@finnbear

finnbear commented Nov 11, 2025

Copy link
Copy Markdown
Collaborator

@svenstaro I'm in favor of merging as-is, yes :)

I see user-facing benefit from the diff, and the only user-facing risk I see is that a higher MSRV could force a rustc upgrade (but raising it is consistent with our stated policy). Diff code style and commit history looks good (not perfect but not something I have time to fix or enumerate as review comments at the moment).

@svenstaro

Copy link
Copy Markdown
Owner

I would like to see a mention of this new feature in the README so that users are aware it exists.

@oschijns

Copy link
Copy Markdown
Contributor Author

Ok, I'll add it.

Comment thread README.md Outdated
@svenstaro

Copy link
Copy Markdown
Owner

Sorry about the conflicts. Should be easy to fix though.

@oschijns
oschijns force-pushed the no_std branch 2 times, most recently from 3cee6f5 to adf1e30 Compare November 14, 2025 18:23
@oschijns

Copy link
Copy Markdown
Contributor Author

Sorry about the conflicts. Should be easy to fix though.

No problem.

However there may still be issues with clippy this time

cargo clippy --all-targets --all-features -- -D warnings
    Checking bvh v0.11.0 (/media/oschijns/Projects/rust/bvh)
error[E0554]: `#![feature]` may not be used on the stable release channel
  --> src/lib.rs:81:32
   |
81 | #![cfg_attr(feature = "bench", feature(test))]
   |                                ^^^^^^^^^^^^^

error[E0554]: `#![feature]` may not be used on the stable release channel
  --> src/lib.rs:82:31
   |
82 | #![cfg_attr(feature = "simd", feature(min_specialization))]
   |                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^

For more information about this error, try `rustc --explain E0554`.
error: could not compile `bvh` (lib) due to 2 previous errors
warning: build failed, waiting for other jobs to finish...
error: could not compile `bvh` (lib test) due to 2 previous errors

@svenstaro svenstaro left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Almost there! Just tiny fixes.

Comment thread CHANGELOG.md Outdated
Comment thread README.md Outdated
@svenstaro

Copy link
Copy Markdown
Owner

LGTM! @finnbear any last comments?

@finnbear

Copy link
Copy Markdown
Collaborator

Reread the diff; LGTM too 👍

@svenstaro
svenstaro merged commit cd597c5 into svenstaro:master Nov 16, 2025
20 of 21 checks passed
@svenstaro

svenstaro commented Nov 16, 2025

Copy link
Copy Markdown
Owner

Cool! Thanks @oschijns

@oschijns

Copy link
Copy Markdown
Contributor Author

Thank you for the crate and for the merge!
:)

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