search

KhronosGroup / Vulkan-ExtensionLayer

Layer providing Vulkan features when native support is unavailable
https://vulkan.lunarg.com/doc/sdk/latest/linux/synchronization2_layer.html
Apache License 2.0
132 stars 43 forks source link

noop timeline semaphore when implementation has support #15

Open djdeath opened 4 years ago

djdeath commented 4 years ago

We should check the status of the extension support in the underlying implementation and return the function pointers of the next layer if it's available.

juan-lunarg commented 1 year ago

Discussed internally. Timeline semaphores were added for Window 7 support. Due to lack of interest/activity marking as wontfix.

cubanismo commented 1 year ago

Going through old emails, what does "discussed internally" mean? Within LunarG, or within Khronos? If within Khronos, I don't recall any such discussion.

What does "Timeline semaphores were added for Windows 7 support," mean? Unless it's a typo or something, that was definitely not the case. Timeline semaphores were added to Vulkan to give it a modern synchronization primitive similar to native OS and GPU primitives available on operating systems and GPUs newer than Windows 7. If applications shipping with this layer and running on those newer GPUs, operating systems, and drivers are falling back to some slower layered implementation that doesn't support external sharing just because no one at LunarG has interest in fixing the layer, that's not great. I would have expected such functionality to be a blocker for even shipping such a layer in the first place.

juan-lunarg commented 1 year ago

Within LunarG, or within Khronos?

Within LunarG.

What does "Timeline semaphores were added for Windows 7 support," mean?

I don't know what I meant either. I may have mixed up a conversation here? Apologies for the confusion.

charles-lunarg commented 1 year ago

Since it has been 3+ years since the layer has shipped, the need for improvements to the layer diminishes with each passing month. Sure, it would be good for this layer to gracefully handle hardware that does support the extension, but unless someone comes forth to make the necessary improvements, the issue will sit open. While I am fine with keeping the issue open to broadcast that this is a feature which should be done, the reality of people updating their drivers to versions that support the extension makes improvements to this layer less important as time goes by.

jeremyg-lunarg commented 1 year ago

@cubanismo Do you have or know of an application that is using this layer? This layer isn't part of the Vulkan SDK, so it is not shipping unless someone else is taking action to ship it. And if an application is shipping it, it is fruitless to update the code here unless we have a way to tell them to update.

Please also note that the newer layers in the repository do provide a pass through functionality if the underlying driver supports the extension.

jeremyg-lunarg commented 1 year ago

Additionally, an application could also only enable the layer if they detect the extension isn't supported by the driver. This isn't an ideal solution, but it is possible.

cubanismo commented 1 year ago

Do you have or know of an application that is using this layer?

I'm not clear how I'm supposed to be aware of the applications using a piece of code on the internet. I'm not, and I don't know if that means anyone is using it or not.

And if an application is shipping it, it is fruitless to update the code here unless we have a way to tell them to update.

So software isn't worth updating because no one is capable of tracking upstream updates themselves? This is a pretty defeatist point to try to make.

Please also note that the newer layers in the repository do provide a pass through functionality if the underlying driver supports the extension.

Then why is it so unreasonable to backport such a thing to this layer?

Additionally, an application could also only enable the layer if they detect the extension isn't supported by the driver. This isn't an ideal solution, but it is possible.

Exactly, this is not ideal. Presumably this is why other layers provide this capability as part of the layer.

KarenGhavam-lunarG commented 1 year ago

Then why is it so unreasonable to backport such a thing to this layer?

It is not unreasonable nor was it indicated as unreasonable to backport.

The issue is 2.5 years old with no other comments from users indicating it was a high priority issue. The original developer of the layer as documented in the README.md hasn't seem motivated to enhance the layer as well.

Our resources here have many priorities we are juggling and enhancing this layer isn't on the priority list currently. If you feel passionate about it, PRs are accepted. Do note that there are no CI regression tests implemented for this layer as well and it would need some tests.