fix(💣): fix thread boundaries in GPU bound resources #3407
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes thread boundaries issues for GPU-bound resources in React Native Skia, specifically addressing crashes when GPU resources are used across different threads on iOS and Android Ganesh rendering backend.
- Replaces
ThreadSafeDeletionsystem with a newDispatcherpattern for managing cross-thread GPU resource cleanup - Ensures GPU resources (Images, Pictures, Surfaces) are properly cleaned up on their creation threads
- Updates texture creation to avoid cross-thread GPU resource sharing issues
Reviewed Changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/skia/src/sksg/Recorder/ReanimatedRecorder.ts | Adds documentation comment about GPU resource thread limitations |
| packages/skia/src/renderer/Offscreen.tsx | Removes conditional texture conversion logic and always creates non-texture images |
| packages/skia/src/external/reanimated/textures.tsx | Refactors image-to-texture conversion to avoid cross-thread GPU resource issues |
| packages/skia/cpp/api/recorder/RNRecorder.h | Adds platform context to recorder for proper resource cleanup |
| packages/skia/cpp/api/recorder/JsiRecorder.h | Initializes recorder's context reference |
| packages/skia/cpp/api/recorder/Drawings.h | Adds destructors for GPU resource commands to ensure main thread cleanup |
| packages/skia/cpp/api/JsiSkThreadSafeDeletion.h | Removes old thread-safe deletion system |
| packages/skia/cpp/api/JsiSkSurface.h | Replaces ThreadSafeDeletion with Dispatcher pattern |
| packages/skia/cpp/api/JsiSkPicture.h | Adds Dispatcher-based cleanup for Picture objects |
| packages/skia/cpp/api/JsiSkImage.h | Updates Image cleanup and memory pressure calculation |
| packages/skia/cpp/api/JsiSkDispatcher.h | New thread-local dispatcher system for managing cross-thread operations |
| packages/skia/cpp/api/JsiSkDispatcher.cpp | Implementation file for Dispatcher thread-local storage |
| packages/skia/cpp/api/JsiSkCanvas.h | Adds GPU context validation for image drawing operations |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
packages/skia/cpp/api/JsiSkImage.h
Outdated
| return image->imageInfo().computeMinByteSize(); | ||
| } | ||
| } | ||
| return 0; |
There was a problem hiding this comment.
Inconsistent indentation using tabs and spaces. The code should use consistent indentation throughout - either all spaces or all tabs according to the project's style guide.
| return 0; | |
| return 0; |
Copilot uses AI. Check for mistakes.
packages/skia/cpp/api/JsiSkCanvas.h
Outdated
| void validateImageForDrawing(jsi::Runtime &runtime, const sk_sp<SkImage> image) { | ||
| #if !defined(SK_GRAPHITE) | ||
| auto ctx = getContext()->getDirectContext(); | ||
| if (!ctx) { | ||
| throw jsi::JSError(runtime, "No GPU context available"); | ||
| } | ||
| if (image && !image->isValid(ctx->asRecorder())) { | ||
| throw jsi::JSError(runtime, "image used drawImage() does not belong in this context"); | ||
| } | ||
| #endif | ||
| } |
There was a problem hiding this comment.
Inconsistent indentation mixing tabs and spaces. The function body should use consistent indentation throughout.
Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 22 out of 23 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
🎉 This PR is included in version 2.2.20 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
fixes #3390