-
-
Notifications
You must be signed in to change notification settings - Fork 957
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
Xenko matrix memory layout discussion #291
Comments
@tebjan I think the idea at the time was to send data to HLSL as is (memcpy) and be able to keep HLSL order I also remember something like However, I would be totally fine to revisit this and have your opinion. I personally don't like the M11/M21 memory layout either, it's opposite to what most engine do 😄 |
FYI we had M11/M12 memory layout in first iteration of the engine the two reasons for the switch were: (1) previously mentionned HLSL opcodes reasons: dot3 line by line now possible (or maybe it was the opposite? sorry it was in 2012!) (2) Also I could find in our git history that contrary to what I thought, we had M11/M12 with Note that due to the various rewrite and optimizations we are now doing, cbuffer is now treated as a memory block without distinction for individual variables when uploading so this kind of transpose in (2) is not possible anymore (and probably something we want to avoid anyway). If we go back to M11/M12, I think now the only ways would be to either: So probably if we decide to go through, we would have to review a bunch of shaders and swap the multiplications. |
interesting write up, at first glance it sounds like a good decision. so mainly it's to have the matrix layout correct for the shaders... i have never seen this combination of memory layout and matrix/vector convention before. it remains to evaluate where this gets the unsuspecting user into trouble. i guess as soon as you get in contact with other libraries (as seen in #289). before changing anything i would evaluate how other game engines solve that and what kind of conventions they follow? would be interesting to know. |
had a look into a vulkan example since it will probably be the dominant API in the future and they seem to write multiplications in openGL style, also they use the glm math lib: https://vulkan-tutorial.com/Uniform_buffers/Descriptor_layout_and_buffer#page_Vertex_shader could the xksl -> v-spir compiler take care of something like this? |
OpenGL/Spir-V conversion is already taking care of that (and behavior can even be swapped using a flag): https://github.com/xenko3d/xenko/search?q=NoSwapForBinaryMatrixOperation&unscoped_q=NoSwapForBinaryMatrixOperation However, this is irrelevent: If we were to swap memory layout and I think we currently end up with |
For reference: Shader code with M11/M12:
Shader code with M11/M21:
So no worries there, it's not bigger and the write differences (component by component vs full vector update) should be negligeable compared to all other stuff we do in the shader. Engines:
So far, the biggest downside to switch back to M11/M12 I can think of is inconsistencies between shader and C# code:
|
One last detail that comes to mind: simpler matrices (i.e. bones) can often be encoded in |
So all things considered, once we decide what to do with the C#/HLSL differences (keep it like this? or swap C#?), I would be fine to go back to M11/M12 memory layout order. |
excellent write up, and now that i understand the decisions you made better, i am not sure about my vote. personally i also like the but if the majority of engines and examples from vulkan and nvidia use the openGL style, i would rather go this way and re-learn it. just be be able to easily copy GPU gems or other examples into XKSL and have minimal work to adapt it. another wild idea could be, since you have a flag and the code already, that you can define the mul order in the shader file somehow, via a flag/#define or something like that... |
Took a quick look, it's true most D3D samples tend to have the
|
that is indeed a bit problematic. since directx as well es opengl and i guess bullet have exactly the same memory layout. i wonder what was leading to that decision and which action was taken. my feeling is that there was a couple of wrong assumptions of how matrix transposition and coordinate system handedness works.
Originally posted by @tebjan in #289 (comment)
The text was updated successfully, but these errors were encountered: