Fix crash on renderbuffers with no sampler filter (#674)#870
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds defensive handling in the GL emulation layer to avoid crashes when other mods create or configure custom framebuffer objects (FBOs) in ways that real OpenGL tolerates (notably Xaero’s ImprovedFramebuffer).
Changes:
- Skip
VkGlRenderbuffer.updateSampler()work whenminFilteris unset (0) to avoid throwing on renderbuffer-backed images. - Add a guard in
VkGlFramebuffer.framebufferTexture2Dto avoid attaching/begin-rendering when the target texture (or itsVulkanImage) is not yet allocated.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/main/java/net/vulkanmod/gl/VkGlRenderbuffer.java | Adds an early-return in updateSampler() for renderbuffers with unset min filter. |
| src/main/java/net/vulkanmod/gl/VkGlFramebuffer.java | Adds a deferred-attach guard in framebufferTexture2D to avoid null-image attachment crashes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| VkGlTexture vkGlTexture = VkGlTexture.getTexture(texture); | ||
| if (vkGlTexture == null || vkGlTexture.getVulkanImage() == null) | ||
| return; |
VkGlRenderbuffer treats a GL renderbuffer like a sampled texture and
demands a min-filter. A GL renderbuffer is a render target, not a sampled
resource, so it legitimately has no sampler/filter: minFilter stays at its
unset default of 0. updateSampler() then falls through to its default ->
branch and throws:
java.lang.IllegalStateException: Unexpected min filter value: 0
at net.vulkanmod.gl.VkGlRenderbuffer.updateSampler(VkGlRenderbuffer.java:178)
at net.vulkanmod.gl.VkGlRenderbuffer.allocateIfNeeded(VkGlRenderbuffer.java:133)
at net.vulkanmod.gl.VkGlRenderbuffer.renderbufferStorage(VkGlRenderbuffer.java:58)
This crashes any mod that builds a custom FBO with a depth renderbuffer;
in the wild it is triggered by Xaero's ImprovedFramebuffer.renderbufferStorage
(issue xCollateral#674).
Fix: treat minFilter == 0 as "no sampler needed" and return early from
updateSampler(), skipping sampler creation for the renderbuffer.
9b1d271 to
18b37c3
Compare
Chaos02
changed the title
|
Scope note while this is open, since it is Xaero motivated: The renderbuffer crash fixed here (#674), and the FBO deferred-attach issue mentioned in the description, are both legitimately VulkanMod's side. VulkanMod emulates GL framebuffers, so it should tolerate an FBO built the way Xaero builds one (attach-before-allocate). There is a third Xaero-on-VulkanMod crash that I would call out of scope for VulkanMod though. Xaero's World Map calls Xaero's trackers, for cross reference:
Flagging it so it does not get lumped into VulkanMod's Xaero compat surface. |
|
Related PRs from the same effort (getting a VulkanMod based 1.21.1 pack working):
One correction to the scope note in the description above. I said Xaero's map needed a proper deferred attachment resolve in The deferred attach case is still real (a texture attached to an FBO before its storage is allocated does throw "Image is null"), it is just a separate crash rather than the map corruption. Happy to put that up separately if it is wanted. |
VkGlRenderbuffertreats a GL renderbuffer like a sampled texture and demands a min filter. A renderbuffer is a render target with no sampler, sominFilterstays at its unset default of 0 andupdateSampler()falls through tothrow "Unexpected min filter value: 0". TreatminFilter == 0as "no sampler needed" and return early.Triggered in the wild by Xaero's
ImprovedFramebuffer(issue #674). Confirmed fixed in-game on MC 1.21.1 with Xaero's Minimap / World Map.Scope note: I originally paired this with a framebuffer deferred-attach guard, but that only turns the crash into wrong-texture map rendering — Xaero's map needs a proper deferred attachment-resolve in
VkGlFramebuffer, which I'm working on separately. This PR is just the standalone renderbuffer crash fix.