Skip to content

Commit 2727f13

Browse files
mrousavywcandillon
andauthored
fix(🗑️): fix bug in explicit management of GPU bound resources (#3705)
Co-authored-by: William Candillon <wcandillon@gmail.com>
1 parent e50000b commit 2727f13

3 files changed

Lines changed: 40 additions & 55 deletions

File tree

packages/skia/cpp/api/JsiSkImage.h

Lines changed: 14 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -289,31 +289,26 @@ class JsiSkImage : public JsiSkWrappingSkPtrHostObject<SkImage> {
289289
std::move(image)) {
290290
// Get the dispatcher for the current thread
291291
_dispatcher = Dispatcher::getDispatcher();
292-
// Process any pending operations
292+
// Process any pending operations (e.g. deletions of previous resources)
293293
_dispatcher->processQueue();
294294
}
295295

296-
protected:
297-
void releaseResources() override {
298-
// Queue deletion on the creation thread if needed
299-
auto image = getObjectUnchecked();
300-
if (image && _dispatcher) {
301-
_dispatcher->run([image]() {
302-
// Image will be deleted when this lambda is destroyed
303-
});
304-
}
305-
// Clear the object
306-
JsiSkWrappingSkPtrHostObject<SkImage>::releaseResources();
307-
}
308-
309296
public:
310297
~JsiSkImage() override {
311-
// If already disposed, resources were already cleaned up
312-
if (isDisposed()) {
313-
return;
298+
if (!isDisposed()) {
299+
// This JSI Object is being deleted from a GC, which might happen
300+
// on a separate Thread. GPU resources (like SkImage) must be deleted
301+
// on the same Thread they were created on, so in this case we schedule
302+
// deletion to run on the Thread this Object was created on.
303+
auto image = getObjectUnchecked();
304+
if (image && _dispatcher) {
305+
_dispatcher->run([image]() {
306+
// Image will be deleted when this lambda is destroyed, on the
307+
// original Thread.
308+
});
309+
}
310+
releaseResources();
314311
}
315-
// Use the same cleanup path as dispose()
316-
releaseResources();
317312
}
318313

319314
size_t getMemoryPressure() const override {

packages/skia/cpp/api/JsiSkPicture.h

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -34,27 +34,22 @@ class JsiSkPicture : public JsiSkWrappingSkPtrHostObject<SkPicture> {
3434
_dispatcher->processQueue();
3535
}
3636

37-
protected:
38-
void releaseResources() override {
39-
// Queue deletion on the creation thread if needed
40-
auto picture = getObjectUnchecked();
41-
if (picture && _dispatcher) {
42-
_dispatcher->run([picture]() {
43-
// Picture will be deleted when this lambda is destroyed
44-
});
45-
}
46-
// Clear the object
47-
JsiSkWrappingSkPtrHostObject<SkPicture>::releaseResources();
48-
}
49-
5037
public:
5138
~JsiSkPicture() override {
52-
// If already disposed, resources were already cleaned up
53-
if (isDisposed()) {
54-
return;
39+
if (!isDisposed()) {
40+
// This JSI Object is being deleted from a GC, which might happen
41+
// on a separate Thread. GPU resources (like SkPicture) must be deleted
42+
// on the same Thread they were created on, so in this case we schedule
43+
// deletion to run on the Thread this Object was created on.
44+
auto picture = getObjectUnchecked();
45+
if (picture && _dispatcher) {
46+
_dispatcher->run([picture]() {
47+
// Picture will be deleted when this lambda is destroyed, on the
48+
// original Thread.
49+
});
50+
}
51+
releaseResources();
5552
}
56-
// Use the same cleanup path as dispose()
57-
releaseResources();
5853
}
5954

6055
JSI_HOST_FUNCTION(makeShader) {

packages/skia/cpp/api/JsiSkSurface.h

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -45,27 +45,22 @@ class JsiSkSurface : public JsiSkWrappingSkPtrHostObject<SkSurface> {
4545
_dispatcher->processQueue();
4646
}
4747

48-
protected:
49-
void releaseResources() override {
50-
// Queue deletion on the creation thread if needed
51-
auto surface = getObjectUnchecked();
52-
if (surface && _dispatcher) {
53-
_dispatcher->run([surface]() {
54-
// Surface will be deleted when this lambda is destroyed
55-
});
56-
}
57-
// Clear the object
58-
JsiSkWrappingSkPtrHostObject<SkSurface>::releaseResources();
59-
}
60-
6148
public:
6249
~JsiSkSurface() override {
63-
// If already disposed, resources were already cleaned up
64-
if (isDisposed()) {
65-
return;
50+
if (!isDisposed()) {
51+
// This JSI Object is being deleted from a GC, which might happen
52+
// on a separate Thread. GPU resources (like SkSurface) must be deleted
53+
// on the same Thread they were created on, so in this case we schedule
54+
// deletion to run on the Thread this Object was created on.
55+
auto surface = getObjectUnchecked();
56+
if (surface && _dispatcher) {
57+
_dispatcher->run([surface]() {
58+
// Surface will be deleted when this lambda is destroyed, on the
59+
// original Thread.
60+
});
61+
}
62+
releaseResources();
6663
}
67-
// Use the same cleanup path as dispose()
68-
releaseResources();
6964
}
7065

7166
EXPORT_JSI_API_TYPENAME(JsiSkSurface, Surface)

0 commit comments

Comments
 (0)