Instanced rendering not working with Metal. #891
Comments
|
Looking at the Vulkan backend as a point of comparison it seems like Metal is not using the It seems like Kope has the beginnings of support for this with, |
|
Kope (I now call it Kore 3) only started drawing its first triangle recently, it will be a little while. |
|
I think I am a bit closer to a fix for the Metal backend in Kore 2, but I’m struggling with one thing. I don’t think the stride value for the memory layouts are being set correctly. Currently the stride value is set based on the MTLVertexFormat values Swift code from a working example: I suppose I could just create a swift app to output all of the stride values and create a switch based on the If you would prefer me to not update this issue with research, please let me know. |
|
We really don't need "MemoryLayout" to figure out the size of a float3. It's 12. kinc_g4_vertex_data_size does the right thing (and if you doubt it feel free to compare the values you need to whatever you get from "MemoryLayout". |
|
Putting together a small test for this, the value for MemoryLayout<SIMD3>.size is actually 16 not 12. This is also mentioned on page 28 of Apple’s documentation for Metal (screenshot attached). As per the documentation the float3 type uses the same size and alignment as float4. 
Metal rendering seems to work as expected in Kore 2 when instancing isn’t applied, and I am doing some experimenting with the pipeline for Metal specifically to see if I can get instanced rendering working as expected. You can run the following in Swift to see that MemoryLayout<SIMD3>.size does in fact return 16, which is inline with what Apple’s Metal documentation says. //: A UIKit based Playground for presenting user interface
import UIKit
import PlaygroundSupport
class MyViewController : UIViewController {
override func loadView() {
let view = UIView()
view.backgroundColor = .white
// vertex descriptor set with MemoryLayout<SIMD3<Float32>>.size vs 12
//let vertexDescriptor = MTLVertexDescriptor();
//vertexDescriptor.attributes[0].format = MTLVertexFormat.float3
//vertexDescriptor.attributes[0].offset = 0
//vertexDescriptor.attributes[0].bufferIndex = 0;
//vertexDescriptor.layouts[0].stride = MemoryLayout<SIMD3<Float32>>.size;
//vertexDescriptor.layouts[0].stepRate = 1;
//vertexDescriptor.layouts[0].stepFunction = MTLVertexStepFunction.perVertex;
let label = UILabel()
label.frame = CGRect(x: 150, y: 200, width: 200, height: 100)
label.lineBreakMode = .byWordWrapping
label.numberOfLines = 3
label.textColor = .black
let float_stride = MemoryLayout<Float>.stride * 3;
let float_size = MemoryLayout<Float>.size * 3
let stride_size = MemoryLayout<SIMD3<Float32>>.size;
label.text = "\(float_stride)\n\(float_size)\n\(stride_size)"
view.addSubview(label)
self.view = view
}
}
// Present the view controller in the Live View window
PlaygroundPage.current.liveView = MyViewController() |
|
Good find, I'll trust you on that. Feel free to update kinc_g4_vertex_data_size with an ifdef for Metal. |
|
Thanks. I think I have actually identified the main issue with the Metal rendering pipeline in that only the first vertex buffer is used in the Kore/Backends/Graphics5/Metal/Sources/kinc/backend/graphics5/commandlist.m.h Lines 169 to 171 in 188e29c If you add I had a go at rewriting this function for the Metal backend using MTLRenderCommandEncoder.setVertexbuffers but I am unsure how to calculate the range value it is expecting from the array of buffers. This might be of use if you or anyone else wants to finalize the metal implementation. |
|
range should simply be {0, count}. |
|
I think this problem has multiple parts and that I am closer to having it working. These are the things that seem to be causing the issue.
I believe the void kinc_g5_command_list_set_vertex_buffers(kinc_g5_command_list_t *list, struct kinc_g5_vertex_buffer **vertexBuffers, int *offsets_, int count) {
id<MTLRenderCommandEncoder> encoder = getMetalEncoder();
// Array to store Metal buffers and offsets in bytes
id<MTLBuffer> buffers[count];
NSUInteger offsets[count];
for (int i = 0; i < count; ++i) {
buffers[i] = (__bridge id<MTLBuffer>)vertexBuffers[i]->impl.mtlBuffer;
offsets[i] = (NSUInteger)(offsets_[i] * kinc_g5_vertex_buffer_stride(vertexBuffers[i]));
}
// Metal allows binding multiple buffers at once with a range of indices
[encoder setVertexBuffers:buffers offsets:offsets withRange:NSMakeRange(0, count)];
}I believe the generated shader will end up having to look like this as stage_in does not include per instance variables. I might be able to look at the compiled Vulkan shader to gain some insight as well. Note: I was wrong about this, stage_in seems to be smart enough to pass the per vertex and per instance data as needed. // shader_vert_main
#include <metal_stdlib>
#include <simd/simd.h>
using namespace metal;
struct shader_vert_main_out
{
float4 gl_Position [[position]];
};
// Per-vertex data structure
struct VertexData
{
float3 pos;
};
// Per-instance data structure
struct InstanceData
{
float3 off;
};
vertex shader_vert_main_out shader_vert_main(
device const VertexData* vertexData [[buffer(0)]],
device const InstanceData* instanceData [[buffer(1)]],
uint vertexID [[vertex_id]],
uint instanceID [[instance_id]]
)
{
shader_vert_main_out out = {};
// Access data from the appropriate buffers using IDs
float3 pos = vertexData[vertexID].pos;
float3 off = instanceData[instanceID].off;
// Use the vertex and instance data
out.gl_Position = float4(pos.x + off.x, pos.y + off.y, 0.5, 1.0);
out.gl_Position.z = (out.gl_Position.z + out.gl_Position.w) * 0.5; // Adjust clip-space for Metal
return out;
} |
|
I think I have it working! I was wrong about the requirement of changing the vertex shader format, it looks like stage_in is smart enough to recognize per vertex and per instance attributes. I'll clean up the code changes I've made to the Metal backend and submit them by the weekend at the latest. The only issue I think I still have to resolve is with void kinc_g5_command_list_set_vertex_constant_buffer(kinc_g5_command_list_t *list, struct kinc_g5_constant_buffer *buffer, int offset, size_t size) {
id<MTLBuffer> buf = (__bridge id<MTLBuffer>)buffer->impl._buffer;
id<MTLRenderCommandEncoder> encoder = getMetalEncoder();
[encoder setVertexBuffer:buf offset:offset atIndex:1]; // this can conflict with an index that is used by the fixed kinc_g5_command_list_set_vertex_buffers
}Screenshot of the latest metal test and some of the updated code:
|
|
Super cool! And no need to hurry. |
|
I have moved this into a commit, but require your input on one outstanding issue regarding vertex buffer indexes and the metal code generated by kinc/make. Thanks! |
Describe the bug
As per , instanced rendering is not implemented for Metal on macOS. I took a quick look at pipeline.m.h and it seems like the issue is that the code that iterates the pipeline->inputLayout array does not account for the correct number of input layouts, as other backends account for this. I have a patched version that is based on the Vulkan back-end that no longer triggers a run-time error, however if I compile and run the instanced drawing sample, only one triangle is drawn versus the expected number.
To Reproduce
Steps to reproduce the behavior:
Expected behavior
Instanced rendering should work as expected.
Additional context
I am fairly new to metal, but this is the partially modified code that no longer throws a runtime error, but it still only draws a single triangle, so I am missing something. I have included a note as to where the code modifications in this function begin and end.
The text was updated successfully, but these errors were encountered: