-
Notifications
You must be signed in to change notification settings - Fork 2.1k
vk: refactor to use polymorphism for platform specialization #9398
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
2ac7709 to
fc45820
Compare
We refactor VulkanPlatform so that getSwapchainInstanceExtensions() and createVkSurfaceKHR() are virtual functions that are implemented by each platform. This will enable the use of polymorphism for platform-dependent bits.
fc45820 to
a9bb51e
Compare
| VulkanPlatform::SurfaceBundle VulkanPlatformWindows::createVkSurfaceKHR(void* nativeWindow, | ||
| VkInstance instance, uint64_t flags) const noexcept { | ||
| VkSurfaceKHR surface; | ||
| VkExtent2D extent; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extent is not initialized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I should've added more details. I thought this extent needs to be populated with values. Is this method intentionally returning { 0, 0 } for the second element? Then maybe this VkExtent2D extent; can be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's not needed for this implementation, but needed for others.
| VulkanPlatform::SurfaceBundle VulkanPlatformWindows::createVkSurfaceKHR(void* nativeWindow, | ||
| VkInstance instance, uint64_t flags) const noexcept { | ||
| VkSurfaceKHR surface; | ||
| VkExtent2D extent; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I should've added more details. I thought this extent needs to be populated with values. Is this method intentionally returning { 0, 0 } for the second element? Then maybe this VkExtent2D extent; can be removed.
We refactor VulkanPlatform so that getSwapchainInstanceExtensions() and createVkSurfaceKHR() are virtual functions that are implemented by each platform. This will enable the use of polymorphism for platform-dependent bits.