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

[1.20.2] Fix blending issues caused by item decorators #241

Merged
merged 5 commits into from
Nov 10, 2023

Conversation

XFactHD
Copy link
Member

@XFactHD XFactHD commented Nov 3, 2023

This PR fixes blending issues in inventories by properly resetting the render state - disabling blending specifically - after the ItemDecoratorHandler has drawn all decorators. As far as I can tell, this doesn't cause any regressions in other places that call any of the two GuiGraphics#renderItemDecorations() overloads (i.e. recipe book ghost recipe, bundle tooltip, merchant list, etc.), but I would appreciate it if someone else would cross-check this.

Fixes #185

@sciwhiz12 sciwhiz12 added bug A bug or error 1.20 Targeted at Minecraft 1.20 rendering Related to rendering labels Nov 3, 2023
@Technici4n
Copy link
Member

None of the item rendering in Vanilla seems to be touching the RenderSystem state anymore - do we still need to do this?

@XFactHD
Copy link
Member Author

XFactHD commented Nov 6, 2023

The decorator renderers might touch it though, so some kind of reseting is still a good idea in my opinion, considering that people are unlikely to notice this kind of issue (especially with the blending stuff as evidenced by the issue referenced by this PR) because it's not visible with vanilla assets.

@Technici4n
Copy link
Member

What I would suggest instead is that we add a few Blaze3D patches to retrieve the current blend mode etc, and then use that to always reset to the correct state:

+ boolean depthTest = RenderSystem.isDepthTestEnabled();

  resetRenderState();
  for (IItemDecorator itemDecorator : itemDecorators) {
      if (itemDecorator.render(guiGraphics, font, stack, xOffset, yOffset))
          resetRenderState();
  }

+ if (!depthTest) RenderSystem.disableDepthTest();

and similar for the others. BlendFunc might be a bit tricky because it is defined by 4 integers, maybe we want to write that to a mutable struct.

This system will generalize nicely to fix other similar issues, and will ensure that we always reset to the correct render state. Might be worth giving this some thought because I expect that mods might want to use similar hooks. Maybe we want to make this more general for GUI state?

Something like RenderSystem.captureGuiState(holderStruct) and RenderSystem.applyGuiState(holderStruct)...

@XFactHD
Copy link
Member Author

XFactHD commented Nov 7, 2023

Yeah, I could see that backup and restore approach working quite well for this. I'll prototype something tomorrow.

@XFactHD
Copy link
Member Author

XFactHD commented Nov 10, 2023

I've implemented a simple way to backup and restore the GL state. Not my prettiest work, but it does the trick. To avoid confusion: I have intentionally not added the cull mode to the backup because it's currently never modified and I have also intentionally not added the shader colors stored in RenderSystem because they have to be handled through GuiGraphics due to the partial batching it does for untextured stuff and text.

Copy link
Member

@Technici4n Technici4n left a comment

Choose a reason for hiding this comment

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

OK that's a lot of state saving, but that should be fast enough (since it's just Java field reads and writes). I think we should restrict these methods to the render thread though.

patches/com/mojang/blaze3d/systems/RenderSystem.java.patch Outdated Show resolved Hide resolved
@Technici4n Technici4n merged commit c0f6ec7 into neoforged:1.20.x Nov 10, 2023
2 checks passed
@XFactHD XFactHD deleted the creative_inv_blend branch November 10, 2023 16:23
Xing-C added a commit to Xing-C/NeoForge that referenced this pull request Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.20 Targeted at Minecraft 1.20 bug A bug or error rendering Related to rendering
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The scroll bar blinks with some resource packs that modify Minecraft GUI textures.
3 participants