Skip to content
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

Maya: JSON layout for Unreal using local transform matrix instead of world matrix #12

Conversation

moonyuet
Copy link
Member

@moonyuet moonyuet commented Jul 8, 2024

Changelog Description

When extracting layout, the json data should align with what Unreal used to load for the maya presets.
You can find the screengrab of the loaded preset for abc here:
ynput/ayon-unreal#15 (comment)

Additional Info

Please test along with this PR ynput/ayon-unreal#35

Testing Notes

  1. Launch Maya
  2. Create Layout with loaded assets
  3. Publish
  4. Load the layout back to the related loaded assets.

@BigRoy
Copy link
Contributor

BigRoy commented Jul 8, 2024

Don't have access to unreal setup on my end and it's hard to tell whether this is the correct fix. It is somewhat odd to me that just above we are storing translate, rotate, scale into the JSON file but store the matrix in an altered way.

Anyway, the code change itself seems fine. Whether it resolves an issue, I'm not sure.

@moonyuet
Copy link
Member Author

moonyuet commented Jul 8, 2024

Don't have access to unreal setup on my end and it's hard to tell whether this is the correct fix. It is somewhat odd to me that just above we are storing translate, rotate, scale into the JSON file but store the matrix in an altered way.

Anyway, the code change itself seems fine. Whether it resolves an issue, I'm not sure.

it also looks odd to me too ngl. But what I have been seen in the unreal settings seems to be that the layout setting doesn't align with the presets set in the unreal maya preset, so that's why some transformations mess up. It would be great to know whether this extract layout has been used anywhere other than Unreal or it has been used in other extensions as I do change something to be suitable for just loading abc in unreal @antirotor @LiborBatek would be great if you can give more info.

Copy link
Member

@LiborBatek LiborBatek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once I have produced layout product in maya and loaded into UE as load JSON layout I got my layout with applied transforms as when set for maya Alembic to UE but seems wrong oriented anyway... as seen on first img (my room being rotated to side instead of floor being the ground level) - note those transforms and its values...

Screenshot 2024-07-08 141743

once reset it to zero transform it looks much sensible aka correctly oriented in space as floor being ground level (wether its good or bad I cant tell but obviously closer to normal behaviour for sure)

Screenshot 2024-07-08 141844

and as seen the interior from FPView:

Screenshot 2024-07-08 142029

@moonyuet moonyuet requested a review from LiborBatek July 8, 2024 14:03
Copy link
Member

@LiborBatek LiborBatek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now the RotX being set to 90 and still not correct?
Screenshot 2024-07-08 173646

@moonyuet
Copy link
Member Author

moonyuet commented Jul 8, 2024

The updated commit should import layout as if this one.
image

@moonyuet moonyuet requested a review from LiborBatek July 8, 2024 16:03
@LiborBatek
Copy link
Member

ok I will do another test round and remove all assets (as some can be already existing with wrong transforms??) outside of the layout which re-uses such asset.

will update here asap, thx!

@LiborBatek
Copy link
Member

LiborBatek commented Jul 9, 2024

@moonyuet it seems working when FBX assets been loaded into layout ...not sure if the integration first checks if FBX repre exists but it did for model of the environment (as FBX exists for that one), but the rest of the assets in alembic repre seems not correct - I guess due to the fact we import those with wrong transforms?? not necessirely when layout product been loaded but maybe as they have been imported aka loaded in advance using AYON Loader (wrong transforms by AYON ??)

Another issue being that if any assets do not exists already within UE project (aka been loaded in advance) the Load Layout will fail resulting into empty UE map. Again I dont know if any particular UE addon branch needed in companion to this PR?? to make that working...

Screenshot 2024-07-09 094701

Screenshot 2024-07-09 094720

@moonyuet
Copy link
Member Author

moonyuet commented Jul 9, 2024

