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

Rotated block models have the wrong quad facing, breaking block light rendering #461

Open
jellysquid3 opened this issue Dec 31, 2020 · 1 comment
Labels
A-vanilla-issue Area: Vanilla issue F-good-first-issue Flag: Good first issue for new contributors F-help-wanted Flag: Help wanted S-confirmed Status: Confirmed T-bug Type: Bug

Comments

@jellysquid3
Copy link
Member

If a JSON block model specifies the rotation attribute, vanilla will generate baked quads with the vertices transposed correctly, but the quad's facing will be that of the quad before it was rotated. This allows a call to BakedModel.getQuads to return BakedQuads which do not have the requested facing, and causes the lighting engine in Sodium to render incorrect results as it depends on the baked quad's facing being correct in order to avoid unnecessary computation.

There are a few ways to approach this issue, each with varying amounts of invasiveness:

  • Throw out the problematic optimization and ignore the baked quad's facing, which will hurt rendering performance. It also doesn't fix other mods which may try to use the quad's normal data which gets baked into chunk meshes (i.e. Iris)
  • Hook into BakedQuad initialization and set a new facing based on the normal vector, computed by the quad's vertices, and overwrite what vanilla passed along... Should be safe, as vanilla is bugged by this as well in normal data.
  • When rendering blocks, check whether the BakedQuad's facing matches the direction we just passed to BakedModel.getQuads, and if not, calculate new data with the "correct" facing. This makes the slow case a rarity, but still doesn't fix other things in the pipeline which depend on the BakedQuad having the right data in the first place.

A test case was provided by Layl#8888 on the Fabric Discord which reproduces this issue clearly.

Example of broken lighting
Example of broken block model

@jellysquid3 jellysquid3 added T-bug Type: Bug E-external Closed: External issue labels Dec 31, 2020
@amnotbananaama amnotbananaama added the S-confirmed Status: Confirmed label Dec 31, 2020
@LaylBongers
Copy link

Thinking over this issue, I personally do not see any situation where a rotation set to 0 giving a different result from no rotation specified at all is an intended feature rather than a bug. Maybe fix it in BakedQuad initialization with a conditional option for compatibility just in case?

@jellysquid3 jellysquid3 added A-vanilla-issue Area: Vanilla issue F-good-first-issue Flag: Good first issue for new contributors F-help-wanted Flag: Help wanted and removed E-external Closed: External issue labels Oct 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-vanilla-issue Area: Vanilla issue F-good-first-issue Flag: Good first issue for new contributors F-help-wanted Flag: Help wanted S-confirmed Status: Confirmed T-bug Type: Bug
Development

No branches or pull requests

3 participants