bkaradzic / bgfx

Cross-platform, graphics API agnostic, "Bring Your Own Engine/Framework" style rendering library.
https://bkaradzic.github.io/bgfx/overview.html
BSD 2-Clause "Simplified" License
14.59k stars 1.92k forks source link

fix iOS 16.x version failed to pass MTLBinding's arg.used check #3271

Closed junjie020 closed 3 months ago

junjie020 commented 3 months ago

It failed to pass this check:

https://github.com/bkaradzic/bgfx/blob/4cb7b7138a3f4b7ef8f1d3764b3fc9529bd01c61/src/renderer_mtl.mm#L1956-L1957

on:

My test devices are : iPhone X with iOS version 16.7.6 and iPhone 8 Plus with iOS version 16.7.6 iPhone 15 pro with iOS version 17.2/17.4 iPhone SE3 with iOS version 17.2/17.4

image

The option Queue Debugging enable or not will affect MTLBinding's used value. If disable, it will always false; ortherwise true.

But in iOS 17.x, this value(MTLBinding.used) are always true.

This problem also relate to this issue:

https://github.com/bkaradzic/bgfx/pull/3116

I write this code to locate this problom:

bool findArgument(id<MTLBinding> b, NSArray<MTLArgument*>* arguments){
    const char* name = utf8String(b.name);
    for (MTLArgument *a in arguments){
        if (a.active && 0 == strcmp(utf8String(a.name), name)){
            return true;
        }
    }
    return false;
}

void checkArguments(NSArray<id<MTLBinding>> *bindings, NSArray<MTLArgument*>* arguments) {
    for (id<MTLBinding> b in bindings){
        assert(findArgument(b, arguments));
    }
}
void checkRelfectionArguments(RenderPipelineReflection reflection){
    checkArguments(reflection.vertexBindings, reflection.vertexArguments);
    checkArguments(reflection.fragmentBindings, reflection.fragmentArguments);
}

add this code before this line:

checkRelfectionArguments(reflection);

https://github.com/bkaradzic/bgfx/blob/master/src/renderer_mtl.mm#L2361

I found that reflection.vertexBindings / reflection.fragmentBindings is equal to reflection.vertexArgumens/reflection.fragmentArguments's MTLArgument with active is true

So, I think it's no need to check MTLBinding's used in ProcessArguments function.

junjie020 commented 3 months ago

After few more test, this assumption is wrong:

So, I think it's no need to check MTLBinding's used in ProcessArguments function.

That test is: create a metal shader with a buffer, but not use it in this shader. That not use value in MTLBinding.used will return false, also MTLArgument.active is the same value in iOS 17.x version.

That's my fault.