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

use SpecialRenderer in ItemRenderer #33

Merged
merged 3 commits into from
Apr 22, 2024

Conversation

jacobsjo
Copy link
Contributor

This allows rendering chests and decorated pots as items

mat4.scale(t, t, [16, 16, 16])
additionalMesh.transform(t)
}
const mesh = model.getDisplayMesh('gui', this.resources, tint, additionalMesh)
Copy link
Owner

Choose a reason for hiding this comment

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

Can the additionalMesh not be merged with the mesh returned from getDisplayMesh instead of passing it to the function?

const mesh = model.getDisplayMesh('gui', this.resources, tint)
mesh.merge(additionalMesh)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The additionalMesh also need to get all the transformations that getDisplayMesh applies on the mesh, so it has to be merged before the transformations (or be transformed separately).

@misode
Copy link
Owner

misode commented Apr 22, 2024

I changed it so the ItemRenderer now transforms using the display setting separately, this way it can merge the special renderer mesh itself. I wondered whether I had to add back the deprecated method in BlockModel, but I don't mind a small breaking change for someone that is using this internal method.

Maybe this is not a problem, but when testing the demo (npm run demo) it seems like the special renderer texture mapping is slightly off. @jacobsjo do you have any idea why this is happening?
image

@jacobsjo
Copy link
Contributor Author

That rewrite looks good, certainly much cleaner. The texture mapping in the demo was an issue with the UV of non-16x16 textures in the demo code itself.

@misode misode merged commit df2fe84 into misode:main Apr 22, 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.

2 participants