Skip to content

Commit 6adde28

Browse files
DaPiCuclaude
DaPiCu
and
claude
committed
fix(26.2): crash-safe framebuffer readback for loupe and eyedropper
glReadPixels in MagnifierRenderer and ColorPickerGradients trusted the current pixel-pack state. If another mod (or vanilla) leaves GL_PACK_ROW_LENGTH / GL_PACK_SKIP_* or a bound GL_PIXEL_PACK_BUFFER behind, the driver writes past the end of the destination buffer, which hard-crashes the JVM (EXCEPTION_ACCESS_VIOLATION in atio6axx.dll on AMD). Route both reads through a new SafePixelReader that: - clamps the region to the framebuffer - unbinds GL_PIXEL_PACK_BUFFER and forces a known pack state, restoring it after - uses bounds-checked glReadnPixels (GL 4.5 / KHR / ARB robustness) when available - adds slack to the destination buffer as a last line of defence Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01877Hsz7gNYWAkb5JTx8WBm
1 parent 55704db commit 6adde28

3 files changed

Lines changed: 107 additions & 29 deletions

File tree

‎multiloader-26.2/common/src/main/java/io/github/marcsanzdev/chestseparators/client/ui/MagnifierRenderer.java‎

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
package io.github.marcsanzdev.chestseparators.client.ui;
22

3-
import java.nio.ByteBuffer;
43
import net.minecraft.client.Minecraft;
54
import net.minecraft.client.gui.GuiGraphicsExtractor;
6-
import org.lwjgl.opengl.GL11;
7-
import org.lwjgl.system.MemoryUtil;
85

