Add GRVkBackendContext.DeviceLost managed binding (#4601) - #4655
Open
ramezgerges wants to merge 1 commit into
Open
ramezgerges wants to merge 1 commit into
ramezgerges wants to merge 1 commit into
Conversation
9 tasks
Contributor
📦 Try the packages from this PRWarning Do not run these scripts without first reviewing the code in this PR. Step 1 — Download the packages bash / macOS / Linux: curl -fsSL https://raw.githubusercontent.com/mono/SkiaSharp/main/scripts/get-skiasharp-pr.sh | bash -s -- 4655PowerShell / Windows: iex "& { $(irm https://raw.githubusercontent.com/mono/SkiaSharp/main/scripts/get-skiasharp-pr.ps1) } 4655"Step 2 — Add the local NuGet source dotnet nuget add source ~/.skiasharp/hives/pr-4655/packages --name skiasharp-pr-4655More options
Or download manually from Azure Pipelines — look for the Remove the source when you're done: dotnet nuget remove source skiasharp-pr-4655 |
Contributor
|
Hey there @@ramezgerges! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
ramezgerges
force-pushed
the
dev/issue-4601-vulkan-device-lost-callback
branch
from
August 4, 2026 17:42
b71a826 to
87efc84
Compare
Contributor
|
/azp run |
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
1 similar comment
|
Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command. |
ramezgerges
force-pushed
the
dev/issue-4601-vulkan-device-lost-callback
branch
3 times, most recently
from
September 14, 2026 18:54
feaf6d2 to
6778b92
Compare
Wraps the new gr_vk_device_lost_handler_new / _delete C API so managed callers can register a VK_ERROR_DEVICE_LOST callback on both Ganesh (GRVkBackendContext.DeviceLost) and Graphite (SKGraphiteVkBackendContext.DeviceLost). Skia stores the callback pointer non-owning on the Context, so the BackendContext must outlive the GRContext / SKGraphiteContext built from it — same rule as GetProcedureAddress today; document it. * new GRVkDeviceLostDelegate(GRVkDeviceLostInfo info) delegate. The payload carries Skia's description plus the full VK_EXT_device_fault output — address-info array, vendor-info array and vendor binary blob. The proxy in DelegateProxies snapshots all of it into managed arrays before invoking the handler, because the native storage is Skia's transient std::string / std::vector memory and is only valid for the duration of the call; handlers may therefore stash the info object past the callback's return. The proxy never throws across the FFI boundary (device-lost fires on a driver-owned thread; an unhandled managed exception would tear down the process), and a null description is normalized to string.Empty. * the generator emits gr_vk_device_fault_vendor_info_t's fixed char[256] slot as a bare pointer, so the proxy walks the vendor-info array with byte* arithmetic over the known 272-byte stride rather than trusting the generated struct's layout. * DeviceLost property on both backend contexts: setter allocates the native bridge, pins the delegate, and stashes both; Dispose deletes the bridge and frees the pin. Assigning a new delegate atomically tears down the previous state. * ToNative() plumbs the native bridge handle into fDeviceLostHandler. * Tests: tests/Tests/SkiaSharp/GRVkDeviceLostProxyTest.cs drives DelegateProxies.GRVkDeviceLostProxy directly with synthesized native payloads to cover each marshalling path; tests/VulkanTests/ GRVkDeviceLostTest.cs covers the wiring — property round-trip on both backend contexts, Ganesh + Graphite Context construction with a handler surviving Snap/Insert/Submit/Dispose, and repeated setter assignment neither leaking nor double-freeing. Every test in GRVkDeviceLostTest goes through GpuPolicy.RequireOrSkip, including the three that never build a Context. The DeviceLost setter installs a native bridge via gr_vk_device_lost_handler_new, which only exists in a libSkiaSharp compiled with SK_VULKAN, so on a host that does not build the Vulkan backend (Apple) those three would otherwise fail on a null handle instead of skipping with the rest of the suite. Actually forcing VK_ERROR_DEVICE_LOST from managed code isn't realistic in CI; end-to-end firing is verifiable only on hardware that experiences a TDR. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ramezgerges
force-pushed
the
dev/issue-4601-vulkan-device-lost-callback
branch
from
September 14, 2026 23:13
6778b92 to
ba5bd29
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #4601. Exposes Skia's Vulkan device-lost callback on
GRVkBackendContextandSKGraphiteVkBackendContextso managed callers can register aVK_ERROR_DEVICE_LOSThandler — the intended channel for reacting to TDRs / driver resets. Wraps the new C API added in the companionmono/skiaPR.Related issues
Fixes #4601
Required skia PR
Requires mono/skia#338
Areas affected
binding/)externals/skia/src/c,include/c)Changes
Public API
ABI-stable additive changes only.
Behavior
DeviceLostnow receive a full-fidelity callback on the driver-owned thread whenever Skia detectsVK_ERROR_DEVICE_LOST. The payload includes Skia's description string plus the completeVK_EXT_device_faultoutput (address-info array, vendor-info array, vendor binary blob).GRVkDeviceLostInfois a snapshot — the marshalling proxy copies everything out of Skia's transientstd::string/std::vectorstorage into managed arrays, so handlers may safely stash the object past the callback's return (e.g. to log later, dispatch to a UI thread).GRVkBackendContext/SKGraphiteVkBackendContextmust outlive theGRContext/SKGraphiteContextbuilt from it. Skia holds the callback pointer non-owning on the Context and calls it later; the pin lives on the backend context. Same rule that already applies toGetProcedureAddress. Standardusingblock ordering handles this naturally.VK_EXT_device_faultis not enabled by the app, the address/vendor arrays and binary data are empty; the description is still populated.DeviceLostnull get today's behavior.Testing
Two test files:
tests/Tests/SkiaSharp/GRVkDeviceLostProxyTest.cs— 5 proxy-level unit tests that invokeDelegateProxies.GRVkDeviceLostProxydirectly with synthesizedGRVkDeviceLostInfoNativepayloads, verifying each marshalling path: description, address-info array, the manual byte-offset walk for vendor infos (the generator emitschar[256]as a pointer, so the proxy walks the array withbyte*arithmetic), vendor binary blob copy, and exception swallowing.tests/VulkanTests/GRVkDeviceLostTest.cs— 5 Vulkan integration tests: property round-trip on both backend contexts, Ganesh + Graphite Context construction with a handler survives full Snap/Insert/Submit/Dispose cycle, repeated setter assignment doesn't leak or double-free.Re-verified after rebasing onto
main(post chrome/m154 and post the per-type binding split in #5056), on Windows x64 with natives rebuilt from source:mainbaselineSkiaSharp.Tests)The
+11core passes are the 5 new proxy tests plus 6 newApiTeststheory cases (3 theories x the 2 new P/Invokes). Core failures are byte-identical to the baseline.The failures on that machine are environmental, not regressions. That box has no Vulkan ICD and no usable OpenGL context, so the pre-existing GPU failures reproduce identically on unmodified
main: all 169 core failures areSystem.Exception : DC does not have extensions, and all Vulkan failures arevkCreateInstance failed/SharpVk.IncompatibleDriverException. Per the GPU policy these are correctly failures rather than skips, and this change does not add or widen any skip.Confirmed on real GPU hardware
The suite was then re-run on a machine with a working Vulkan driver (Intel UHD Graphics 620, driver 31.0.101.2137), in an interactive console session:
SkiaSharp.Vulkan.Tests(incl. all 5 new device-lost tests)All five
GRVkDeviceLostTestcases pass against a real Vulkan device, including the two that build a liveGRContext/SKGraphiteContextwith a handler installed and take it through a full Snap/Insert/Submit/Dispose cycle. Nothing in this change is left unverified by the CPU-only run.Actually firing
VK_ERROR_DEVICE_LOSTstill requires hardware experiencing a TDR, which remains out of reach in CI and on a healthy dev machine — that specific path is covered by the proxy tests, which drive the marshalling directly.Actually firing
VK_ERROR_DEVICE_LOSTrequires hardware experiencing a TDR — unreachable in CI. The proxy tests cover the marshalling; the integration tests cover the wiring.Checklist
Changesabove lists all public API and behavioral changes (or "None.")mono/skiaPR linked above and bindings regenerated