Skip to content

Conversation

@billhollings
Copy link
Contributor

MoltenVK now supports the VK_EXT_metal_surface extension. This PR modifies the cube demo to use that extension to create the surface, instead of the obsolete VK_MVK_macos_surface & VK_MVK_ios_surface extensions.

@CLAassistant
Copy link

CLAassistant commented Jul 3, 2019

CLA assistant check
All committers have signed the CLA.

@billhollings
Copy link
Contributor Author

Per the request on my related VulkanSamples PR, I've squashed the commits here into one.

Copy link
Contributor

@TonyBarbour TonyBarbour left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. @jeremyk-lunarg does it pass your testing?

@billhollings
Copy link
Contributor Author

Ping. Can I get this merged so that I can update MoltenVK to use it?

@jeremyk-lunarg
Copy link
Contributor

Getting the following error when building on my Mac Laptop:

[ 24%] Linking C executable vkcube.app/Contents/MacOS/vkcube
Undefined symbols for architecture x86_64:
  "_demo_main", referenced from:
      -[DemoViewController viewDidLoad] in DemoViewController.m.o
  "_demo_run", referenced from:
      _DisplayLinkCallback in DemoViewController.m.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [cube/vkcube.app/Contents/MacOS/vkcube] Error 1
make[1]: *** [cube/CMakeFiles/vkcube.dir/all] Error 2
make: *** [all] Error 2

@billhollings
Copy link
Contributor Author

@jeremy-lunarg

What version of MoltenVK are you using?

There is a coordination component here...in that the cube demo is being modified to use a different surface extension...which is only supported in MoltenVK repository as of July 3. The MoltenVK in the last SDK won't work.

@jeremyk-lunarg
Copy link
Contributor

@billhollings
Tried with master and aa6a10570a4ce0077120d49dd24c70fd5f5d8481.

Getting the following error during build still:

[ 24%] Linking C executable vkcube.app/Contents/MacOS/vkcube
Undefined symbols for architecture x86_64:
  "_vkCreateMetalSurfaceEXT", referenced from:
      _demo_init_vk_swapchain in DemoViewController.m.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [cube/vkcube.app/Contents/MacOS/vkcube] Error 1
make[1]: *** [cube/CMakeFiles/vkcube.dir/all] Error 2
make: *** [all] Error 2

@jeremyk-lunarg
Copy link
Contributor

Might be best if you update the known_good.json with the MoltenVK commit that works for you and add that change to this PR as well.

@billhollings
Copy link
Contributor Author

@jeremy-lunarg

The MoltenVK commit that you specified compiles against MoltenVK in the MoltenVK cube demo, which builds against MoltenVK directly.

Does the build process in Vulkan-Tools build against MoltenVK directly, or the Loader? And does the Loader support vkCreateMetalSurfaceEXT() from the VK_EXT_metal_surface extension?

@jeremyk-lunarg
Copy link
Contributor

Vulkan-Tools builds against the Loader.

@lenny-lunarg, you would probably be able to answer @billhollings question better than I could.

extensions with VK_EXT_metal_surface extension.
Make cube demo DemoViewController.m compatible with VK_EXT_metal_surface extension.
Update version of MoltenVK in scripts/known_good.json.
Run clang-format.
@lenny-lunarg
Copy link
Contributor

The loader does not currently support the VK_EXT_metal_surface extension. Furthermore, surfaces are a little bit of a special case within the loader.

I can add loader support for this extension, but I probably can't get around to that until the end of the week. On top of that, you may need to make changes to moltenvk to make them work together nicely. A driver needs to implement version 3 of the interface for the loader to allow it to implement the create surface functions.

Of course, since I don't have easy access to a machine running macOS, I might need some help to get this loader change up and running.

@billhollings
Copy link
Contributor Author

Thanks, @lenny-lunarg .

As I understand the Loader versioning, MoltenVK already supports version 5:

MVK_PUBLIC_SYMBOL VkResult vk_icdNegotiateLoaderICDInterfaceVersion(
	uint32_t*                                   pSupportedVersion) {

	MVKTraceVulkanCallStart();

	// This ICD expects to be loaded by a loader of at least version 5.
	VkResult rslt = VK_SUCCESS;
	if (pSupportedVersion && *pSupportedVersion >= 5) {
		*pSupportedVersion = 5;
	} else {
		rslt = VK_ERROR_INCOMPATIBLE_DRIVER;
	}
	MVKTraceVulkanCallEnd();
	return rslt;
}

...and it certainly has always supported the other surface extensions: VK_MVK_macos_surface & VK_MVK_ios_surface.

But let me know if there is anything else we can add to MoltenVK to help with this.

And I'm happy to help with testing on macOS if you need.

@billhollings
Copy link
Contributor Author

Just a note....

On macOS, the Cube demo should be run with the --use_staging CLI argument, because macOS does not support backing textures with host-coherent memory.

@jeremyk-lunarg
Copy link
Contributor

@billhollings, now that we have support for the metal surface in the loader, this can probably be retested and merged after you update it with resolutions for the conflicts.

@jeremyk-lunarg
Copy link
Contributor

Because this PR is so far behind, I cherry-picked the commit onto a new branch and created a new PR.

Please refer to PR #297 for future discussion on this change.
Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants