WorldRenderEvents.AFTER_SETUP fires immediately before a glClear() preventing rendering in MC 1.21.3

[jeseibel]

Minecraft added a glClear call right after the WorldRenderEvents.AFTER_SETUP call is fired, preventing any rendering from happening during that event.

This is a blocking issue for Distant Horizons since the next event in the stack BEFORE_ENTITIES is fired too late in Minecraft's rendering pipeline (I need to draw into the skybox).
If this is expected behavior moving forward please let me know so I can look into other solutions, thanks!

Previous behavior: MC 1.21.1

Fabric API 0.107.0+1.21.1

Final Frame:

Render Doc OpenGL event list:

---

New behavior: MC 1.21.3

Fabric API 0.107.0+1.21.3

Final Frame:

Render Doc OpenGL event list:

---

Here are render doc snapshots of both Minecraft versions in case you want to look closer:
render dock snapshots.zip

[adsumit]

How can I participate? I'm new to Java, OpenGL and also in GitHub.

[Alexander01998]

It seems like the cause of this issue is that WorldRenderer.renderSky() in 1.21.1 used to happen before WorldRenderer.setupTerrain() (and thus before the AFTER_SETUP event), but now in 1.21.3 it happens at a later stage.

It sounds like you would need a new AFTER_SKY event for Distant Horizons. To completely match the old behavior, this event would need to be fired after the if(!thickFogEnabled) statement that now surrounds the renderSky() call. The next instruction after that is WorldRenderer.renderMain().

So basically:

public class WorldRenderer {
  public void render(...) {
    // other code...
    if (!thickFogEnabled) {
      renderSky(...);
    }
    // --> Inject new AFTER_SKY event here. <--
    renderMain(...);
    // other code...
  }
}

This is all assuming that it wouldn't make more sense to move the AFTER_SETUP to that position instead. I don't know how other mods are using that event and what behavior they are relying on. Adding a new event seems like a safer option for compatibility.

I also don't know if adding this event would be considered out of scope for Fabric API. That's up to the maintainers to decide.

Either way, I hope these findings help to move this issue forward and look forward to seeing Distant Horizons work in 1.21.3. 👍

[JustRed23]

It seems like the cause of this issue is that WorldRenderer.renderSky() in 1.21.1 used to happen before WorldRenderer.setupTerrain() (and thus before the AFTER_SETUP event), but now in 1.21.3 it happens at a later stage.

It sounds like you would need a new AFTER_SKY event for Distant Horizons. To completely match the old behavior, this event would need to be fired after the if(!thickFogEnabled) statement that now surrounds the renderSky() call. The next instruction after that is WorldRenderer.renderMain().

So basically:

public class WorldRenderer {
  public void render(...) {
    // other code...
    if (!thickFogEnabled) {
      renderSky(...);
    }
    // --> Inject new AFTER_SKY event here. <--
    renderMain(...);
    // other code...
  }
}

This is all assuming that it wouldn't make more sense to move the AFTER_SETUP to that position instead. I don't know how other mods are using that event and what behavior they are relying on. Adding a new event seems like a safer option for compatibility.

I also don't know if adding this event would be considered out of scope for Fabric API. That's up to the maintainers to decide.

Either way, I hope these findings help to move this issue forward and look forward to seeing Distant Horizons work in 1.21.3. 👍

You were on the right track here, but the actual event needed to be fired after the terrain fog shader was set up, this was moved to renderMain, so I just fire a new event directly after that

[adsumit]

Is moving the injection point sufficient?

[JustRed23]

Yes, I did a local test with a locally published version of this fix, made a quick fix to distant horizons and it rendered fine

[jeseibel]

From an API perspective I feel like it'd be better to fix the existing event vs adding a new one. But that's not my decision to make and I can work with either.
Thanks for the confirmed fix @JustRed23