Skip to content

Commit

Permalink
#1626: Fixing access to Skia Values from UI Thread (#1639)
Browse files Browse the repository at this point in the history
* Fixing access to Skia Values from UI Thread

Previously the following would crash:

```ts
const x = useValue(50);
  const y = useValue(50);

  const gesture = useMemo(
    () =>
      Gesture.Pan().onChange(({ changeX, changeY }) => {
        x.current += changeX;
        y.current += changeY;
      }),
    []
  );
```

This was caused by the Skia Values storing its value in a JS object for fast access causing a crash when trying to write to this value from another runtime (REA / Main UI Thread)

This commit updates the Skia Value to use the JsiValue class instead - which unwraps / wraps the value to native primitives. There might be a small overhead doing this, but I've not been able to measure such a thing.

Fixes Skia value can't update on UI thread #1626

* Fix lint

* Fix ci

---------

Co-authored-by: William Candillon <[email protected]>
  • Loading branch information
chrfalch and wcandillon authored Jun 29, 2023
1 parent 4f2d7c3 commit a69ddc8
Showing 1 changed file with 6 additions and 7 deletions.
13 changes: 6 additions & 7 deletions package/cpp/rnskia/values/RNSkReadonlyValue.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
#include <jsi/jsi.h>

#include "JsiSkHostObjects.h"
#include "JsiValueWrapper.h"
#include "JsiValue.h"
#include "RNSkPlatformContext.h"

namespace RNSkia {
Expand All @@ -29,8 +29,7 @@ class RNSkReadonlyValue
explicit RNSkReadonlyValue(
std::shared_ptr<RNSkPlatformContext> platformContext)
: JsiSkHostObject(platformContext),
_valueHolder(std::make_shared<RNJsi::JsiValueWrapper>(
*platformContext->getJsRuntime())) {}
_valueHolder(std::make_shared<RNJsi::JsiValue>()) {}

virtual ~RNSkReadonlyValue() { invalidate(); }

Expand Down Expand Up @@ -107,7 +106,7 @@ class RNSkReadonlyValue
@param value Next value
*/
virtual void update(jsi::Runtime &runtime, const jsi::Value &value) {
auto equal = _valueHolder->equals(runtime, value);
auto equal = _valueHolder->operator==(RNJsi::JsiValue(runtime, value));
if (!equal) {
_valueHolder->setCurrent(runtime, value);
notifyListeners(runtime);
Expand All @@ -127,14 +126,14 @@ class RNSkReadonlyValue
Returns the current value as a jsi::Value
*/
jsi::Value getCurrent(jsi::Runtime &runtime) {
return _valueHolder->getCurrent(runtime);
return _valueHolder->getAsJsiValue(runtime);
}

/**
Returns the underlying current value wrapper. This can be used to query the
holder for data type and get pointers to elements in the holder.
*/
std::shared_ptr<RNJsi::JsiValueWrapper> getCurrent() { return _valueHolder; }
std::shared_ptr<RNJsi::JsiValue> getCurrent() { return _valueHolder; }

protected:
/**
Expand All @@ -158,7 +157,7 @@ class RNSkReadonlyValue
}

private:
std::shared_ptr<RNJsi::JsiValueWrapper> _valueHolder;
std::shared_ptr<RNJsi::JsiValue> _valueHolder;

long _listenerId = 0;
std::unordered_map<long, std::function<void(jsi::Runtime &)>> _listeners;
Expand Down

0 comments on commit a69ddc8

Please sign in to comment.