From 354dab918f91528af7fc40407179cca7f50e08ef Mon Sep 17 00:00:00 2001 From: louix Date: Tue, 16 Jan 2024 22:29:44 +0000 Subject: [PATCH 1/5] Handle SkColor creation from rgba tuple --- package/cpp/api/JsiSkColor.h | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/package/cpp/api/JsiSkColor.h b/package/cpp/api/JsiSkColor.h index 3edda89563..c2503b3ec5 100644 --- a/package/cpp/api/JsiSkColor.h +++ b/package/cpp/api/JsiSkColor.h @@ -76,7 +76,21 @@ class JsiSkColor : public RNJsi::JsiHostObject { return JsiSkColor::toValue( runtime, SkColorSetARGB(color.a * 255, color.r, color.g, color.b)); } else if (arguments[0].isObject()) { - return arguments[0].getObject(runtime); + auto obj = arguments[0].getObject(runtime); + if (obj.isArrayBuffer(runtime)) { + return obj; + } else if (obj.isArray(runtime)) { + auto array = obj.asArray(runtime); + double r = std::clamp(array.getValueAtIndex(runtime, 0).getNumber(), + 0.0, 255.0); + double g = std::clamp(array.getValueAtIndex(runtime, 1).getNumber(), + 0.0, 255.0); + double b = std::clamp(array.getValueAtIndex(runtime, 2).getNumber(), + 0.0, 255.0); + double a = std::clamp(array.getValueAtIndex(runtime, 3).getNumber(), + 0.0, 255.0); + return JsiSkColor::toValue(runtime, SkColorSetARGB(a, r, g, b)); + } } return jsi::Value::undefined(); }; From 8a956becb136326ecb65d343c265a6acf9d51d2b Mon Sep 17 00:00:00 2001 From: William Candillon Date: Wed, 17 Jan 2024 08:14:36 +0100 Subject: [PATCH 2/5] Add tests --- .../renderer/__tests__/e2e/Colors.spec.tsx | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 package/src/renderer/__tests__/e2e/Colors.spec.tsx diff --git a/package/src/renderer/__tests__/e2e/Colors.spec.tsx b/package/src/renderer/__tests__/e2e/Colors.spec.tsx new file mode 100644 index 0000000000..c6cbfb9d05 --- /dev/null +++ b/package/src/renderer/__tests__/e2e/Colors.spec.tsx @@ -0,0 +1,38 @@ +import { surface } from "../setup"; + +describe("Colors", () => { + it("should support int, string, array, and Float32Array", async () => { + const result = await surface.eval((Skia) => { + const areEqualFloat32Arrays = (...arrays: Float32Array[]) => { + // Check if all arrays have the same length + const allSameLength = arrays.every( + (array) => array.length === arrays[0].length + ); + if (!allSameLength) { + return false; + } + + // Compare elements across all arrays for each index + for (let i = 0; i < arrays[0].length; i++) { + if (!arrays.every((array) => array[i] === arrays[0][i])) { + return false; + } + } + + return true; + }; + + const c1 = Skia.Color("cyan"); + const c2 = Skia.Color([0, 1, 1, 1]); + const c3 = Skia.Color(0xff00ffff); + const c4 = Skia.Color(Float32Array.of(0, 1, 1, 1)); + + const r = areEqualFloat32Arrays(c1, c2, c3, c4); + if (!r) { + console.log({ c1, c2, c3, c4 }); + } + return r; + }); + expect(result).toBe(true); + }); +}); From 017d46f3aa5fc68079c6fedf0cdef73c7e4ad888 Mon Sep 17 00:00:00 2001 From: louix Date: Wed, 17 Jan 2024 12:11:00 +0000 Subject: [PATCH 3/5] Fix Float32Array check --- package/cpp/api/JsiSkColor.h | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/package/cpp/api/JsiSkColor.h b/package/cpp/api/JsiSkColor.h index c2503b3ec5..5e1892ab25 100644 --- a/package/cpp/api/JsiSkColor.h +++ b/package/cpp/api/JsiSkColor.h @@ -45,10 +45,7 @@ class JsiSkColor : public RNJsi::JsiHostObject { static SkColor fromValue(jsi::Runtime &runtime, const jsi::Value &obj) { const auto &object = obj.asObject(runtime); jsi::ArrayBuffer buffer = - object - .getProperty(runtime, jsi::PropNameID::forAscii(runtime, "buffer")) - .asObject(runtime) - .getArrayBuffer(runtime); + object.getPropertyAsObject(runtime, "buffer").getArrayBuffer(runtime); auto bfrPtr = reinterpret_cast(buffer.data(runtime)); if (bfrPtr[0] > 1 || bfrPtr[1] > 1 || bfrPtr[2] > 1 || bfrPtr[3] > 1) { return SK_ColorBLACK; @@ -77,22 +74,23 @@ class JsiSkColor : public RNJsi::JsiHostObject { runtime, SkColorSetARGB(color.a * 255, color.r, color.g, color.b)); } else if (arguments[0].isObject()) { auto obj = arguments[0].getObject(runtime); - if (obj.isArrayBuffer(runtime)) { - return obj; - } else if (obj.isArray(runtime)) { + if (obj.isArray(runtime)) { auto array = obj.asArray(runtime); - double r = std::clamp(array.getValueAtIndex(runtime, 0).getNumber(), - 0.0, 255.0); - double g = std::clamp(array.getValueAtIndex(runtime, 1).getNumber(), - 0.0, 255.0); - double b = std::clamp(array.getValueAtIndex(runtime, 2).getNumber(), - 0.0, 255.0); - double a = std::clamp(array.getValueAtIndex(runtime, 3).getNumber(), - 0.0, 255.0); - return JsiSkColor::toValue(runtime, SkColorSetARGB(a, r, g, b)); + // turn the array into a Float32Array + return runtime.global() + .getPropertyAsFunction(runtime, "Float32Array") + .callAsConstructor(runtime, array) + .getObject(runtime); + } else if (obj.hasProperty(runtime, "buffer") && + obj.getPropertyAsObject(runtime, "buffer") + .isArrayBuffer(runtime)) { + return obj; } } - return jsi::Value::undefined(); + throw jsi::JSError(runtime, + "Skia.Color expected number, Float32Array, number[], " + "or string and got: " + + arguments[0].toString(runtime).utf8(runtime)); }; } }; From 4c45ea1df6a2dea8e0ddd75ffcc9c88b9712f84a Mon Sep 17 00:00:00 2001 From: louix Date: Wed, 17 Jan 2024 12:11:12 +0000 Subject: [PATCH 4/5] Add offscreen canvas test --- .../renderer/__tests__/e2e/Colors.spec.tsx | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/package/src/renderer/__tests__/e2e/Colors.spec.tsx b/package/src/renderer/__tests__/e2e/Colors.spec.tsx index c6cbfb9d05..231a907eba 100644 --- a/package/src/renderer/__tests__/e2e/Colors.spec.tsx +++ b/package/src/renderer/__tests__/e2e/Colors.spec.tsx @@ -35,4 +35,42 @@ describe("Colors", () => { }); expect(result).toBe(true); }); + it("should render the same color regardless of constructor input types", async () => { + // given (different inputs representing the same color + const colors = [ + { kind: "string" as const, value: "red" }, + { kind: "float32Array" as const, value: [1, 0, 0, 1] }, + { kind: "number" as const, value: 0xffff0000 }, + { kind: "array" as const, value: [1, 0, 0, 1] }, + ]; + + // when (for each input we draw a colored canvas and encode it to bytes) + const buffers = await Promise.all( + colors.map((color) => + surface + .drawOffscreen( + (Skia, canvas, ctx) => { + const c = + ctx.color.kind === "float32Array" + ? // we cannot pass in a Float32Array via ctx, need to construct it inside + new Float32Array(ctx.color.value) + : ctx.color.value; + canvas.drawColor(Skia.Color(c)); + }, + { color } + ) + .then((image) => image.encodeToBytes()) + ) + ); + + // then (expect the encoded bytes are equal) + for (let i = 1; i < buffers.length; i++) { + const prev = buffers[i - 1]; + const curr = buffers[i]; + expect(prev.length).toBe(curr.length); + for (let j = 0; j < prev.length; j++) { + expect(prev[j]).toBe(curr[j]); + } + } + }); }); From 7e747d60b6203274e70194e00182e0147a0dc9f2 Mon Sep 17 00:00:00 2001 From: louix Date: Wed, 17 Jan 2024 12:11:15 +0000 Subject: [PATCH 5/5] Add exhaustive web case (to match C++) --- package/src/skia/web/JsiSkColor.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/package/src/skia/web/JsiSkColor.ts b/package/src/skia/web/JsiSkColor.ts index 9ab43036e5..1b8258a1ff 100644 --- a/package/src/skia/web/JsiSkColor.ts +++ b/package/src/skia/web/JsiSkColor.ts @@ -1,4 +1,4 @@ -import type { SkColor, Color as InputColor } from "../types"; +import type { Color as InputColor, SkColor } from "../types"; const alphaf = (c: number) => ((c >> 24) & 255) / 255; const red = (c: number) => (c >> 16) & 255; @@ -329,12 +329,17 @@ export const Color = (color: InputColor): SkColor => { rgba[2] / 255, rgba[3] ); - } else { + } else if (typeof color === "number") { return Float32Array.of( red(color) / 255, green(color) / 255, blue(color) / 255, alphaf(color) ); + } else { + // Exhaustive case (though not possible on the type level, could be possible at runtime) + throw new Error( + `Skia.Color expected number, Float32Array, number[], or string and got: ${color}` + ); } };