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

ContainerScreenEvent$Render$Background runs after renderables are drawn #9862

Open
Fuzss opened this issue Feb 4, 2024 · 1 comment
Open
Labels
Triage This request requires the active attention of the Triage Team. Requires labelling or reviews.

Comments

@Fuzss
Copy link

Fuzss commented Feb 4, 2024

Minecraft Version: 1.20.4

Forge Version: 49.0.26

Description of issue:
In 1.20.1 the order in AbstractContainerScreen::render used to be:

  • AbstractContainerScreen::renderBg
    • Screen::renderBackground
      • Draws the black transparent background overlay
    • Draws the menu background sprite
  • ContainerScreenEvent$Render$Background (Forge event)
  • Screen::render (super method)
    • Draws the screen renderables
  • ...

In 1.20.2+ AbstractContainerScreen::renderBg has been removed in favor of using Screen::renderBackground which is now implemented in subclasses. Therefore the order in AbstractContainerScreen::render has changed to:

  • Screen::render (super method)
    • Screen::renderBackground
      • Draws the black transparent background overlay
      • Draws the menu background sprite
    • Draws the screen renderables
  • ContainerScreenEvent$Render$Background (Forge event)
  • ...

This new behavior makes it impossible to render background sprites which are drawn behind renderables like buttons & widgets.
For intentionally rendering sprites in front of renderables ContainerScreenEvent$Render$Foreground already exists.

My suggestion for restoring the 1.20.1 behavior would be to move firing ContainerScreenEvent$Render$Background to Screen::render (with an instanceof check for AbstractContainerScreen) right in-between the call to Screen::renderBackground and screen renderables being drawn.

Moving ContainerScreenEvent$Render$Background to AbstractContainerScreen::renderBackground is not a good option as it would fire after the black transparent background overlay is drawn, but before the menu background sprite is.

@Fuzss Fuzss added the Triage This request requires the active attention of the Triage Team. Requires labelling or reviews. label Feb 4, 2024
@LexManos
Copy link
Member

LexManos commented Feb 4, 2024

Instanceof is rather costly especially for things done every frame.
Better option would be to change AbstractContainerScreen to call renderBackground instead of render. And manually render the renderables as they are an accessible field. It'd make noticing changes a bit more painful during updates but it keeps the logic and patch to one place and doesn't have the performance cost of instance checks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Triage This request requires the active attention of the Triage Team. Requires labelling or reviews.
Projects
None yet
Development

No branches or pull requests

2 participants