Skip to content

Commit

Permalink
Migrate off deprecated GrVkBackendContext fields (#53122)
Browse files Browse the repository at this point in the history
Context: https://g-issues.skia.org/issues/309785258

Skia would like to remove these deprecated fields, so this updates
Flutter's use of them to use the new ways.

Note that `VulkanWindow` seems to be unused (or no longer used?) since
there was a mistyped define (`SK_VUKLAN` -> `SK_VULKAN`) causing the
code I am fixing to never be run.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide] and the [C++,
Objective-C, Java style guides].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I added new tests to check the change I am making or feature I am
adding, or the PR is [test-exempt]. See [testing the engine] for
instructions on writing and running engine tests.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I signed the [CLA].
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[test-exempt]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
  • Loading branch information
kjlubick authored Jun 3, 2024
1 parent ea72558 commit d1365ee
Show file tree
Hide file tree
Showing 13 changed files with 76 additions and 81 deletions.
24 changes: 16 additions & 8 deletions shell/common/shell_test_platform_view_vulkan.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "third_party/skia/include/core/SkSurface.h"
#include "third_party/skia/include/gpu/ganesh/SkSurfaceGanesh.h"
#include "third_party/skia/include/gpu/ganesh/vk/GrVkDirectContext.h"
#include "third_party/skia/include/gpu/vk/VulkanExtensions.h"

#if OS_FUCHSIA
#define VULKAN_SO_PATH "libvulkan.so"
Expand Down Expand Up @@ -128,8 +129,15 @@ ShellTestPlatformViewVulkan::OffScreenSurface::OffScreenSurface(

bool ShellTestPlatformViewVulkan::OffScreenSurface::CreateSkiaGrContext() {
GrVkBackendContext backend_context;

if (!CreateSkiaBackendContext(&backend_context)) {
skgpu::VulkanExtensions no_extensions;
// For now, Skia crashes if fDeviceFeatures is set but fVkExtensions is not.
backend_context.fVkExtensions = &no_extensions;
VkPhysicalDeviceFeatures features;
// It may be tempting to put features into backend_context here
// and pass just backend_context into the below function, however the pointers
// for features are const, so we won't be able to update them.

if (!this->CreateSkiaBackendContext(&backend_context, &features)) {
FML_DLOG(ERROR) << "Could not create Skia backend context.";
return false;
}
Expand All @@ -153,16 +161,18 @@ bool ShellTestPlatformViewVulkan::OffScreenSurface::CreateSkiaGrContext() {
}

bool ShellTestPlatformViewVulkan::OffScreenSurface::CreateSkiaBackendContext(
GrVkBackendContext* context) {
GrVkBackendContext* context,
VkPhysicalDeviceFeatures* features) {
FML_CHECK(context);
FML_CHECK(features);
auto getProc = CreateSkiaGetProc(vk_);

if (getProc == nullptr) {
FML_DLOG(ERROR) << "GetProcAddress is null";
return false;
}

uint32_t skia_features = 0;
if (!logical_device_->GetPhysicalDeviceFeaturesSkia(&skia_features)) {
if (!logical_device_->GetPhysicalDeviceFeatures(features)) {
FML_DLOG(ERROR) << "Failed to get Physical Device features";
return false;
}
Expand All @@ -172,11 +182,9 @@ bool ShellTestPlatformViewVulkan::OffScreenSurface::CreateSkiaBackendContext(
context->fDevice = logical_device_->GetHandle();
context->fQueue = logical_device_->GetQueueHandle();
context->fGraphicsQueueIndex = logical_device_->GetGraphicsQueueIndex();
context->fMinAPIVersion = application_->GetAPIVersion();
context->fMaxAPIVersion = application_->GetAPIVersion();
context->fFeatures = skia_features;
context->fDeviceFeatures = features;
context->fGetProc = std::move(getProc);
context->fOwnsInstanceAndDevice = false;
context->fMemoryAllocator = memory_allocator_;

return true;
Expand Down
3 changes: 2 additions & 1 deletion shell/common/shell_test_platform_view_vulkan.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ class ShellTestPlatformViewVulkan : public ShellTestPlatformView {
sk_sp<GrDirectContext> context_;

bool CreateSkiaGrContext();
bool CreateSkiaBackendContext(GrVkBackendContext* context);
bool CreateSkiaBackendContext(GrVkBackendContext*,
VkPhysicalDeviceFeatures*);

FML_DISALLOW_COPY_AND_ASSIGN(OffScreenSurface);
};
Expand Down
12 changes: 5 additions & 7 deletions shell/platform/embedder/embedder_surface_vulkan.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
#include "flutter/vulkan/vulkan_skia_proc_table.h"
#include "include/gpu/GrDirectContext.h"
#include "include/gpu/vk/GrVkBackendContext.h"
#include "include/gpu/vk/GrVkExtensions.h"
#include "include/gpu/vk/VulkanExtensions.h"
#include "third_party/skia/include/gpu/ganesh/vk/GrVkDirectContext.h"

namespace flutter {
Expand Down Expand Up @@ -128,8 +128,8 @@ sk_sp<GrDirectContext> EmbedderSurfaceVulkan::CreateGrContext(
size_t device_extension_count,
const char** device_extensions,
ContextType context_type) const {
uint32_t skia_features = 0;
if (!device_.GetPhysicalDeviceFeaturesSkia(&skia_features)) {
VkPhysicalDeviceFeatures features;
if (!device_.GetPhysicalDeviceFeatures(&features)) {
FML_LOG(ERROR) << "Failed to get physical device features.";

return nullptr;
Expand All @@ -141,20 +141,18 @@ sk_sp<GrDirectContext> EmbedderSurfaceVulkan::CreateGrContext(
return nullptr;
}

GrVkExtensions extensions;
skgpu::VulkanExtensions extensions;

GrVkBackendContext backend_context = {};
backend_context.fInstance = instance;
backend_context.fPhysicalDevice = device_.GetPhysicalDeviceHandle();
backend_context.fDevice = device_.GetHandle();
backend_context.fQueue = device_.GetQueueHandle();
backend_context.fGraphicsQueueIndex = device_.GetGraphicsQueueIndex();
backend_context.fMinAPIVersion = version;
backend_context.fMaxAPIVersion = version;
backend_context.fFeatures = skia_features;
backend_context.fDeviceFeatures = &features;
backend_context.fVkExtensions = &extensions;
backend_context.fGetProc = get_proc;
backend_context.fOwnsInstanceAndDevice = false;

uint32_t vulkan_api_version = version;
sk_sp<skgpu::VulkanMemoryAllocator> allocator =
Expand Down
3 changes: 2 additions & 1 deletion shell/platform/fuchsia/flutter/vulkan_surface.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "third_party/skia/include/gpu/GrDirectContext.h"
#include "third_party/skia/include/gpu/ganesh/SkSurfaceGanesh.h"
#include "third_party/skia/include/gpu/ganesh/vk/GrVkBackendSurface.h"
#include "third_party/skia/include/gpu/vk/VulkanTypes.h"
#include "vulkan/vulkan_core.h"
#include "vulkan/vulkan_fuchsia.h"

Expand Down Expand Up @@ -394,7 +395,7 @@ bool VulkanSurface::SetupSkiaSurface(sk_sp<GrDirectContext> context,
const VkMemoryRequirements& memory_reqs) {
FML_CHECK(context != nullptr);

GrVkAlloc alloc;
skgpu::VulkanAlloc alloc;
alloc.fMemory = vk_memory_;
alloc.fOffset = 0;
alloc.fSize = memory_reqs.size;
Expand Down
12 changes: 5 additions & 7 deletions shell/platform/fuchsia/flutter/vulkan_surface_producer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
#include "third_party/skia/include/gpu/ganesh/vk/GrVkBackendSurface.h"
#include "third_party/skia/include/gpu/ganesh/vk/GrVkDirectContext.h"
#include "third_party/skia/include/gpu/vk/GrVkBackendContext.h"
#include "third_party/skia/include/gpu/vk/GrVkExtensions.h"
#include "third_party/skia/include/gpu/vk/GrVkTypes.h"
#include "third_party/skia/include/gpu/vk/VulkanExtensions.h"

namespace flutter_runner {

Expand Down Expand Up @@ -112,8 +112,8 @@ bool VulkanSurfaceProducer::Initialize() {
return false;
}

uint32_t skia_features = 0;
if (!logical_device_->GetPhysicalDeviceFeaturesSkia(&skia_features)) {
VkPhysicalDeviceFeatures features;
if (!logical_device_->GetPhysicalDeviceFeatures(&features)) {
FML_LOG(ERROR)
<< "VulkanSurfaceProducer: Failed to get physical device features.";

Expand All @@ -132,11 +132,9 @@ bool VulkanSurfaceProducer::Initialize() {
backend_context.fQueue = logical_device_->GetQueueHandle();
backend_context.fGraphicsQueueIndex =
logical_device_->GetGraphicsQueueIndex();
backend_context.fMinAPIVersion = application_->GetAPIVersion();
backend_context.fMaxAPIVersion = application_->GetAPIVersion();
backend_context.fFeatures = skia_features;
backend_context.fDeviceFeatures = &features;
backend_context.fGetProc = std::move(getProc);
backend_context.fOwnsInstanceAndDevice = false;
backend_context.fMemoryAllocator = memory_allocator_;

// The memory_requirements_2 extension is required on Fuchsia as the AMD
Expand All @@ -146,7 +144,7 @@ bool VulkanSurfaceProducer::Initialize() {
};
const int device_extensions_count =
sizeof(device_extensions) / sizeof(device_extensions[0]);
GrVkExtensions vk_extensions;
skgpu::VulkanExtensions vk_extensions;
vk_extensions.init(backend_context.fGetProc, backend_context.fInstance,
backend_context.fPhysicalDevice, 0, nullptr,
device_extensions_count, device_extensions);
Expand Down
12 changes: 5 additions & 7 deletions testing/test_vulkan_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#include "third_party/skia/include/core/SkSurface.h"
#include "third_party/skia/include/gpu/GrDirectContext.h"
#include "third_party/skia/include/gpu/ganesh/vk/GrVkDirectContext.h"
#include "third_party/skia/include/gpu/vk/GrVkExtensions.h"
#include "third_party/skia/include/gpu/vk/VulkanExtensions.h"
#include "vulkan/vulkan_core.h"

namespace flutter {
Expand Down Expand Up @@ -70,8 +70,8 @@ TestVulkanContext::TestVulkanContext() {
// For creating SkSurfaces from VkImages and snapshotting them, etc.
// ---------------------------------------------------------------------------

uint32_t skia_features = 0;
if (!device_->GetPhysicalDeviceFeaturesSkia(&skia_features)) {
VkPhysicalDeviceFeatures features;
if (!device_->GetPhysicalDeviceFeatures(&features)) {
FML_LOG(ERROR) << "Failed to get physical device features.";

return;
Expand All @@ -88,20 +88,18 @@ TestVulkanContext::TestVulkanContext() {
VK_MAKE_VERSION(1, 0, 0), application_->GetInstance(),
device_->GetPhysicalDeviceHandle(), device_->GetHandle(), vk_, true);

GrVkExtensions extensions;
skgpu::VulkanExtensions extensions;

GrVkBackendContext backend_context = {};
backend_context.fInstance = application_->GetInstance();
backend_context.fPhysicalDevice = device_->GetPhysicalDeviceHandle();
backend_context.fDevice = device_->GetHandle();
backend_context.fQueue = device_->GetQueueHandle();
backend_context.fGraphicsQueueIndex = device_->GetGraphicsQueueIndex();
backend_context.fMinAPIVersion = VK_MAKE_VERSION(1, 0, 0);
backend_context.fMaxAPIVersion = VK_MAKE_VERSION(1, 0, 0);
backend_context.fFeatures = skia_features;
backend_context.fDeviceFeatures = &features;
backend_context.fVkExtensions = &extensions;
backend_context.fGetProc = get_proc;
backend_context.fOwnsInstanceAndDevice = false;
backend_context.fMemoryAllocator = allocator;

GrContextOptions options =
Expand Down
27 changes: 0 additions & 27 deletions vulkan/vulkan_device.cc
Original file line number Diff line number Diff line change
Expand Up @@ -258,33 +258,6 @@ bool VulkanDevice::GetPhysicalDeviceFeatures(
return true;
}

bool VulkanDevice::GetPhysicalDeviceFeaturesSkia(uint32_t* sk_features) const {
if (sk_features == nullptr) {
return false;
}

VkPhysicalDeviceFeatures features;

if (!GetPhysicalDeviceFeatures(&features)) {
return false;
}

uint32_t flags = 0;

if (features.geometryShader) {
flags |= kGeometryShader_GrVkFeatureFlag;
}
if (features.dualSrcBlend) {
flags |= kDualSrcBlend_GrVkFeatureFlag;
}
if (features.sampleRateShading) {
flags |= kSampleRateShading_GrVkFeatureFlag;
}

*sk_features = flags;
return true;
}

std::vector<VkQueueFamilyProperties> VulkanDevice::GetQueueFamilyProperties()
const {
uint32_t count = 0;
Expand Down
3 changes: 0 additions & 3 deletions vulkan/vulkan_device.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,6 @@ class VulkanDevice {
[[nodiscard]] bool GetPhysicalDeviceFeatures(
VkPhysicalDeviceFeatures* features) const;

[[nodiscard]] bool GetPhysicalDeviceFeaturesSkia(
uint32_t* /* mask of GrVkFeatureFlags */ features) const;

[[nodiscard]] int ChooseSurfaceFormat(
const VulkanSurface& surface,
const std::vector<VkFormat>& desired_formats,
Expand Down
2 changes: 0 additions & 2 deletions vulkan/vulkan_native_surface.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ class VulkanNativeSurface {

virtual const char* GetExtensionName() const = 0;

virtual uint32_t GetSkiaExtensionName() const = 0;

virtual VkSurfaceKHR CreateSurfaceHandle(
VulkanProcTable& vk,
const VulkanHandle<VkInstance>& instance) const = 0;
Expand Down
5 changes: 1 addition & 4 deletions vulkan/vulkan_native_surface_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,10 @@ VulkanNativeSurfaceAndroid::~VulkanNativeSurfaceAndroid() {
}

const char* VulkanNativeSurfaceAndroid::GetExtensionName() const {
// https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_KHR_android_surface.html
return VK_KHR_ANDROID_SURFACE_EXTENSION_NAME;
}

uint32_t VulkanNativeSurfaceAndroid::GetSkiaExtensionName() const {
return kKHR_android_surface_GrVkExtensionFlag;
}

VkSurfaceKHR VulkanNativeSurfaceAndroid::CreateSurfaceHandle(
VulkanProcTable& vk,
const VulkanHandle<VkInstance>& instance) const {
Expand Down
2 changes: 0 additions & 2 deletions vulkan/vulkan_native_surface_android.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ class VulkanNativeSurfaceAndroid : public VulkanNativeSurface {

const char* GetExtensionName() const override;

uint32_t GetSkiaExtensionName() const override;

VkSurfaceKHR CreateSurfaceHandle(
VulkanProcTable& vk,
const VulkanHandle<VkInstance>& instance) const override;
Expand Down
48 changes: 37 additions & 11 deletions vulkan/vulkan_window.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "third_party/skia/include/core/SkSurface.h"
#include "third_party/skia/include/gpu/GrDirectContext.h"
#include "third_party/skia/include/gpu/ganesh/vk/GrVkDirectContext.h"
#include "third_party/skia/include/gpu/vk/VulkanExtensions.h"

namespace vulkan {

Expand Down Expand Up @@ -120,10 +121,13 @@ GrDirectContext* VulkanWindow::GetSkiaGrContext() {
}

bool VulkanWindow::CreateSkiaGrContext() {
#ifdef SK_VUKLAN
#ifdef SK_VULKAN
GrVkBackendContext backend_context;
VkPhysicalDeviceFeatures features;
skgpu::VulkanExtensions extensions;

if (!CreateSkiaBackendContext(&backend_context)) {
if (!this->CreateSkiaBackendContext(&backend_context, &features,
&extensions)) {
return false;
}

Expand All @@ -146,15 +150,21 @@ bool VulkanWindow::CreateSkiaGrContext() {
#endif // SK_VULKAN
}

bool VulkanWindow::CreateSkiaBackendContext(GrVkBackendContext* context) {
bool VulkanWindow::CreateSkiaBackendContext(
GrVkBackendContext* context,
VkPhysicalDeviceFeatures* features,
skgpu::VulkanExtensions* extensions) {
#ifdef SK_VULKAN
FML_CHECK(context);
FML_CHECK(features);
FML_CHECK(extensions);
auto getProc = CreateSkiaGetProc(vk_);

if (getProc == nullptr) {
return false;
}

uint32_t skia_features = 0;
if (!logical_device_->GetPhysicalDeviceFeaturesSkia(&skia_features)) {
if (!logical_device_->GetPhysicalDeviceFeatures(features)) {
return false;
}

Expand All @@ -163,15 +173,31 @@ bool VulkanWindow::CreateSkiaBackendContext(GrVkBackendContext* context) {
context->fDevice = logical_device_->GetHandle();
context->fQueue = logical_device_->GetQueueHandle();
context->fGraphicsQueueIndex = logical_device_->GetGraphicsQueueIndex();
context->fMinAPIVersion = application_->GetAPIVersion();
context->fExtensions = kKHR_surface_GrVkExtensionFlag |
kKHR_swapchain_GrVkExtensionFlag |
surface_->GetNativeSurface().GetSkiaExtensionName();
context->fFeatures = skia_features;
context->fMaxAPIVersion = application_->GetAPIVersion();
context->fDeviceFeatures = features;
context->fGetProc = std::move(getProc);
context->fOwnsInstanceAndDevice = false;
context->fMemoryAllocator = memory_allocator_;

constexpr uint32_t instance_extension_count = 2;
const char* instance_extensions[instance_extension_count] = {
// https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_KHR_surface.html
VK_KHR_SURFACE_EXTENSION_NAME,
surface_->GetNativeSurface().GetExtensionName(),
};
constexpr uint32_t device_extension_count = 1;
const char* device_extensions[device_extension_count] = {
// https://registry.khronos.org/vulkan/specs/1.3-extensions/man/html/VK_KHR_swapchain.html
VK_KHR_SWAPCHAIN_EXTENSION_NAME,
};
extensions->init(context->fGetProc, context->fInstance,
context->fPhysicalDevice, instance_extension_count,
instance_extensions, device_extension_count,
device_extensions);
context->fVkExtensions = extensions;
return true;
#else
return false;
#endif
}

sk_sp<SkSurface> VulkanWindow::AcquireSurface() {
Expand Down
4 changes: 3 additions & 1 deletion vulkan/vulkan_window.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@ class VulkanWindow {

bool CreateSkiaGrContext();

bool CreateSkiaBackendContext(GrVkBackendContext* context);
bool CreateSkiaBackendContext(GrVkBackendContext*,
VkPhysicalDeviceFeatures*,
skgpu::VulkanExtensions*);

[[nodiscard]] bool RecreateSwapchain();

Expand Down

0 comments on commit d1365ee

Please sign in to comment.