@moonyuet it seems working when FBX assets been loaded into layout ...not sure if the integration first checks if FBX repre exists but it did for model of the environment (as FBX exists for that one), but the rest of the assets in alembic repre seems not correct - I guess due to the fact we import those with wrong transforms?? not necessirely when layout product been loaded but maybe as they have been imported aka loaded in advance using AYON Loader (wrong transforms by AYON ??)

We need to have the option which allows users to export layout for alembic and fbx separately.

Another issue being that if any assets do not exists already within UE project (aka been loaded in advance) the Load Layout will fail resulting into empty UE map. Again I dont know if any particular UE addon branch needed in companion to this PR?? to make that working...

This should be issue for the layout loader in unreal. I will make an issue for that.

@moonyuet
Copy link
Member Author

@LiborBatek The updated PR can successfully flip y axis to z axis. The only circumstance is not working when the y rotation and z rotation both filled out with different values
Should we make a validator to make sure these y and z rotations are not filled at the same time to ensure it works correctly in unreal?

@LiborBatek
Copy link
Member

@LiborBatek The updated PR can successfully flip y axis to z axis. The only circumstance is not working when the y rotation and z rotation both filled out with different values Should we make a validator to make sure these y and z rotations are not filled at the same time to ensure it works correctly in unreal?

thats sounds really restrictive...as things living in 3 dimensional space, no?? I guess we have to solve it somehow...maybe someone can step in and help you out?

@moonyuet
Copy link
Member Author

moonyuet commented Jul 11, 2024

@LiborBatek The updated PR can successfully flip y axis to z axis. The only circumstance is not working when the y rotation and z rotation both filled out with different values Should we make a validator to make sure these y and z rotations are not filled at the same time to ensure it works correctly in unreal?

thats sounds really restrictive...as things living in 3 dimensional space, no?? I guess we have to solve it somehow...maybe someone can step in and help you out?

Yes It would be great if @antirotor or @BigRoy or even some potential community members can join to discuss on that and needs more research or discussion on figuring out as when y axis moves and it would move the z axis and vice versa. The numbers are varied under the condition of z-axis being over 90, and y axis being less than 90. Anyways, it would be great to have some discussion on how to fix the rotation when it has Y axis and Z axis.

Not sure if i gonna try on the graph discussed below but I think the best idea is to create another issue to discuss and do the decent fix.
image

Comment on lines 112 to 119
t = transform.translation(om.MSpace.kWorld)
t = om.MVector(t.x, t.z, -t.y)
s = transform.scale(om.MSpace.kObject)
s = [s[0], s[2], s[1]]
r = transform.rotation()
transform.setTranslation(t, om.MSpace.kWorld)
transform.rotateBy(
om.MEulerRotation(math.radians(-90), 0, 0), om.MSpace.kWorld)
transform.scaleBy([1.0, 1.0, -1.0], om.MSpace.kObject)
transform.setRotation(om.MEulerRotation(r.x, r.z, r.y))
transform.setScale(s, om.MSpace.kObject)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is it that we're trying to do - are we trying to rotate an object in world-space around the pivot point?
Isn't that just a matrix multiplication?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As maya by default is Y-axis up and unreal is Z-axis up, what we are trying to do is make sure transformation data which are Y-axis up converting to the data which are Z-axis up. Would be helpful if you can come up with some hints(or codes) for the matrix multiplication. We can edit the identity matrix from the basis, of which we dont need to setScale and setTranslation like this. But for the rotation is more complicated issue, it really needs some maths to be done.

@moonyuet
Copy link
Member Author

@LiborBatek
the current rotation should be correct for anything less than 90 degrees
it should be "correct" too for greater than 90 degrees, but you need to double check on the placement of the object as it looks a bit different.

Copy link
Member

@antirotor antirotor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

the left most and the right most are wrong. Left one in Unreal is X:0 Y:0 Z:-90 -> Z should +90 to look correct. Also the right most in Unreal is X:0 Y:0 Z:90 but the Z should be -90.

So I guess there is still issue of signs.


Note: in Maya, the left-most is 0, -90, 0 and the right one is 0, +90, 0

