feat: add fast random float gen based on pcg#1158
Conversation
WalkthroughThe PCG random number generator function was refactored to update and persist its state externally via an Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Random
Caller->>Random: f1/f2/f3/f4(inout seed [, out word])
Random->>Random: pcg(inout seed)
Random-->>Caller: float/float2/float3/float4 (random values), [word]
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
package/Shaders/Common/Random.hlsli (2)
170-175: Mask mantissa for the second component for consistency & clarity
bits1is built without the mantissa mask:uint bits1 = word >> 9 | 0x3F800000u;While
word >> 9is only 23 bits wide, masking expresses the intent and protects against accidental changes:- uint bits1 = word >> 9 | 0x3F800000u; + uint bits1 = (word >> 9) & 0x007FFFFFu | 0x3F800000u;Apply the same pattern wherever a rotated/shifted word is converted to mantissa bits.
155-160: Minor: mark helperwordasuintonly when needed
wordis anoutparameter, but you often need it only when the caller asks for it.
An alternative is to overload with a defaulted parameter:float f1(inout uint seed, out uint word = uint(0));This would avoid duplicating wrapper overloads. Not critical, just a size/readability consideration.
Also applies to: 168-176, 184-194, 202-214
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
package/Shaders/Common/Random.hlsli(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Build plugin and addons
- GitHub Check: Validate shader compilation (Flatrim, .github/configs/shader-validation.yaml)
- GitHub Check: Validate shader compilation (VR, .github/configs/shader-validation-vr.yaml)
🔇 Additional comments (1)
package/Shaders/Common/Random.hlsli (1)
105-111: pcg stateful variant looks correctThe math matches the canonical PCG-XSH-RR 32-bit step and the state is correctly updated via the
inoutparameter.
No issues noticed here.
|
✅ A pre-release build is available for this PR: |
Summary by CodeRabbit
New Features
Refactor