-
Notifications
You must be signed in to change notification settings - Fork 76
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
Fixed large uv wrapping #249
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.
Does fast64 use the max shift of two textures for exporting? (Can you link to code) (why is this even a per texture setting in fast64)
This is the code that wraps uv´s back into valid bounds |
Arthur tested this and confirmed it works as intended btw. Should be ready for one more review and then being merged |
Arthur suggested this a solution.
72e460c
to
117ac42
Compare
All known issues should be fixed now |
fast64_internal/f3d/f3d_writer.py
Outdated
return (SShift * sMirrorScale, TShift * tMirrorScale) | ||
|
||
|
||
def getUVInterval(f3dMat) -> tuple[int, int]: |
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.
I don't think this behaves as expected if e.g. tex0 has shift=0 mirror=True and tex1 has shift=1 mirror=False
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.
When shift is 0, SShift will be 1, because 2^0 is 1. What´s wrong here?
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.
The result of your case should be 2.
2 if f3dMat.tex0.T.mirror or f3dMat.tex1.T.mirror else 1, | ||
] | ||
# To prevent wrong UVs when wrapping UVs into valid bounds, | ||
# we need to account for the highest texture shift and if mirroring is active. |
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.
To reiterate on my previous comments why "highest"? There must be a piece of code elsewhere on exporting that handles this similarly? Where
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.
There isn´t because this is doing something else, we don´t need to account for texture resolutions and what not, so there is no function which does this behavior specifically.
I have some comments on style but the actual code makes sense and if it is tested then I don't feel the need for any functionality changes |
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.
code is ok to me but I still need to get feedback from mario people and I need to see if anything breaks on oot (since this is F3D stuff)
I'll approve once both sides are tested and ok
Oops closed on accident, what I was going to say is that fazana had incorrect mask values, the pr should be working correctly. |
getSTUVRepeats couldn´t auto type hint, getUVInverval can. Use 1.0 instead of 1
Could you test then please? This shouldn´t affect oot differently at all. |
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.
works on my machine
Arthur suggested this a solution, he said he could test it later today. Until then this will remain a draft pr.