Skip to content

Conversation

@poweifeng
Copy link
Contributor

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.

@poweifeng poweifeng added the internal Issue/PR does not affect clients label Nov 3, 2025
@poweifeng poweifeng force-pushed the pf/vk-refactor-platform branch from 2ac7709 to fc45820 Compare November 3, 2025 23:34
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.
@poweifeng poweifeng force-pushed the pf/vk-refactor-platform branch from fc45820 to a9bb51e Compare November 3, 2025 23:38
VulkanPlatform::SurfaceBundle VulkanPlatformWindows::createVkSurfaceKHR(void* nativeWindow,
VkInstance instance, uint64_t flags) const noexcept {
VkSurfaceKHR surface;
VkExtent2D extent;
Copy link
Contributor

Choose a reason for hiding this comment

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

extent is not initialized.

Copy link
Contributor

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.

Copy link
Contributor Author

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;
Copy link
Contributor

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.

@pixelflinger pixelflinger self-requested a review November 4, 2025 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal Issue/PR does not affect clients

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants