-
Notifications
You must be signed in to change notification settings - Fork 33
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
Zero instance for TransformMatrixKHR #240
Comments
Removing it would cause VkAccelerationStructureInstanceKHR to lose its zero
instance too which is sad.
It's not too annoying to implement like that but I think it's a bit weird!
There are probably other zero instances with the same problem, like struts
with non optional arrays (which are currently empty)
…On Sat, Dec 12, 2020, 3:58 AM sheaf ***@***.***> wrote:
The current Zero instance for TransformMatrixKHR doesn't abide by the
specification:
instance Zero TransformMatrixKHR where
zero = TransformMatrixKHR
(zero, zero, zero, zero)
(zero, zero, zero, zero)
(zero, zero, zero, zero)
- The first three columns of matrix must define an invertible 3x3
matrix
If possible, the instance should be changed to
instance Zero TransformMatrixKHR where
zero = TransformMatrixKHR
(1,0,0,0)
(0,1,0,0)
(0,0,1,0)
However, if for any reason that's annoying, I think it would be better to
remove the instance entirely.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#240>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAGRJXBMYQPIBJPII4EXACDSUJ2VNANCNFSM4UXFDLGA>
.
|
I think the zero instance should be left as it is. But we could consider adding an 'Identity'/'One' class for this and some other things (such as |
Well I don't really see the point of the |
Yeah, you're right, it's actually only useful for satisfying that law which is pretty flimsy anyway, shouldn't be too hard to make the change. |
The current
Zero
instance forTransformMatrixKHR
doesn't abide by the specification:If possible, the instance should be changed to
However, if for any reason that's annoying, I think it would be better to remove the instance entirely.
The text was updated successfully, but these errors were encountered: