fix: isPointInPath/isPointInStroke should respect transform (#787)

Brooooooklyn · Feb 21, 2024 · 9b068f1 · 9b068f1
1 parent 9a4ade5
commit 9b068f1
Show file tree

Hide file tree

Showing 7 changed files with 76 additions and 66 deletions.
diff --git a/.github/workflows/CI.yaml b/.github/workflows/CI.yaml
@@ -147,6 +147,7 @@ jobs:
               touch "${ANDROID_NDK_LATEST_HOME}/toolchains/llvm/prebuilt/linux-x86_64/lib/clang/${CLANG_VERSION}/lib/linux/aarch64/libgcc.a"
               chmod 777 "${ANDROID_NDK_LATEST_HOME}/toolchains/llvm/prebuilt/linux-x86_64/lib/clang/${CLANG_VERSION}/lib/linux/aarch64/libgcc.a"
               echo "INPUT(-lunwind)" > "${ANDROID_NDK_LATEST_HOME}/toolchains/llvm/prebuilt/linux-x86_64/lib/clang/${CLANG_VERSION}/lib/linux/aarch64/libgcc.a"
+              export LDFLAGS="--target=aarch64-linux-android24"
               yarn build --target aarch64-linux-android
 
     name: stable - ${{ matrix.settings.target }} - node@18

diff --git a/__test__/regression.spec.ts b/__test__/regression.spec.ts
@@ -192,6 +192,16 @@ test('draw-svg-with-text', async (t) => {
 })
 
 test('DOMMatrix::transformPoint', (t) => {
-  console.info(new DOMPoint(1, 2))
   t.deepEqual(new DOMMatrix().transformPoint({ x: 1, y: 2 }), new DOMPoint(1, 2))
 })
+
+test('isPointInPath with translate', (t) => {
+  const canvas = createCanvas(1200, 700)
+  const ctx = canvas.getContext('2d')
+  ctx.translate(10, 10)
+  ctx.rect(0, 0, 100, 100)
+  t.false(ctx.isPointInPath(0, 0))
+  t.true(ctx.isPointInPath(10, 10))
+  t.true(ctx.isPointInPath(100, 100))
+  t.true(ctx.isPointInPath(110, 110))
+})
diff --git a/build.rs b/build.rs
@@ -73,9 +73,6 @@ fn main() {
           "/usr/include/c++/{gcc_version_trim}/x86_64-alpine-linux-musl"
         ));
     }
-    "aarch64-apple-darwin" => {
-      build.target("arm64-apple-macos");
-    }
     "aarch64-linux-android" => {
       let nkd_home = env::var("ANDROID_NDK_LATEST_HOME").unwrap();
       let host = if cfg!(target_os = "windows") {
@@ -211,6 +208,8 @@ fn main() {
       println!("cargo:rustc-link-lib=framework=ApplicationServices");
     }
     "android" => {
+      build.cpp_set_stdlib("c++").flag("-static");
+      println!("cargo:rustc-link-lib=static=c++");
       println!("cargo:rustc-cdylib-link-arg=-Wl,--allow-multiple-definition");
     }
     _ => {}

diff --git a/skia-c/skia_c.cpp b/skia-c/skia_c.cpp
@@ -1265,19 +1265,15 @@ extern "C"
     return reinterpret_cast<skiac_matrix *>(r);
   }
 
-  void skiac_matrix_map_points(skiac_matrix *c_matrix, float x1, float y1, float x2, float y2, skiac_mapped_point *mapped_point)
-  {
-    SkPoint dst[2];
-    auto p1 = SkPoint::Make(x1, y1);
-    auto p2 = SkPoint::Make(x2, y2);
-    SkPoint src[] = {p1, p2};
-    MATRIX_CAST->mapPoints(src, dst, 2);
-    auto dp1 = dst[0];
-    auto dp2 = dst[1];
-    mapped_point->x1 = dp1.fX;
-    mapped_point->y1 = dp1.fY;
-    mapped_point->x2 = dp2.fX;
-    mapped_point->y2 = dp2.fY;
+  void skiac_matrix_map_points_1(skiac_matrix *c_matrix, float x, float y, skiac_mapped_point *mapped_point)
+  {
+    SkPoint dst[1];
+    auto p = SkPoint::Make(x, y);
+    SkPoint src[] = {p};
+    MATRIX_CAST->mapPoints(src, dst, 1);
+    auto dp = dst[0];
+    mapped_point->x = dp.fX;
+    mapped_point->y = dp.fY;
   }
 
   skiac_matrix *skiac_matrix_clone(skiac_matrix *c_matrix)

diff --git a/skia-c/skia_c.hpp b/skia-c/skia_c.hpp
@@ -217,10 +217,8 @@ struct skiac_sk_data
 
 struct skiac_mapped_point
 {
-  float x1;
-  float y1;
-  float x2;
-  float y2;
+  float x;
+  float y;
 };
 
 extern "C"
@@ -445,7 +443,7 @@ extern "C"
   skiac_matrix *skiac_matrix_concat(skiac_matrix *c_matrix, skiac_matrix *other);
   skiac_matrix *skiac_matrix_multiply(skiac_matrix *c_matrix, skiac_matrix *other);
   skiac_matrix *skiac_matrix_clone(skiac_matrix *c_matrix);
-  void skiac_matrix_map_points(skiac_matrix *c_matrix, float x1, float y1, float x2, float y2, skiac_mapped_point *mapped_point);
+  void skiac_matrix_map_points_1(skiac_matrix *c_matrix, float x, float y, skiac_mapped_point *mapped_point);
   void skiac_matrix_pre_translate(skiac_matrix *c_matrix, float dx, float dy);
   void skiac_matrix_pre_concat(skiac_matrix *c_matrix, skiac_matrix *other);
   void skiac_matrix_pre_scale(skiac_matrix *c_matrix, float sx, float sy);

diff --git a/src/ctx.rs b/src/ctx.rs
@@ -238,22 +238,22 @@ impl Context {
   }
 
   pub fn translate(&mut self, x: f32, y: f32) {
-    let s = &mut self.state;
+    let current_state = &mut self.state;
     let inverse = Matrix::translated(-x, -y);
     self.path.transform_self(&inverse);
-    s.transform.pre_translate(x, y);
-    self.surface.canvas.set_transform(&s.transform);
+    current_state.transform.pre_translate(x, y);
+    self.surface.canvas.set_transform(&current_state.transform);
   }
 
   pub fn transform(&mut self, ts: Matrix) -> result::Result<(), SkError> {
-    let s = &mut self.state;
+    let current_state = &mut self.state;
     self.path.transform_self(
       &ts
         .invert()
         .ok_or_else(|| SkError::InvalidTransform(ts.clone()))?,
     );
-    s.transform = ts.multiply(&s.transform);
-    self.surface.set_transform(&s.transform);
+    current_state.transform = ts.multiply(&current_state.transform);
+    self.surface.set_transform(&current_state.transform);
     Ok(())
   }
 
@@ -1571,21 +1571,28 @@ impl CanvasRenderingContext2D {
     y_or_fill_rule: Option<Either<f64, String>>,
     maybe_fill_rule: Option<String>,
   ) -> Result<bool> {
+    let inverted = self.context.state.transform.invert();
     match x_or_path {
       Either::A(x) => {
-        let y = x_or_y;
+        let mut x = x as f32;
+        let mut y = x_or_y as f32;
         let fill_rule = y_or_fill_rule
           .and_then(|v| match v {
             Either::B(rule) => rule.parse().ok(),
             _ => None,
           })
           .unwrap_or(FillType::Winding);
-        Ok(self.context.path.hit_test(x as f32, y as f32, fill_rule))
+        if let Some(inverted) = inverted {
+          let (mapped_x, mapped_y) = inverted.map_points(x, y);
+          x = mapped_x;
+          y = mapped_y;
+        }
+        Ok(self.context.path.hit_test(x, y, fill_rule))
       }
       Either::B(path) => {
-        let x = x_or_y;
-        let y = match y_or_fill_rule {
-          Some(Either::A(y)) => y,
+        let mut x = x_or_y as f32;
+        let mut y = match y_or_fill_rule {
+          Some(Either::A(y)) => y as f32,
           _ => {
             return Err(Error::new(
               Status::InvalidArg,
@@ -1596,7 +1603,12 @@ impl CanvasRenderingContext2D {
         let fill_rule = maybe_fill_rule
           .and_then(|s| s.parse().ok())
           .unwrap_or(FillType::Winding);
-        Ok(path.inner.hit_test(x as f32, y as f32, fill_rule))
+        if let Some(inverted) = inverted {
+          let (mapped_x, mapped_y) = inverted.map_points(x, y);
+          x = mapped_x;
+          y = mapped_y;
+        }
+        Ok(path.inner.hit_test(x, y, fill_rule))
       }
     }
   }
@@ -1609,20 +1621,28 @@ impl CanvasRenderingContext2D {
     maybe_y: Option<f64>,
   ) -> Result<bool> {
     let stroke_w = self.context.get_stroke_width();
+    let inverted = self.context.state.transform.invert();
     match x_or_path {
       Either::A(x) => {
-        let y = x_or_y;
-        Ok(
-          self
-            .context
-            .path
-            .stroke_hit_test(x as f32, y as f32, stroke_w),
-        )
+        let mut x = x as f32;
+        let mut y = x_or_y as f32;
+        if let Some(inverted) = inverted {
+          let (mapped_x, mapped_y) = inverted.map_points(x, y);
+          x = mapped_x;
+          y = mapped_y;
+        }
+        Ok(self.context.path.stroke_hit_test(x, y, stroke_w))
       }
       Either::B(path) => {
-        let x = x_or_y;
+        let mut x = x_or_y as f32;
         if let Some(y) = maybe_y {
-          Ok(path.inner.stroke_hit_test(x as f32, y as f32, stroke_w))
+          let mut y = y as f32;
+          if let Some(inverted) = inverted {
+            let (mapped_x, mapped_y) = inverted.map_points(x, y);
+            x = mapped_x;
+            y = mapped_y;
+          }
+          Ok(path.inner.stroke_hit_test(x, y, stroke_w))
         } else {
           Err(Error::new(
             Status::InvalidArg,

diff --git a/src/sk.rs b/src/sk.rs
@@ -203,10 +203,8 @@ pub mod ffi {
   #[repr(C)]
   #[derive(Debug, Clone, Copy)]
   pub struct skiac_mapped_point {
-    pub x1: f32,
-    pub y1: f32,
-    pub x2: f32,
-    pub y2: f32,
+    pub x: f32,
+    pub y: f32,
   }
 
   #[repr(C)]
@@ -718,12 +716,10 @@ pub mod ffi {
 
     pub fn skiac_matrix_clone(matrix: *mut skiac_matrix) -> *mut skiac_matrix;
 
-    pub fn skiac_matrix_map_points(
+    pub fn skiac_matrix_map_points_1(
       c_matrix: *mut skiac_matrix,
-      x1: f32,
-      y1: f32,
-      x2: f32,
-      y2: f32,
+      x: f32,
+      y: f32,
       mapped_point: *mut skiac_mapped_point,
     );
 
@@ -3005,20 +3001,10 @@ impl Matrix {
     Matrix(unsafe { ffi::skiac_matrix_create_translated(x, y) })
   }
 
-  pub fn map_points(&self, x1: f32, y1: f32, x2: f32, y2: f32) -> (f32, f32, f32, f32) {
-    let mut mapped_point = ffi::skiac_mapped_point {
-      x1: 0.0,
-      y1: 0.0,
-      x2: 0.0,
-      y2: 0.0,
-    };
-    unsafe { ffi::skiac_matrix_map_points(self.0, x1, y1, x2, y2, &mut mapped_point) };
-    (
-      mapped_point.x1,
-      mapped_point.y1,
-      mapped_point.x2,
-      mapped_point.y2,
-    )
+  pub fn map_points(&self, x: f32, y: f32) -> (f32, f32) {
+    let mut mapped_point = ffi::skiac_mapped_point { x: 0.0, y: 0.0 };
+    unsafe { ffi::skiac_matrix_map_points_1(self.0, x, y, &mut mapped_point) };
+    (mapped_point.x, mapped_point.y)
   }
 
   pub fn pre_translate(&mut self, dx: f32, dy: f32) {