96
/**
107
* Cursor zoom loupe. It samples a small block of the real framebuffer under the cursor and draws it
@@ -44,20 +41,9 @@ public static void render(GuiGraphicsExtractor context, int cursorX, int cursorY
4441
int fbX = clamp((int) Math.round((cursorX - srcHalf) * scale), 0, fbW - blockFb);
4542
int fbYBottom = clamp(fbH - (int) Math.round((cursorY + srcHalf) * scale), 0, fbH - blockFb);
4643

47-
int[] pixels = new int[blockFb * blockFb];
48-
// Heap-allocated native buffer: a stack buffer (MemoryStack) overflows for large loupe sizes.
49-
ByteBuffer buf = MemoryUtil.memAlloc(blockFb * blockFb * 4);
50-
try {
51-
GL11.glReadPixels(fbX, fbYBottom, blockFb, blockFb, GL11.GL_RGBA, GL11.GL_UNSIGNED_BYTE, buf);
52-
for (int i = 0; i < pixels.length; i++) {
53-
int r = buf.get(i * 4) & 0xFF;
54-
int g = buf.get(i * 4 + 1) & 0xFF;
55-
int b = buf.get(i * 4 + 2) & 0xFF;
56-
pixels[i] = 0xFF000000 | (r << 16) | (g << 8) | b;
57-
}
58-
} finally {
59-
MemoryUtil.memFree(buf);
60-
}
44+
// Crash-safe readback (forces a sane pixel-pack state, bounds-checked read). See SafePixelReader.
45+
int[] pixels = SafePixelReader.readArgb(fbX, fbYBottom, blockFb, blockFb);
46+
if (pixels == null) return;
6147

6248
// Place the loupe up-right of the cursor, flipping to stay on screen.
6349
int sw = client.getWindow().getGuiScaledWidth();

‎multiloader-26.2/common/src/main/java/io/github/marcsanzdev/chestseparators/client/ui/SafePixelReader.java‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
package io.github.marcsanzdev.chestseparators.client.ui;
2+
3+
import java.nio.ByteBuffer;
4+
import net.minecraft.client.Minecraft;
5+
import org.lwjgl.opengl.ARBRobustness;
6+
import org.lwjgl.opengl.GL;
7+
import org.lwjgl.opengl.GL11C;
8+
import org.lwjgl.opengl.GL15C;
9+
import org.lwjgl.opengl.GL21C;
10+
import org.lwjgl.opengl.GL45C;
11+
import org.lwjgl.opengl.GLCapabilities;
12+
import org.lwjgl.system.MemoryUtil;
13+
14+
/**
15+
* Crash-safe framebuffer readback for the loupe and the eyedropper.
16+
*
17+
* <p>Plain {@code glReadPixels} trusts the current pixel-pack state. Other mods (or vanilla) may leave
18+
* {@code GL_PACK_ROW_LENGTH}, {@code GL_PACK_SKIP_*} or a bound {@code GL_PIXEL_PACK_BUFFER} behind, which
19+
* makes the driver write more bytes than the destination buffer holds (hard JVM crash on AMD,
20+
* EXCEPTION_ACCESS_VIOLATION in atio6axx.dll). This helper:
21+
* <ul>
22+
* <li>clamps the region to the framebuffer,</li>
23+
* <li>forces a known pack state (no PBO, alignment 4, row length 0, no skips) and restores it after,</li>
24+
* <li>uses the bounds-checked {@code glReadnPixels} (GL 4.5 / KHR / ARB robustness) when available,</li>
25+
* <li>adds slack to the destination buffer as a last line of defence.</li>
26+
* </ul>
27+
*/
28+
public final class SafePixelReader {
29+
30+
private SafePixelReader() {}
31+
32+
private static final int SLACK = 64 * 1024;
33+
34+
/**
35+
* Reads a w*h block at framebuffer coords (x, yBottom) as opaque ARGB ints, row 0 = bottom row.
36+
* Returns {@code null} if nothing could be read; the caller should then skip drawing.
37+
*/
38+
public static int[] readArgb(int x, int yBottom, int w, int h) {
39+
Minecraft client = Minecraft.getInstance();
40+
if (client == null || client.getWindow() == null) return null;
41+
int fbW = client.getWindow().getWidth();
42+
int fbH = client.getWindow().getHeight();
43+
if (w <= 0 || h <= 0 || fbW <= 0 || fbH <= 0 || w > fbW || h > fbH) return null;
44+
x = Math.max(0, Math.min(x, fbW - w));
45+
yBottom = Math.max(0, Math.min(yBottom, fbH - h));
46+
47+
GLCapabilities caps;
48+
try {
49+
caps = GL.getCapabilities();
50+
} catch (Throwable t) {
51+
return null; // no GL context on this thread
52+
}
53+
54+
int bytes = w * h * 4;
55+
ByteBuffer buf = MemoryUtil.memCalloc(bytes + SLACK);
56+
buf.limit(bytes); // glReadnPixels uses remaining() as the hard size limit
57+
58+
int prevPbo = GL11C.glGetInteger(GL21C.GL_PIXEL_PACK_BUFFER_BINDING);
59+
int prevAlign = GL11C.glGetInteger(GL11C.GL_PACK_ALIGNMENT);
60+
int prevRowLen = GL11C.glGetInteger(GL11C.GL_PACK_ROW_LENGTH);
61+
int prevSkipRows = GL11C.glGetInteger(GL11C.GL_PACK_SKIP_ROWS);
62+
int prevSkipPixels = GL11C.glGetInteger(GL11C.GL_PACK_SKIP_PIXELS);
63+
int prevSwap = GL11C.glGetInteger(GL11C.GL_PACK_SWAP_BYTES);
64+
int prevLsb = GL11C.glGetInteger(GL11C.GL_PACK_LSB_FIRST);
65+
try {
66+
if (prevPbo != 0) GL15C.glBindBuffer(GL21C.GL_PIXEL_PACK_BUFFER, 0);
67+
GL11C.glPixelStorei(GL11C.GL_PACK_ALIGNMENT, 4);
68+
GL11C.glPixelStorei(GL11C.GL_PACK_ROW_LENGTH, 0);
69+
GL11C.glPixelStorei(GL11C.GL_PACK_SKIP_ROWS, 0);
70+
GL11C.glPixelStorei(GL11C.GL_PACK_SKIP_PIXELS, 0);
71+
GL11C.glPixelStorei(GL11C.GL_PACK_SWAP_BYTES, 0);
72+
GL11C.glPixelStorei(GL11C.GL_PACK_LSB_FIRST, 0);
73+
74+
if (caps.OpenGL45 || caps.GL_KHR_robustness) {
75+
GL45C.glReadnPixels(x, yBottom, w, h, GL11C.GL_RGBA, GL11C.GL_UNSIGNED_BYTE, buf);
76+
} else if (caps.GL_ARB_robustness) {
77+
ARBRobustness.glReadnPixelsARB(x, yBottom, w, h, GL11C.GL_RGBA, GL11C.GL_UNSIGNED_BYTE, buf);
78+
} else {
79+
GL11C.glReadPixels(x, yBottom, w, h, GL11C.GL_RGBA, GL11C.GL_UNSIGNED_BYTE, buf);
80+
}
81+
82+
int[] out = new int[w * h];
83+
for (int i = 0; i < out.length; i++) {
84+
int r = buf.get(i * 4) & 0xFF;
85+
int g = buf.get(i * 4 + 1) & 0xFF;
86+
int b = buf.get(i * 4 + 2) & 0xFF;
87+
out[i] = 0xFF000000 | (r << 16) | (g << 8) | b;
88+
}
89+
return out;
90+
} finally {
91+
GL11C.glPixelStorei(GL11C.GL_PACK_ALIGNMENT, prevAlign);
92+
GL11C.glPixelStorei(GL11C.GL_PACK_ROW_LENGTH, prevRowLen);
93+
GL11C.glPixelStorei(GL11C.GL_PACK_SKIP_ROWS, prevSkipRows);
94+
GL11C.glPixelStorei(GL11C.GL_PACK_SKIP_PIXELS, prevSkipPixels);
95+
GL11C.glPixelStorei(GL11C.GL_PACK_SWAP_BYTES, prevSwap);
96+
GL11C.glPixelStorei(GL11C.GL_PACK_LSB_FIRST, prevLsb);
97+
if (prevPbo != 0) GL15C.glBindBuffer(GL21C.GL_PIXEL_PACK_BUFFER, prevPbo);
98+
MemoryUtil.memFree(buf);
99+
}
100+
}
101+
}