@moonyuet moonyuet requested a review from antirotor July 23, 2024 12:22
@moonyuet moonyuet requested a review from BigRoy July 23, 2024 15:00
@moonyuet
Copy link
Member Author

image

the left most and the right most are wrong. Left one in Unreal is X:0 Y:0 Z:-90 -> Z should +90 to look correct. Also the right most in Unreal is X:0 Y:0 Z:90 but the Z should be -90.

So I guess there is still issue of signs.

Note: in Maya, the left-most is 0, -90, 0 and the right one is 0, +90, 0

Should be working now
discord_image
discord_image_2

@antirotor
Copy link
Member

antirotor commented Jul 23, 2024

Not for me (middle one and the top one):

image
'
I am wondering if we are not running into gimbal lock problem where to rotations "flip". I'll try to run some more test to find out.

The middle one is 0, 0, -180 and the top one -90, 0, -180 in Unreal

@moonyuet
Copy link
Member Author

Not for me (middle one and the top one):

image ' I am wondering if we are not running into gimbal lock problem where to rotations "flip". I'll try to run some more test to find out.

The middle one is 0, 0, -180 and the top one -90, 0, -180 in Unreal

Have you pulled the updated commit? (Or I replicate the scene wrong?)

demonstration_for_transformation.mp4

Copy link
Member

@LiborBatek LiborBatek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is still some rotation offset on Z axis in UE as seen here

Screenshot 2024-07-30 115746

in maya the spacecraft has different orientation in ROT

Screenshot 2024-07-30 115733

Also worth noting that if I use Load Layout as JSON it wont bring any assets into UE just uses the existing ones (aka already loaded) ...it used do it automatically before - when some uasset missing it just loaded it in into Content / Ayon

@BigRoy
Copy link
Contributor

BigRoy commented Jul 30, 2024

There is still some rotation offset on Z axis in UE as seen here

I want to stress again that we should avoid really doing very explicit rotation checks and fixing each 'individual case' as if those are the only problematic areas and we're much better of "finding" the right algorithm that fixes the rotation Maya->Unreal. We referenced an algorithm from Unreal Live Link plug-in somewhere midway through this PR; I still feel that something along those lines is the only real approach... so in essence, finding the right 'math' for the conversion instead of trying to nail it by if statements to resolve very specific edge cases which I assume will (at best) only fix those specific cases and will end up leaving plethora of cases that are still problematic.

Unfortunately I don't have a "do this" solution (don't have unreal running here either). But I definitely feel we must be missing something 'obvious' since this should be a problem solved by many apps and/or importers/exporters throughout the years. If publicly available solutions do not work I keep the feeling that we may just be somehow preparing the data wrong to begin with. :D

…l-using-local-transform-matrix-instead-of-world-matrix
@moonyuet
Copy link
Member Author

Also worth noting that if I use Load Layout as JSON it wont bring any assets into UE just uses the existing ones (aka already loaded) ...it used do it automatically before - when some uasset missing it just loaded it in into Content / Ayon

we can create issues in unreal hosts to fix it.

…l-using-local-transform-matrix-instead-of-world-matrix
@moonyuet
Copy link
Member Author

moonyuet commented Aug 5, 2024

@antirotor @LiborBatek can you test this PR along with the unreal PR?

Copy link
Member

@LiborBatek LiborBatek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The layout has still some invalid rotations on asset level. I have tested it with corresponding PR but that didnt solve it.

image

@moonyuet
Copy link
Member Author

moonyuet commented Aug 5, 2024

The layout has still some invalid rotations on asset level. I have tested it with corresponding PR but that didnt solve it.

image

can you show me the transformation slot to see which rotation has been messed up?

Copy link
Member

@LiborBatek LiborBatek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR works with ayon-unreal and loading Layout into UE results into correct placement / rotation of assets.

image

…l-using-local-transform-matrix-instead-of-world-matrix
@antirotor antirotor merged commit a0d1933 into develop Aug 7, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants