Use C++11 random number generator for fuzzy skin #5682
Merged
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.
Description
Fixes #5521
Fixes bambulab/BambuStudio#4253
This is an alternate, probably better choice, to #5659. The root cause analysis is the same so you can read that PR description for more info.
The current implementation in this function is pretty flawed. In runtimes that use thread-local RNG state (Windows), you get problems like #5521 where each thread produces the same output, leading to obvious artifacts whenever a series of layers have identical geometry. It's awkward to get a per-thread initial seed without a true entropy source, as #5659 shows. I also don't like how that PR is technically capable of calling
srand
in non-thread-local runtimes through multiple threads.In runtimes that don't use thread-local RNG state, the situation is arguably worse.
rand
andsrand
are not thread-safe, so there's possible state corruption -- I don't have any evidence, but we do have reports of AVs specific to fuzzy skin, and this could be a root cause. These functions get called a lot here and any edge case is going to get hit sooner or later.Since don't really know in-code whether our runtime has thread-local state or not, it's not safe to call
srand
(might corrupt state in global-state runtimes), and it's not safe to not callsrand
(might get layer patterning in thread-local runtimes). The STL primitives here are an easy solution since they're instanced and we can simply make one instance per thread.Note that
std::random_device
is allowed to "generate the same number sequence" if a hardware entropy source is not available. In that case.entropy()
returns0
and we'll fall back to the thread ID hash method instead.Screenshots/Recordings/Graphs
See images in #5659 for the defect cases.
To compare the old PRNG with the new one, here's a Stanford bunny, cylinder, cube, and sphere, with the default fuzzy skin setting applied. Apologies for not getting the camera lined up exactly the same in both shots but it's reasonably easy to tell that there's not an observable difference.
Current behavior in Linux (

main
branch)With this PR

Tests
Tested on Linux