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

Fix rotation + flipping for GT textures #4016

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

RecursivePineapple
Copy link
Contributor

@RecursivePineapple RecursivePineapple commented Mar 2, 2025

When you flip + rotate an AL, its texture doesn't match its orientation. From what I can tell, this is because structurelib and RenderBlocks rotate & flip in different orders. To fix this, I copied the face rendering code from RenderBlocks and cleaned it up significantly then modified it as needed.

While I'm sure there's a simpler/lower code way to do this, I didn't want to deal with RenderBlocks' quirks if I didn't have to. From what I can see, RenderBlocks' only benefit is AO, but I copied all of that code here. This change won't affect the angelica animation system because of this PR: GTNewHorizons/Angelica#868

This change adds a few more allocations to the ISBRH render, but I figure it's worth it to keep it simple. Pooling would reduce the allocs, but I don't want to do that unless there's a clear performance hit.

2025-03-02_15 35 09

Testing done:

  • Make sure AL's texture is correct in all six facings (top & bottom have up = negative z, which is what it was previously)
  • Check each facing's 4 rotations to make sure they match the structure (AL's white control panel thingy should point to its main body)
  • Do the same, but with the AL flipped
  • Disable useExtFacing and check the six facings again to make sure the facings match the unrotated & unflipped ExtendedFacing
  • Check maintenance hatches (since they can be rotated but not flipped)
  • Check normal gt singleblocks with multiple textures (biolab in this case since it has a top texture) to make sure they work the same

@RecursivePineapple RecursivePineapple added the bug fix Fix a bug. Please link it in the PR. label Mar 2, 2025
@RecursivePineapple RecursivePineapple requested a review from a team March 2, 2025 20:37
Copy link
Contributor

@leagris leagris left a comment

Choose a reason for hiding this comment

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

Please do not replace the standard renderer with your own implementation that merely duplicates existing code. Even if it does not cause issues now, it will create problems later. For example, if someone patches the renderer to leverage rendering improvements, that patch will not apply to your duplicated implementation.

The standard renderer already handles all texture rotations and flips. I see no valid reason for a custom implementation that directly interacts with the tessellator.

@Dream-Master Dream-Master requested review from leagris and a team March 3, 2025 16:24
@Dream-Master Dream-Master added the 🚧 Testing on Zeta Do not merge yet, testing this PR on Zeta label Mar 3, 2025
Copy link
Contributor

@leagris leagris left a comment

Choose a reason for hiding this comment

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

Looks much better. Thank you for this nice contribution.

@Dream-Master Dream-Master enabled auto-merge (squash) March 3, 2025 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fix Fix a bug. Please link it in the PR. 🚧 Testing on Zeta Do not merge yet, testing this PR on Zeta
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants