From ab58d00d664355ab04fa9bb37174ec7dea8647cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elena=20Torr=C3=B3?= Date: Tue, 14 Jul 2026 07:43:52 +0200 Subject: [PATCH] :zap: Improve bool intersection perfomance (#10671) --- .../app/main/data/workspace/modifiers.cljs | 9 ++- .../src/app/main/data/workspace/shapes.cljs | 17 ++++- frontend/src/app/render_wasm/api.cljs | 25 ++++---- render-wasm/src/math/bools.rs | 62 ++++++++++++++++--- 4 files changed, 90 insertions(+), 23 deletions(-) diff --git a/frontend/src/app/main/data/workspace/modifiers.cljs b/frontend/src/app/main/data/workspace/modifiers.cljs index b5696414c8..41f14c0eea 100644 --- a/frontend/src/app/main/data/workspace/modifiers.cljs +++ b/frontend/src/app/main/data/workspace/modifiers.cljs @@ -838,8 +838,13 @@ (dwsh/update-shapes ids update-shape options) ;; The update to the bool path needs to be in a different operation because it - ;; needs to have the updated children info - (dwsh/update-shapes bool-ids path/update-bool-shape (assoc options :with-objects? true))) + ;; needs to have the updated children info. + ;; `update-layout? false`: recalculating a bool path can never change + ;; `:hidden`, and the layout check would recompute the whole boolean + ;; path in WASM once per bool shape just to find that out. + (dwsh/update-shapes bool-ids path/update-bool-shape (assoc options + :with-objects? true + :update-layout? false))) (if undo-transation? (rx/of (dwu/commit-undo-transaction undo-id)) diff --git a/frontend/src/app/main/data/workspace/shapes.cljs b/frontend/src/app/main/data/workspace/shapes.cljs index 771f3147d9..ac3ccae82b 100644 --- a/frontend/src/app/main/data/workspace/shapes.cljs +++ b/frontend/src/app/main/data/workspace/shapes.cljs @@ -154,12 +154,14 @@ ([ids update-fn {:as props :keys [reg-objects? save-undo? stack-undo? attrs ignore-tree page-id - ignore-touched undo-group with-objects? changed-sub-attr translation?] + ignore-touched undo-group with-objects? changed-sub-attr translation? + update-layout?] :or {reg-objects? false save-undo? true stack-undo? false ignore-touched false - with-objects? false}}] + with-objects? false + update-layout? true}}] (assert (every? uuid? ids) "expect a coll of uuid for `ids`") (assert (fn? update-fn) "the `update-fn` should be a valid function") @@ -181,8 +183,17 @@ (filter #(some update-layout-attr? (pcb/changed-attrs % objects update-fn {:attrs attrs :with-objects? with-objects?}))) (map :id)) + ;; `changed-attrs` runs `update-fn` in full for every shape, which + ;; can be expensive (e.g. `update-bool-shape` recalculates the whole + ;; boolean path in WASM). Skip the pass entirely when we can prove it + ;; cannot match: when the caller declares `attrs`, `changed-attrs` + ;; filters its result to that set, so if no layout attr is present + ;; the check is always empty. update-layout-ids - (when-not translation? + (when-not (or translation? + (not update-layout?) + (and (some? attrs) + (not (some update-layout-attr? attrs)))) (->> (into [] xf-update-layout ids) (not-empty))) diff --git a/frontend/src/app/render_wasm/api.cljs b/frontend/src/app/render_wasm/api.cljs index 3a5357d5d1..e0273d0125 100644 --- a/frontend/src/app/render_wasm/api.cljs +++ b/frontend/src/app/render_wasm/api.cljs @@ -2498,19 +2498,22 @@ ;; After the content is returned we discard that temporary context (h/call wasm/internal-module "_start_temp_objects") - (let [bool-type (get shape :bool-type) - ids (get shape :shapes) - all-children - (->> ids - (mapcat #(cfh/get-children-with-self objects %)))] + (try + (let [bool-type (get shape :bool-type) + ids (get shape :shapes) + all-children + (->> ids + (mapcat #(cfh/get-children-with-self objects %)))] - (h/call wasm/internal-module "_init_shapes_pool" (count all-children)) - (run! set-object all-children) + (h/call wasm/internal-module "_init_shapes_pool" (count all-children)) + (run! set-object all-children) - (let [content (-> (calculate-bool* bool-type ids) - (path.impl/path-data))] - (h/call wasm/internal-module "_end_temp_objects") - content))) + (-> (calculate-bool* bool-type ids) + (path.impl/path-data))) + (finally + ;; Always restore the main shapes pool: leaving the temp pool + ;; active would make the next `_start_temp_objects` panic. + (h/call wasm/internal-module "_end_temp_objects")))) (def POSITION-DATA-U8-SIZE 36) (def POSITION-DATA-U32-SIZE (/ POSITION-DATA-U8-SIZE 4)) diff --git a/render-wasm/src/math/bools.rs b/render-wasm/src/math/bools.rs index 0f7e01612d..c1f5b82edc 100644 --- a/render-wasm/src/math/bools.rs +++ b/render-wasm/src/math/bools.rs @@ -82,14 +82,31 @@ pub fn split_intersections(segment: Bezier, intersections: &[f64]) -> Vec = intersections.iter().map(|t| t.clamp(0.0, 1.0)).collect(); intersections.sort_by(|a, b| a.partial_cmp(b).unwrap_or(Ordering::Equal)); let mut prev = 0.0; let mut cur_segment = segment; for t_i in &intersections { - let rti = (t_i - prev) / (1.0 - prev); + // Skip duplicated split points (the same crossing can be reported by + // two adjacent opposing segments that share an endpoint): re-splitting + // at (almost) the same t would emit a zero-length sliver segment whose + // midpoint containment test is unstable in union/difference/intersection. + if *t_i - prev < 1e-6 { + continue; + } + let denom = 1.0 - prev; + // Degenerate split (prev already at the segment end); nothing left to cut. + if denom <= f64::EPSILON { + continue; + } + // Re-normalize the global t into the remaining segment, clamped so float + // noise / out-of-order duplicates can never push it outside [0,1]. + let rti = ((t_i - prev) / denom).clamp(0.0, 1.0); let [s, rest] = cur_segment.split(TValue::Parametric(rti)); prev = *t_i; cur_segment = rest; @@ -110,21 +127,52 @@ pub fn split_segments(path_a: &Path, path_b: &Path) -> (Vec, Vec let mut intersects_b = Vec::>::with_capacity(path_b.len()); intersects_b.resize_with(path_b.len(), Default::default); + // Broad-phase: precompute a conservative (control-hull) AABB per segment, + // padded by the intersection tolerance. Two segments can only intersect if + // their boxes overlap, so we skip the expensive `intersections()` call for + // the (typically vast) majority of non-overlapping pairs. This turns the + // O(A*B) inner loop from A*B curve-subdivision solves into A*B cheap box + // tests plus only the handful of solves that can actually produce a hit. + let bbox = |b: &Bezier| { + let [min, max] = b.bounding_box_of_anchors_and_handles(); + [ + DVec2::new(min.x - INTERSECT_ERROR, min.y - INTERSECT_ERROR), + DVec2::new(max.x + INTERSECT_ERROR, max.y + INTERSECT_ERROR), + ] + }; + let boxes_a: Vec<[DVec2; 2]> = path_a.iter().map(bbox).collect(); + let boxes_b: Vec<[DVec2; 2]> = path_b.iter().map(bbox).collect(); + for i in 0..path_a.len() { + let [amin, amax] = boxes_a[i]; for j in 0..path_b.len() { + let [bmin, bmax] = boxes_b[j]; + // AABB overlap test; skip pairs that cannot intersect. + if amin.x > bmax.x || bmin.x > amax.x || amin.y > bmax.y || bmin.y > amax.y { + continue; + } let segment_a = path_a[i]; let segment_b = path_b[j]; - let intersections_a = segment_a.intersections( + let mut intersections_a = segment_a.intersections( &segment_b, Some(INTERSECT_ERROR), Some(INTERSECT_MIN_SEPARATION), ); + // Clamp at the source: float error can report a t just outside + // [0,1], and every `TValue::Parametric` consumer downstream + // (`evaluate`, `project`, `split`) asserts `(0.0..=1.).contains(&t)`. + for t in intersections_a.iter_mut() { + *t = t.clamp(0.0, 1.0); + } + intersects_b[j].extend(intersections_a.iter().map(|t_a| { - segment_b.project( - segment_a.evaluate(TValue::Parametric(*t_a)), - Some(PROJECT_OPTS), - ) + segment_b + .project( + segment_a.evaluate(TValue::Parametric(*t_a)), + Some(PROJECT_OPTS), + ) + .clamp(0.0, 1.0) })); intersects_a[i].extend(intersections_a);