From 8343b57064f53b06605549684969b863e6ebf4cb Mon Sep 17 00:00:00 2001 From: William Candillon Date: Thu, 30 Mar 2023 19:25:46 +0200 Subject: [PATCH] Fix clip property regression (#1454) --------- Co-authored-by: Christian Falch <875252+chrfalch@users.noreply.github.com> --- package/cpp/rnskia/dom/props/ClipProp.h | 33 +++--- package/cpp/rnskia/dom/props/PathProp.h | 30 +++--- package/cpp/rnskia/dom/props/RRectProp.h | 100 +++++++----------- package/cpp/rnskia/dom/props/RectProp.h | 52 ++++----- .../__tests__/e2e/BackdropFilters.spec.tsx | 48 +++++++++ .../src/renderer/__tests__/e2e/Paths.spec.tsx | 6 +- 6 files changed, 145 insertions(+), 124 deletions(-) diff --git a/package/cpp/rnskia/dom/props/ClipProp.h b/package/cpp/rnskia/dom/props/ClipProp.h index b3fe3153db..4ca5a5b479 100644 --- a/package/cpp/rnskia/dom/props/ClipProp.h +++ b/package/cpp/rnskia/dom/props/ClipProp.h @@ -22,43 +22,36 @@ class ClipProp : public BaseDerivedProp { explicit ClipProp(PropId name, const std::function &onChange) : BaseDerivedProp(onChange) { - _pathProp = defineProperty(name); - _rectProp = defineProperty(name); - _rrectProp = defineProperty(name); + _clipProp = defineProperty(name); } void updateDerivedValue() override { - if (_pathProp->isSet()) { - _rect = nullptr; - _rrect = nullptr; - _path = _pathProp->getDerivedValue(); - } else if (_rrectProp->isSet()) { - _rect = nullptr; - _rrect = _rrectProp->getDerivedValue(); - _path = nullptr; - } else if (_rectProp->isSet()) { - _rect = _rectProp->getDerivedValue(); + if (_clipProp->isSet()) { + auto value = _clipProp->value(); + _rect = RectProp::processRect(value); _rrect = nullptr; _path = nullptr; + if (!_rect) { + _path = PathProp::processPath(value); + if (!_path) { + _rrect = RRectProp::processRRect(value); + } + } } } - bool isSet() override { - return _pathProp->isSet() || _rectProp->isSet() || _rrectProp->isSet(); - } + bool isSet() override { return _clipProp->isSet(); } const SkPath *getPath() { return _path.get(); } const SkRect *getRect() { return _rect.get(); } const SkRRect *getRRect() { return _rrect.get(); } private: - PathProp *_pathProp; - RectProp *_rectProp; - RRectProp *_rrectProp; + NodeProp *_clipProp; std::shared_ptr _path; std::shared_ptr _rect; std::shared_ptr _rrect; }; -} // namespace RNSkia +} // namespace RNSkia \ No newline at end of file diff --git a/package/cpp/rnskia/dom/props/PathProp.h b/package/cpp/rnskia/dom/props/PathProp.h index ab75da279f..14a8569e5d 100644 --- a/package/cpp/rnskia/dom/props/PathProp.h +++ b/package/cpp/rnskia/dom/props/PathProp.h @@ -22,32 +22,34 @@ class PathProp : public DerivedProp { _pathProp = defineProperty(name); } - void updateDerivedValue() override { - if (!_pathProp->isSet()) { - setDerivedValue(nullptr); - return; - } - - if (_pathProp->value().getType() == PropType::HostObject) { + static std::shared_ptr processPath(const JsiValue &value) { + if (value.getType() == PropType::HostObject) { // Try reading as Path - auto ptr = std::dynamic_pointer_cast( - _pathProp->value().getAsHostObject()); + auto ptr = std::dynamic_pointer_cast(value.getAsHostObject()); if (ptr != nullptr) { - setDerivedValue(ptr->getObject()); + return ptr->getObject(); } - } else if (_pathProp->value().getType() == PropType::String) { + } else if (value.getType() == PropType::String) { // Read as string - auto pathString = _pathProp->value().getAsString(); + auto pathString = value.getAsString(); SkPath result; if (SkParsePath::FromSVGString(pathString.c_str(), &result)) { - setDerivedValue(std::make_shared(result)); + return std::make_shared(result); } else { throw std::runtime_error("Could not parse path from string."); } - } else { + } + return nullptr; + } + + void updateDerivedValue() override { + if (!_pathProp->isSet()) { setDerivedValue(nullptr); + return; } + auto value = _pathProp->value(); + setDerivedValue(PathProp::processPath(value)); } private: diff --git a/package/cpp/rnskia/dom/props/RRectProp.h b/package/cpp/rnskia/dom/props/RRectProp.h index 8802b14898..3fd8661c9a 100644 --- a/package/cpp/rnskia/dom/props/RRectProp.h +++ b/package/cpp/rnskia/dom/props/RRectProp.h @@ -33,35 +33,33 @@ class RRectProp : public DerivedProp { _prop = defineProperty(name); } - void updateDerivedValue() override { - if (_prop->isSet()) { - // Check for JsiSkRRect - if (_prop->value().getType() == PropType::HostObject) { - // Try reading as rect - auto rectPtr = std::dynamic_pointer_cast( - _prop->value().getAsHostObject()); - if (rectPtr != nullptr) { - auto rrect = rectPtr->getObject(); - setDerivedValue(SkRRect::MakeRectXY( - SkRect::MakeXYWH(rrect->rect().x(), rrect->rect().y(), - rrect->rect().width(), rrect->rect().height()), - rrect->getSimpleRadii().x(), rrect->getSimpleRadii().y())); - } - } else { - if (_prop->isSet() && _prop->value().getType() == PropType::Object) { - auto p = _prop->value(); - if (p.hasValue(PropNameX) && p.hasValue(PropNameY) && - p.hasValue(PropNameWidth) && p.hasValue(PropNameHeight) && - p.hasValue(PropNameRx) && p.hasValue(PropNameRy)) { - auto x = _prop->value().getValue(PropNameX); - auto y = _prop->value().getValue(PropNameY); - auto width = _prop->value().getValue(PropNameWidth); - auto height = _prop->value().getValue(PropNameHeight); - auto rx = _prop->value().getValue(PropNameRx); - auto ry = _prop->value().getValue(PropNameRy); + static std::shared_ptr processRRect(const JsiValue &value) { + if (value.getType() == PropType::HostObject) { + // Try reading as rect + auto rectPtr = + std::dynamic_pointer_cast(value.getAsHostObject()); + if (rectPtr != nullptr) { + auto rrect = rectPtr->getObject(); + return std::make_shared( + SkRRect::MakeRectXY(rrect->rect(), rrect->getSimpleRadii().x(), + rrect->getSimpleRadii().y())); + } + } else { + if (value.getType() == PropType::Object) { + if (value.hasValue(PropNameRect) && value.hasValue(PropNameRx) && + value.hasValue(PropNameRy)) { + auto rect = value.getValue(PropNameRect); + if (rect.hasValue(PropNameX) && rect.hasValue(PropNameY) && + rect.hasValue(PropNameWidth) && rect.hasValue(PropNameHeight)) { + auto x = rect.getValue(PropNameX); + auto y = rect.getValue(PropNameY); + auto width = rect.getValue(PropNameWidth); + auto height = rect.getValue(PropNameHeight); + auto rx = value.getValue(PropNameRx); + auto ry = value.getValue(PropNameRy); // Update cache from js object value - setDerivedValue(SkRRect::MakeRectXY( + return std::make_shared(SkRRect::MakeRectXY( SkRect::MakeXYWH(x.getAsNumber(), y.getAsNumber(), width.getAsNumber(), height.getAsNumber()), rx.getAsNumber(), ry.getAsNumber())); @@ -69,6 +67,14 @@ class RRectProp : public DerivedProp { } } } + return nullptr; + } + + void updateDerivedValue() override { + if (_prop->isSet()) { + auto value = _prop->value(); + setDerivedValue(RRectProp::processRRect(value)); + } } private: @@ -150,40 +156,12 @@ class BoxProps : public DerivedProp { } void updateDerivedValue() override { - if (_boxProp->value().getType() == PropType::HostObject) { - auto rectPtr = std::dynamic_pointer_cast( - _boxProp->value().getAsHostObject()); - auto rrectPtr = std::dynamic_pointer_cast( - _boxProp->value().getAsHostObject()); - // 1. box is SkRect - if (rectPtr != nullptr) { - auto rect = rectPtr->getObject(); - setDerivedValue(SkRRect::MakeRect(*rect)); - // 2. box is SkRRect - } else if (rrectPtr != nullptr) { - setDerivedValue(rrectPtr->getObject()); - } - } else if (_boxProp->value().getType() == PropType::Object) { - if (_boxProp->value().hasValue(PropNameRect)) { - // 3. box is { rect: { x, y, width, height }, rx, ry } - auto rectProp = _boxProp->value().getValue(PropNameRect); - auto x = rectProp.getValue(PropNameX).getAsNumber(); - auto y = rectProp.getValue(PropNameY).getAsNumber(); - auto width = rectProp.getValue(PropNameWidth).getAsNumber(); - auto height = rectProp.getValue(PropNameHeight).getAsNumber(); - auto rx = _boxProp->value().getValue(PropNameRx).getAsNumber(); - auto ry = _boxProp->value().getValue(PropNameRy).getAsNumber(); - setDerivedValue( - SkRRect::MakeRectXY(SkRect::MakeXYWH(x, y, width, height), rx, ry)); - } else { - // 4. box is { x, y, width, height } - auto x = _boxProp->value().getValue(PropNameX).getAsNumber(); - auto y = _boxProp->value().getValue(PropNameY).getAsNumber(); - auto width = _boxProp->value().getValue(PropNameWidth).getAsNumber(); - auto height = _boxProp->value().getValue(PropNameHeight).getAsNumber(); - setDerivedValue( - SkRRect::MakeRect(SkRect::MakeXYWH(x, y, width, height))); - } + auto value = _boxProp->value(); + auto rect = RectProp::processRect(value); + if (rect) { + setDerivedValue(SkRRect::MakeRect(*rect)); + } else { + setDerivedValue(RRectProp::processRRect(value)); } } diff --git a/package/cpp/rnskia/dom/props/RectProp.h b/package/cpp/rnskia/dom/props/RectProp.h index 336f363d8f..d0e2783f83 100644 --- a/package/cpp/rnskia/dom/props/RectProp.h +++ b/package/cpp/rnskia/dom/props/RectProp.h @@ -31,33 +31,35 @@ class RectProp : public DerivedProp { _prop = defineProperty(name); } + static std::shared_ptr processRect(const JsiValue &value) { + if (value.getType() == PropType::HostObject) { + auto rectPtr = + std::dynamic_pointer_cast(value.getAsHostObject()); + if (rectPtr != nullptr) { + return std::make_shared(SkRect::MakeXYWH( + rectPtr->getObject()->x(), rectPtr->getObject()->y(), + rectPtr->getObject()->width(), rectPtr->getObject()->height())); + } + } else if (value.getType() == PropType::Object && + value.hasValue(PropNameX) && value.hasValue(PropNameY) && + value.hasValue(PropNameWidth) && + value.hasValue(PropNameHeight)) { + // Save props for fast access + auto x = value.getValue(PropNameX); + auto y = value.getValue(PropNameY); + auto width = value.getValue(PropNameWidth); + auto height = value.getValue(PropNameHeight); + // Update cache from js object value + return std::make_shared( + SkRect::MakeXYWH(x.getAsNumber(), y.getAsNumber(), + width.getAsNumber(), height.getAsNumber())); + } + return nullptr; + } + void updateDerivedValue() override { if (_prop->isSet()) { - // Check for JsiSkRect - if (_prop->value().getType() == PropType::HostObject) { - auto rectPtr = std::dynamic_pointer_cast( - _prop->value().getAsHostObject()); - if (rectPtr != nullptr) { - setDerivedValue(SkRect::MakeXYWH( - rectPtr->getObject()->x(), rectPtr->getObject()->y(), - rectPtr->getObject()->width(), rectPtr->getObject()->height())); - } - } else { - auto p = _prop->value(); - if (p.hasValue(PropNameX) && p.hasValue(PropNameY) && - p.hasValue(PropNameWidth) && p.hasValue(PropNameHeight)) { - // Save props for fast access - auto x = p.getValue(PropNameX); - auto y = p.getValue(PropNameY); - auto width = p.getValue(PropNameWidth); - auto height = p.getValue(PropNameHeight); - - // Update cache from js object value - setDerivedValue(SkRect::MakeXYWH(x.getAsNumber(), y.getAsNumber(), - width.getAsNumber(), - height.getAsNumber())); - } - } + setDerivedValue(RectProp::processRect(_prop->value())); } } diff --git a/package/src/renderer/__tests__/e2e/BackdropFilters.spec.tsx b/package/src/renderer/__tests__/e2e/BackdropFilters.spec.tsx index ca242bf8cd..92d87e4cd1 100644 --- a/package/src/renderer/__tests__/e2e/BackdropFilters.spec.tsx +++ b/package/src/renderer/__tests__/e2e/BackdropFilters.spec.tsx @@ -90,4 +90,52 @@ describe("Backdrop Filters", () => { ); checkImage(img, docPath("blur-backdrop-aurora.png")); }); + it("should display the Aurora Example with the BackdropBlur component", async () => { + const { width, height } = surface; + const { vec } = importSkia(); + const c = vec(width / 2, height / 2); + const r = c.x - 32 / 3; + const clip = { x: 0, y: c.y, width, height: c.y }; + const img = await surface.draw( + <> + + + + + + + + + ); + checkImage(img, docPath("blur-backdrop-aurora.png")); + }); + it("should display the Aurora Example with the BackdropBlur component and a string as clipping path", async () => { + const { width, height } = surface; + const { vec, Skia } = importSkia(); + const c = vec(width / 2, height / 2); + const r = c.x - 32 / 3; + const path = Skia.Path.Make(); + path.addRect(Skia.XYWHRect(0, c.y, width, c.y)); + const clip = path.toSVGString(); + const img = await surface.draw( + <> + + + + + + + + + ); + checkImage(img, docPath("blur-backdrop-aurora.png")); + }); }); diff --git a/package/src/renderer/__tests__/e2e/Paths.spec.tsx b/package/src/renderer/__tests__/e2e/Paths.spec.tsx index acf93f636b..ca5e9f7b86 100644 --- a/package/src/renderer/__tests__/e2e/Paths.spec.tsx +++ b/package/src/renderer/__tests__/e2e/Paths.spec.tsx @@ -52,11 +52,9 @@ describe("Paths", () => { "M447.955 756.37H44.699C20.5664 756.37 0.820374 736.624 0.820374 712.492V44.8242C0.820374 20.6898 20.5664 0.945557 44.699 0.945557H447.955C472.088 0.945557 491.834 20.6898 491.834 44.8242V712.492C491.834 736.624 472.088 756.37 447.955 756.37Z" )!; - const bg = Skia.Path.MakeFromSVGString( + const bg = // eslint-disable-next-line max-len - "M423.554 40.0679H69.3443C51.3334 40.0679 36.6796 54.7199 36.6796 72.7307V685.881C36.6796 703.892 51.3334 718.546 69.3443 718.546H423.554C441.565 718.546 456.219 703.892 456.219 685.881V72.7307C456.219 54.7199 441.565 40.0679 423.554 40.0679Z" - )!; - + "M423.554 40.0679H69.3443C51.3334 40.0679 36.6796 54.7199 36.6796 72.7307V685.881C36.6796 703.892 51.3334 718.546 69.3443 718.546H423.554C441.565 718.546 456.219 703.892 456.219 685.881V72.7307C456.219 54.7199 441.565 40.0679 423.554 40.0679Z"; const Circles = () => { const c = 12; const delta = 100 / c;