-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
use a u64 for MeshPipelineKey #13015
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know if there's any perf impact from this?
Anyway this is fine since it fixes bugs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Increasing the storage size isn't ideal when we could localize the extension to the places where we're doing those bitshift operations. It's not a surefire way to avoid the issue, but the primary areas where those shifts are done are already encapsulated in functions on MeshPipelineKey
itself (i.e. from_primitive_toplogy
)
Not sure I understand what you mean? The bits should still be reserved for topology, even outside of from_primitive_toplogy The bits from the base key are reserved from the end, while the bits from the key are reserved from the start, they should not overlap |
Oh I was misreading the code and thought this problem was a result of bitshifting erasing some of the bits, disregard my prior comment. |
also fixed morph bit, used les magic numbers, and updated comments |
Objective
MeshPipelineKey
use some bits for two thingsLineStrip
lines
, there should be two groups of lines, the blue one doesn't display currentlySolution
MeshPipelineKey
to be backed by au64
instead, to have enough bits