fix: Schedule deletion only if object was GC'd - allow dispose() fast path

[mrousavy]

This is a follow-up PR to #3704 - this implements point 1 in my Update comment.

It changes the deletion logic of Thread-confined resources (that's SkImage, SkPicture and SkSurface) to only schedule the call on a different Thread if the object is being GC'd, which is in Hermes happening on a different Thread.

This allows dispose() to dispose immediately instead of also scheduling deletion like it did previously.

So in other words; dispose() releases the object now (as we are on the same Thread), and if an object is passively being deleted by the Hermes GC, it's deletion is scheduled on a different Thread.

[mrousavy]

FWIW, we should use an actual Dispatcher to schedule the deletion, not a Queue.

The implementation (both before and after my PR) is prone to stale objects, e.g. if you create an SkImage, and never create another SkImage again - the last SkImage might still float in memory as the Dispatcher::processQueue() is never called, hence it never being deleted.

A solution (as a follow up PR) would be to use a real Dispatcher. Unfortunately there's no Dispatcher API available in JS environments - you can realistically only get a JS Thread Dispatcher from React Native, a UI Thread Dispatcher from the host platform, but then if it's a separate Thread (e.g. createWorkletRuntime(...), or VisionCamera Frame Processor) there's no Dispatcher you can use.
In Nitro there is a Dispatcher you can use for every Runtime, but Skia does not use Nitro.

[mrousavy]

We do need to add tests for this PR though, maybe it'd be a good idea to somehow test the VisionCamera + Skia integration on both our ends (e.g. CI tests) to make sure it's;

1. Working (not errors thrown)
2. Rendering actual content (no black screens)
3. Working stable over a few seconds (no memory leaks, no stalls)

I'll figure out how I can do this on my end in VisionCamera

[wcandillon]

This also makes sense to me 👍 Even if we were to keep the Dispatcher, we need to allow of immediate dispose of GPU bound resources. So potentially we could even merge this PR as an intermediary step.

[wcandillon]

This is great thank you 🙏

[wcandillon]

@mrousavy once we merge this we can also probably merge #3704 too depending on how the testing goes

[Copilot]

Pull request overview

Updates destruction semantics for thread-confined Skia JSI host objects so that dispose() can release the wrapped sk_sp<> immediately, while GC-driven finalization still defers deletion onto the object’s creation thread via Dispatcher.

Changes:

• Remove the releaseResources() overrides that always deferred deletion via Dispatcher.
• Update destructors for JsiSkImage, JsiSkPicture, and JsiSkSurface to only enqueue deferred deletion when the object wasn’t explicitly disposed.
• Keep Dispatcher::processQueue() calls in constructors (with a clarifying comment in JsiSkImage).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
packages/skia/cpp/api/JsiSkSurface.h	Make GC finalization schedule surface deletion; allow dispose() to release immediately.
packages/skia/cpp/api/JsiSkPicture.h	Same pattern applied to SkPicture.
packages/skia/cpp/api/JsiSkImage.h	Same pattern applied to SkImage, plus a comment clarifying queue processing.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[wcandillon]

@mrousavy I appear to have picked up a small regression using the video example with this PR, looking into it now.

[github-actions]

🎉 This PR is included in version 2.4.20 🎉

The release is available on:

• v2.4.20
• GitHub release

Your semantic-release bot 📦🚀

[mrousavy]

Hey what was the regression btw? Is this fixed?

[wcandillon]

yes it was a false alarm