KhronosGroup / Vulkan-Ecosystem

Public repository for Vulkan Ecosystem issues
Apache License 2.0
132 stars 15 forks source link

Why are Loader and Validation Layer Repos Merged? #22

Closed natduca closed 6 years ago

natduca commented 6 years ago

Vulkan Loader and validation layers seem separate to me.

Validation Layers are a critical part of developer productivity when working with Vulkan. But its just "a" layer, abeit an important part.

Loader is a much more intrinsic part of vulkan and defines how we bring layers up.

They are in one repo for historic reasons. Maybe they should be separate repos? It strikes me that merged as they are, we talk a lot about validation issues because those are things developers are passionate about, but forget to talk about loader issues, of which there are many and, arguably just as important. By getting these into separate repos, my instinct says we can start talking about these as distinct things both needing our time and energy.

KarenGhavam-lunarG commented 6 years ago

It is correct that they are combined for historical reasons. It is on LunarG's TO DO list to separate the loader into a separate repo. It was convenient to have the loader and layers together because of shared header files. It is likely that we will create a repo for headers, one for the loader, and one for layers.

karl-lunarg commented 6 years ago

This is a place where some user and WG feedback would be appreciated. As @KarenGhavam-lunarG suggested, LunarG is considering splitting the repos.

Aside from splitting along functionality boundaries, there is also another motivation for the split from other repos that are currently including the Vulkan-LoaderAndValidationLayers repo as a submodule. Many of these include LVL just to get a handful of files, such as the header files. It seems wasteful to include all of LVL just to get these few files.

One proposal for the split might be:

This split orphans a few other misc components:

I don't know where to put these at the moment, but something like

or something like this might be good.

There are a huge number of execution issues related to doing this.

plohrmann commented 6 years ago

Perhaps vulkaninfo and the demos can go into KhronosGroup/Vulkan-Ecosystem? That may be a better option than creating another repository.

nsubtil commented 6 years ago

I'm not sure we want to start hosting code in this repo, since that would mean we have to mix ecosystem issues with demo bugs in the issue tracker.

plohrmann commented 6 years ago

Okay I agree with @nsubtil - ignore my previous suggestion. Sorry!

mark-lunarg commented 6 years ago

While they are labelled as demos, cube, cubepp, and smoke effectively serve as validation layer tests, so keeping them with the other tests would not be unreasonable.

lenny-lunarg commented 6 years ago

There's also some stuff (mainly the NSIS script) in the windowsRuntimeInstaller directory which would need a home. We could make it private with the rest of the SDK build stuff or we could leave it with the loader. If we do leave it in the loader repo, then it would make sense to keep vulkaninfo with the loader, since the runtime just installs the loader and vulkaninfo on a Windows system.

karl-lunarg commented 6 years ago

Maybe we're getting off into the weeds here too much, or maybe it's worth capturing these ideas here.

For both of the previous comments, I think it is better to make a logical partitioning of these components, rather than optimizing for building or testing workflows.

For testing layers, the current layer test suite should probably remain packaged with the layers as an accompanying unit test. But any sort of automated or scripted layer testing process (e.g., CI) should go ahead and make the effort to fetch other repos and build/run these tests. This shouldn't be that hard if the cube/smoke programs were in a Vulkan-Utils or Vulkan-Extras repo. And I'd hope we'd expand this sort of testing to other Vulkan samples repos out there, for the apps that are CI-friendly. Finally, I don't think people would expect to find things like the cube demo in a layer testing location.

KarenGhavam-lunarG commented 6 years ago

After discussion in the ecosystem forum it was determined that this issue should be in the Vulkan-LoaderAndValidation Layers repo. Hence a new issue was created (#2458) and this issue will be closed in this project.