‎multiloader-26.2/common/src/main/java/io/github/marcsanzdev/chestseparators/client/ui/screens/ColorPickerGradients.java‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,8 @@
11
package io.github.marcsanzdev.chestseparators.client.ui.screens;
22

33
import java.awt.Color;
4-
import java.nio.ByteBuffer;
54
import net.minecraft.client.Minecraft;
65
import net.minecraft.client.gui.GuiGraphicsExtractor;
7-
import org.lwjgl.opengl.GL11;
8-
import org.lwjgl.system.MemoryStack;
96

107
/**
118
* Stateless drawing helpers for the color picker: the saturation/value gradient box, the hue
@@ -43,14 +40,8 @@ static int readHoveredPixelColor(int mouseX, int mouseY) {
4340
double scale = client.getWindow().getGuiScale();
4441
int fbX = (int) (mouseX * scale);
4542
int fbY = client.getWindow().getHeight() - (int) (mouseY * scale) - 1;
46-
47-
try (MemoryStack stack = MemoryStack.stackPush()) {
48-
ByteBuffer buffer = stack.malloc(4);
49-
GL11.glReadPixels(fbX, fbY, 1, 1, GL11.GL_RGBA, GL11.GL_UNSIGNED_BYTE, buffer);
50-
int r = buffer.get(0) & 0xFF;
51-
int g = buffer.get(1) & 0xFF;
52-
int b = buffer.get(2) & 0xFF;
53-
return (r << 16) | (g << 8) | b;
54-
}
43+
// Crash-safe readback (forces a sane pixel-pack state, bounds-checked read).
44+
int[] px = io.github.marcsanzdev.chestseparators.client.ui.SafePixelReader.readArgb(fbX, fbY, 1, 1);
45+
return px == null ? 0 : (px[0] & 0x00FFFFFF);
5546
}
5647
}

0 commit comments

Comments
 (0)