diff --git a/src/main/java/com/metallum/client/metal/render/MetalCommandEncoder.java b/src/main/java/com/metallum/client/metal/render/MetalCommandEncoder.java index 40f5d4327..9b0492d5f 100644 --- a/src/main/java/com/metallum/client/metal/render/MetalCommandEncoder.java +++ b/src/main/java/com/metallum/client/metal/render/MetalCommandEncoder.java @@ -1,5 +1,6 @@ package com.metallum.client.metal.render; +import com.metallum.Metallum; import com.metallum.client.metal.render.bridge.MetalNativeBridge; import com.metallum.client.metal.render.mtl.*; import com.mojang.blaze3d.buffers.GpuBuffer; @@ -45,6 +46,7 @@ final class MetalCommandEncoder implements CommandEncoderBackend { private MemorySegment renderColorAttachment = MemorySegment.NULL; private MemorySegment renderDepthAttachment = MemorySegment.NULL; private final Long2ObjectOpenHashMap> dynamicBackingPool = new Long2ObjectOpenHashMap<>(); + private static final int MAX_POOLED_DYNAMIC_BACKINGS_PER_SIZE = 8; MetalCommandEncoder(final MetalDevice device) { this.device = device; @@ -339,6 +341,9 @@ private void orphanWrite(final MetalGpuBuffer buffer, final long offset, final B long size = buffer.allocationSize(); MemorySegment old = buffer.nativeHandle(); MemorySegment fresh = acquireDynamicBacking(size, buffer.resourceOptions()); + if (fresh.address() == 0L) { + return; + } ByteBuffer freshStorage = MetalNativeBridge.nativeByteBufferView( MetalNativeBridge.metallum_get_buffer_contents(fresh), size).order(ByteOrder.nativeOrder()); @@ -353,23 +358,33 @@ private void orphanWrite(final MetalGpuBuffer buffer, final long offset, final B dst.put(data.duplicate()); buffer.swapBacking(fresh, freshStorage); - recycleDynamicBacking(old, size); + recycleDynamicBacking(old, size, buffer.resourceOptions()); } private MemorySegment acquireDynamicBacking(final long size, final long resourceOptions) { - java.util.ArrayDeque bucket = dynamicBackingPool.get(size); + final long key = MetalDevice.composePoolKey(size, resourceOptions); + final java.util.ArrayDeque bucket = dynamicBackingPool.get(key); if (bucket != null && !bucket.isEmpty()) { return bucket.pop(); } - MemorySegment handle = MetalNativeBridge.metallum_create_buffer(device.metalDeviceHandle(), size, resourceOptions); + final MemorySegment handle = MetalNativeBridge.metallum_create_buffer(device.metalDeviceHandle(), size, resourceOptions); if (MetalNativeBridge.isNullHandle(handle)) { - throw new IllegalStateException("Failed to create dynamic backing buffer"); + Metallum.LOGGER.warn("dynamic backing OOM, skipping uniform update this frame"); + return MemorySegment.NULL; } return handle; } - private void recycleDynamicBacking(final MemorySegment handle, final long size) { - queueForDestroy(() -> dynamicBackingPool.computeIfAbsent(size, k -> new java.util.ArrayDeque<>()).push(handle)); + private void recycleDynamicBacking(final MemorySegment handle, final long size, final long resourceOptions) { + queueForDestroy(() -> { + final long key = MetalDevice.composePoolKey(size, resourceOptions); + java.util.ArrayDeque bucket = dynamicBackingPool.computeIfAbsent(key, k -> new java.util.ArrayDeque<>()); + if (bucket.size() < MAX_POOLED_DYNAMIC_BACKINGS_PER_SIZE) { + bucket.push(handle); + } else { + MetalNativeBridge.metallum_release_object(handle); + } + }); } @Override diff --git a/src/main/java/com/metallum/client/metal/render/MetalDevice.java b/src/main/java/com/metallum/client/metal/render/MetalDevice.java index 765f46bbe..091d16c5f 100644 --- a/src/main/java/com/metallum/client/metal/render/MetalDevice.java +++ b/src/main/java/com/metallum/client/metal/render/MetalDevice.java @@ -42,8 +42,20 @@ final class MetalDevice implements GpuDeviceBackend { private final Map compiledPipelines = new IdentityHashMap<>(); private final Map shaderCache = new HashMap<>(); private final Map functionCache = new HashMap<>(); - private final Map> bufferPool = new HashMap<>(); - private static final int MAX_POOLED_BUFFERS_PER_SIZE = 16; + private static final int MAX_POOLED_BUFFER_BUCKETS = 32; + private static final int MAX_POOLED_BUFFERS_PER_SIZE = 8; + private final Map> bufferPool = new LinkedHashMap<>(16, 0.75f, true) { + @Override + protected boolean removeEldestEntry(final Map.Entry> eldest) { + if (size() <= MAX_POOLED_BUFFER_BUCKETS) { + return false; + } + for (MemorySegment handle : eldest.getValue()) { + MetalNativeBridge.metallum_release_object(handle); + } + return true; + } + }; private ShaderSource activeShaderSource; MetalDevice( @@ -126,11 +138,17 @@ final class MetalDevice implements GpuDeviceBackend { @Override public @NonNull GpuBuffer createBuffer(@Nullable final Supplier label, @GpuBuffer.Usage final int usage, final long size) { + if (size <= 0L) { + throw new IllegalArgumentException("Metal buffer size must be > 0 (got " + size + ")"); + } return new MetalGpuBuffer(this, usage, size); } @Override public @NonNull GpuBuffer createBuffer(@Nullable final Supplier label, @GpuBuffer.Usage final int usage, final ByteBuffer data) { + if (data == null || data.remaining() <= 0) { + throw new IllegalArgumentException("Cannot create buffer from empty ByteBuffer"); + } MetalGpuBuffer buffer = (MetalGpuBuffer) this.createBuffer(label, usage | GpuBuffer.USAGE_COPY_DST, data.remaining()); this.commandEncoder.writeToBuffer(buffer.slice(), data.duplicate()); return buffer; @@ -207,6 +225,10 @@ MemorySegment metalDeviceHandle() { return this.metalDeviceHandle; } + long maxBufferAllocationSize() { + return this.deviceInfo.limits().maxMemoryAllocationSize(); + } + void waitForSubmittedGpuWork() { this.commandEncoder.waitForSubmittedGpuWork(); } @@ -236,7 +258,7 @@ void queueBufferRelease(final MemorySegment handle, final long size, final long }); } - private static long composePoolKey(final long size, final long resourceOptions) { + static long composePoolKey(final long size, final long resourceOptions) { return (size << 12) | (resourceOptions & 0xFFFL); } diff --git a/src/main/java/com/metallum/client/metal/render/MetalGpuBuffer.java b/src/main/java/com/metallum/client/metal/render/MetalGpuBuffer.java index 24d3d187c..cb6fe79cb 100644 --- a/src/main/java/com/metallum/client/metal/render/MetalGpuBuffer.java +++ b/src/main/java/com/metallum/client/metal/render/MetalGpuBuffer.java @@ -35,7 +35,14 @@ class MetalGpuBuffer extends GpuBuffer { this.dynamic = isDynamic(usage); this.cpuAccessible = isCpuAccessible(usage) || this.dynamic; this.resourceOptions = toMtlResourceOptions(usage); - this.allocationSize = (size + 15L) & ~15L; + if (size <= 0L) { + throw new IllegalArgumentException("Metal buffer size must be > 0 (got " + size + ")"); + } + long aligned = (size + 15L) & ~15L; + if (aligned <= 0L) { + throw new IllegalArgumentException("Metal buffer size overflow after alignment: " + size); + } + this.allocationSize = aligned; MemorySegment pooled = device.tryAcquirePooledBuffer(this.allocationSize, this.resourceOptions); if (!MetalNativeBridge.isNullHandle(pooled)) { @@ -45,16 +52,20 @@ class MetalGpuBuffer extends GpuBuffer { if (MetalNativeBridge.isNullHandle(contents)) { MetalNativeBridge.metallum_release_object(pooled); this.nativeHandle = null; - throw new IllegalStateException("MTLBuffer.contents returned null for pooled buffer"); + throw new IllegalStateException("MTLBuffer.contents returned null for pooled buffer (size=" + this.allocationSize + ", resourceOptions=" + this.resourceOptions + ")"); } this.storage = MetalNativeBridge.nativeByteBufferView(contents, this.allocationSize).order(ByteOrder.nativeOrder()); } return; } + long max = device.maxBufferAllocationSize(); + if (max > 0L && this.allocationSize > max) { + throw new IllegalArgumentException("Metal buffer size " + this.allocationSize + " exceeds device max " + max); + } this.nativeHandle = MetalNativeBridge.metallum_create_buffer(device.metalDeviceHandle(), this.allocationSize, this.resourceOptions); if (MetalNativeBridge.isNullHandle(this.nativeHandle)) { - throw new IllegalStateException("Failed to create Metal buffer"); + throw new IllegalStateException("Failed to create Metal buffer (size=" + this.allocationSize + ", resourceOptions=" + this.resourceOptions + ", device=" + this.device.getClass().getSimpleName() + ")"); } if (this.cpuAccessible) { @@ -62,7 +73,7 @@ class MetalGpuBuffer extends GpuBuffer { if (MetalNativeBridge.isNullHandle(contents)) { MetalNativeBridge.metallum_release_object(this.nativeHandle); this.nativeHandle = null; - throw new IllegalStateException("MTLBuffer.contents returned null"); + throw new IllegalStateException("MTLBuffer.contents returned null (size=" + this.allocationSize + ", resourceOptions=" + this.resourceOptions + ")"); } this.storage = MetalNativeBridge.nativeByteBufferView(contents, this.allocationSize).order(ByteOrder.nativeOrder()); @@ -92,8 +103,8 @@ ByteBuffer sliceStorage(final long offset, final long length) { } MemorySegment nativeHandle() { - if (this.nativeHandle == null) { - throw new IllegalStateException("Native Metal buffer is closed"); + if (this.nativeHandle == null || this.nativeHandle.address() == 0L) { + throw new IllegalStateException("Native Metal buffer is closed or null"); } return this.nativeHandle; } diff --git a/src/main/java/com/metallum/client/metal/render/MetalRenderPass.java b/src/main/java/com/metallum/client/metal/render/MetalRenderPass.java index 0d356a1d3..e99749e76 100644 --- a/src/main/java/com/metallum/client/metal/render/MetalRenderPass.java +++ b/src/main/java/com/metallum/client/metal/render/MetalRenderPass.java @@ -1,5 +1,6 @@ package com.metallum.client.metal.render; +import com.metallum.Metallum; import com.metallum.client.metal.render.bridge.MetalNativeBridge; import com.metallum.client.metal.render.mtl.*; import com.mojang.blaze3d.GpuFormat; @@ -183,6 +184,10 @@ private void setIndexBuffer(@Nullable final GpuBuffer indexBuffer, final MTLInde @Override public void drawIndexed(final int indexCount, final int instanceCount, final int firstIndex, final int vertexOffset, final int firstInstance) { + if (this.indexBuffer == null) { + Metallum.LOGGER.warn("[metallum] drawIndexed called with null index buffer, skipping draw"); + return; + } MetalGpuBuffer nativeIndexBuffer = (MetalGpuBuffer) indexBuffer; MTLRenderCommandEncoder enc = renderEncoder(); @@ -233,10 +238,26 @@ public void multiDrawIndexed(@NonNull PointerBuffer firstIndexOffsets, @NonNull @Override public void drawIndexedIndirect(final @NonNull GpuBufferSlice commands, final int drawCount) { + if (drawCount <= 0) { + return; + } MTLPrimitiveType primitiveType = primitiveTopology(); if (primitiveType == MTLPrimitiveType.TriangleFan) { throw new UnsupportedOperationException("Metal backend does not support triangle fan indirect draws"); } + if (this.indexBuffer == null) { + Metallum.LOGGER.warn("[metallum] drawIndexedIndirect called with null index buffer, skipping draw"); + return; + } + if (commands.buffer().isClosed()) { + Metallum.LOGGER.warn("[metallum] drawIndexedIndirect called with closed indirect command buffer, skipping draw"); + return; + } + long needed = (long) drawCount * VkDrawIndexedIndirectCommand.SIZEOF; + if (commands.length() < needed) { + Metallum.LOGGER.warn("[metallum] drawIndexedIndirect command buffer too small: need {} bytes, have {} (drawCount={})", needed, commands.length(), drawCount); + return; + } MetalGpuBuffer nativeIndexBuffer = (MetalGpuBuffer) indexBuffer; MTLRenderCommandEncoder enc = renderEncoder(); diff --git a/src/main/native/MetallumNative.swift b/src/main/native/MetallumNative.swift index 0cec1d417..52a596c1d 100644 --- a/src/main/native/MetallumNative.swift +++ b/src/main/native/MetallumNative.swift @@ -839,7 +839,8 @@ public func metallum_create_buffer( _ options: MTLResourceOptions ) -> UnsafeMutableRawPointer? { return autoreleasepool { - retainedPointer(device.makeBuffer(length: length, options: options)) + guard length > 0 else { return nil } + return retainedPointer(device.makeBuffer(length: length, options: options)) } } @@ -1223,6 +1224,14 @@ public func metallum_MTLRenderCommandEncoder_drawIndexedPrimitivesIndirect( _ drawCount: Int, _ stride: UInt64 ) { + if drawCount <= 0 { return } + let mul = Int(stride) * drawCount + if mul < 0 { return } + let needed = Int(indirectBufferOffset) + mul + if needed < 0 || needed > indirectBuffer.length { + return + } + if Int(indirectBufferOffset) < 0 { return } var offset = Int(indirectBufferOffset) for _ in 0..