Clojure's = uses .equals on doubles, and Double.equals(Double.NaN)
returns true, so (not= v v) was always false for NaN. Use
Double/isNaN with a number? guard instead.
The CLJS branch of num-string? checked (string? v) first, but the
JVM branch did not. Passing non-string values (nil, keywords, etc.)
would rely on exception handling inside parse-double for control
flow. Add the string? check for consistency and to avoid using
exceptions for normal control flow.
The removev function used 'fn' as its predicate parameter name,
which shadows clojure.core/fn. Rename to 'pred' for clarity and
to follow the naming convention used elsewhere in the namespace.
When called with an empty string as the base class, append-class
was producing " bar" (with a leading space) because (some? "")
returns true. Use (seq class) instead to treat both nil and empty
string as absent, avoiding invalid CSS class strings with leading
whitespace.
The :else branch of diff-attr was calling (get m1 key) and
(get m2 key) again, but v1 and v2 were already bound to those
exact values. Reuse the existing bindings to avoid the extra
lookups.
The docstring claimed the function removes nil values in addition to
the specified object, but the implementation only removes elements
equal to the given object. Fix the docstring in both data.cljc and
the local copy in files/changes.cljc.
The 3-arity of safe-subvec called (count v) in a let binding before
checking (some? v). While (count nil) returns 0 in Clojure and does
not crash, the nil guard was dead code. Restructure to check (some? v)
first with an outer when, then compute size inside the guarded block.
Replace (empty? items) + (rest items) with (seq items) + (next items)
in enumerate. The seq/next pattern is idiomatic Clojure and avoids
the overhead of empty? which internally calls seq and then negates.
The index-of-pred function used (nil? c) to detect end-of-collection,
which caused premature termination when the collection contained nil
values. Rewrite using (seq coll) / (next s) pattern to correctly
distinguish between nil elements and end-of-sequence.
The deep-mapm function was applying the mapping function twice on
leaf entries (non-map, non-vector values): once when destructuring
the entry, and again on the already-transformed result in the else
branch. Now mfn is applied exactly once per entry.
The patch-object function was calling (dissoc object key value) when
handling nil values. Since dissoc treats each argument after the map
as a key to remove, this was also removing nil as a key from the map.
The correct call is (dissoc object key).
Tests that exercise app.common.types.color were living inside
common-tests.colors-test alongside the app.common.colors tests. Move
them to common-tests.types.color-test so the test namespace mirrors
the source namespace structure, consistent with the rest of the
types/ test suite.
The [app.common.types.color :as colors] require is removed from
colors_test.cljc; the new file is registered in runner.cljc.
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
The five functions interpolate-color, offset-spread, uniform-spread?,
uniform-spread, and interpolate-gradient duplicated the canonical
implementations in app.common.types.color. The copies in colors.cljc
also contained two bugs: a division-by-zero in offset-spread when
num=1, and a crash on nil idx in interpolate-gradient.
All production callers already use app.common.types.color. The
duplicate tests that exercised the old copies are removed; their
coverage is absorbed into expanded tests under the types-* suite,
including a new nil-idx guard test and a single-stop no-crash test.
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
Add indexed-access-with-default in fill_test.cljc to cover the two-arity
(nth fills i default) form on both valid and out-of-range indices, directly
exercising the CLJS Fills -nth path fixed in 593cf125.
Add segment-content->selrect-multi-line in path_data_test.cljc to cover
content->selrect on a subpath with multiple consecutive line-to commands
where move-p diverges from from-p, confirming the bounding box matches
both the expected coordinates and the reference implementation; this
guards the calculate-extremities fix in bb5a04c7.
Add types-uniform-spread? in colors_test.cljc to cover
app.common.types.color/uniform-spread?, which had no dedicated tests.
Exercises the uniform case (via uniform-spread), the two-stop edge case,
wrong-offset detection, and wrong-color detection.
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
In the :line-to branch of calculate-extremities, move-p (the subpath
start point) was being added to the extremities set instead of from-p
(the actual previous point). For all line segments beyond the first one
in a subpath this produced an incorrect bounding-box start point.
The :curve-to branch correctly used from-p; align :line-to to match.
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
In the ClojureScript Fills deftype, the two-arity -nth implementation
called (d/in-range? i size) but the signature is (d/in-range? size i).
This meant -nth always fell through to the default value for any valid
index when called with an explicit default, since i < size is the
condition but the args were swapped.
The no-default -nth sibling on line 378 and both CLJ nth impls on
lines 286 and 291 had the correct argument order.
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
In the CLJS -lookup implementation, when a key is absent from data the
negative cache entry was stored under 'key' (the built-in map-entry
key function) rather than the 'k' parameter. As a result every
subsequent lookup of any missing key bypassed the cache and repeated
the full lookup path, making the negative-cache optimization entirely
ineffective.
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
`(cfh/frame-shape? current-id)` passes a UUID to the single-arity
overload of `frame-shape?`, which expects a shape map; it always
returns false. Fix by passing `current` (the resolved shape) instead.
Update the test to assert the correct behaviour.
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
`(mapcat collect-main-shapes children objects)` passes `objects` as a
second parallel collection instead of threading it as the second
argument to `collect-main-shapes` for each child. Fix by using an
anonymous fn: `(mapcat #(collect-main-shapes % objects) children)`.
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
\`(get "type" shadow)\` always returns nil because the map and key
arguments were swapped. The correct call is \`(get shadow "type")\`,
which allows the legacy innerShadow detection to work correctly.
Update the test expectation accordingly.
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
font-weight-keys was listed twice in the set/union call for
typography-keys, a copy-paste error. The duplicate entry has no
functional effect (sets deduplicate), but it is misleading and
suggests a missing key such as font-style-keys in its place.
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
get-children-rec passed the original children vector to each recursive
call instead of the updated one that already includes the current
shape. This caused descendant results to be accumulated from the wrong
starting point, losing intermediate shapes. Pass children' (which
includes the current shape) into every recursive call.
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
When no gradient stop satisfies (<= offset (:offset %)),
d/index-of-pred returns nil. The previous code called (dec nil) in
the start binding before the nil check, throwing a
NullPointerException/ClassCastException. Guard the start binding with
a cond that handles nil before attempting dec.
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
The key :podition was used instead of :position when updating the
id-from cell in swap-shapes, silently discarding the position value
and leaving the cell's :position as nil after every swap.
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
Fixes three concrete builder issues in common/files/builder:\n- Use bool type from shape when selecting style source for difference bools\n- Persist :strokes correctly (fix typo :stroks)\n- Validate add-file-media params after assigning default id\n\nAlso adds regression tests in common-tests.files-builder-test and registers them in runner.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: raguirref <ricardoaguirredelafuente@gmail.com>
- Fix 'conten' typo to 'content' in path.cljc docstring
- Fix 'curvle' typo to 'curve' in shape_to_path.cljc docstring
- Replace confusing XOR-style filter with readable
(contains? #{:line-to :curve-to} ...) in bool.cljc
- Align handler-indices and opposite-index docstrings with
matching API in path.cljc
The CLJS implementation of PathData's -nth protocol method had
swapped arguments in the 3-arity version (with default value).
The call (d/in-range? i size) should be (d/in-range? size i)
to match the CLJ implementation. With swapped args, valid indices
always returned the default value, and invalid indices attempted
out-of-bounds buffer reads.
Add normalize-coord helper function that clamps coordinate values to
max-safe-int and min-safe-int bounds when reading segments from PathData
binary buffer. Applies normalization to read-segment, impl-walk,
impl-reduce, and impl-lookup functions to ensure coordinates remain
within safe bounds.
Add corresponding test to verify out-of-bounds coordinates are properly
clamped when reading PathData.
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
The backtrace-tokens-tree function used a namespaced keyword :temp/id
which clj->js converted to the JS property "temp/id". The sd-token-uuid
function then tried to access .id on the sd-token top-level object,
which was undefined, causing "Cannot read properties of undefined
(reading uuid)".
Fix by using the existing token :id instead of generating a temporary
one, and read it from sd-token.original (matching sd-token-name pattern).
Replace unsafe std::mem::transmute calls in Rust WASM path code with
validated TryFrom conversions to prevent undefined behavior from invalid
enum discriminant values. This was the most likely root cause of the
"No matching clause: -19772" production crash -- corrupted bytes flowing
through transmute could produce arbitrary invalid enum variants.
Fix byteOffset handling throughout the CLJS PathData serialization
pipeline. DataView instances created via buf/slice carry a non-zero
byteOffset, but from-bytes, transit write handler, -write-to,
buf/clone, and buf/equals? all operated on the full underlying
ArrayBuffer, ignoring offset and length. This could silently produce
PathData with incorrect size or content.
Rust changes (render-wasm):
- RawSegmentData: From<[u8; N]> -> TryFrom<[u8; N]> with discriminant
validation (must be 0x01-0x04) before transmuting
- RawBoolType: From<u8> -> TryFrom<u8> with explicit match on 0-3
- Add #[wasm_error] to set_shape_path_content, current_to_path,
convert_stroke_to_path, and set_shape_bool_type so panics are caught
and routed through the WASM error protocol instead of crashing
- set_shape_path_content: replace .expect() with proper Result/? error
propagation per segment
- Remove unused From<BytesType> bound from SerializableResult trait
CLJS changes (common):
- from-bytes: use DataView.byteLength instead of ArrayBuffer.byteLength
for DataView inputs; preserve byteOffset/byteLength when converting
from Uint8Array, Uint32Array, and Int8Array
- Transit write handler: construct Uint8Array with byteOffset and
byteLength from the DataView, not the full backing ArrayBuffer
- -write-to: same byteOffset/byteLength fix
- buf/clone: copy only the DataView byte range using Uint8Array with
proper offset, not Uint32Array over the full ArrayBuffer
- buf/equals?: compare DataView byte ranges using Uint8Array with
proper offset, not the full backing ArrayBuffers
Frontend changes:
- shape-to-path, stroke-to-path, calculate-bool*: wrap WASM call and
buffer read in try/catch to ensure mem/free is always called, even
when an exception occurs between the WASM call and the free call
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
Add nil defaults to all case expressions that match binary segment
type codes so that corrupted/unknown values are skipped instead of
throwing 'No matching clause'. This prevents a React render crash
(triggered via shape-with-open-path? -> get-subpaths -> reduce)
when a PathData buffer contains invalid bytes, e.g. from a WASM
data transfer or deserialization of damaged stored data.
Affected functions: read-segment, impl-walk, impl-reduce,
impl-lookup, to-string-segment*, and the seq/reduce protocol
implementations on both JVM and CLJS PathData types.
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
The impl-walk, impl-reduce, and impl-lookup functions had the binary
type codes for move-to and line-to swapped (1 mapped to :line-to and
2 to :move-to). This is inconsistent with from-plain, read-segment,
to-string-segment*, and the Rust RawSegmentData enum which all use
1=move-to and 2=line-to. The swap caused incorrect command types to
be reported to callers like get-handlers and get-points.
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
* 🐛 Fix nil path content crash by exposing safe public API
Move nil-safety for path segment helpers to the public API layer
(app.common.types.path) rather than the low-level segment namespace.
Add nil-safe wrappers for get-handlers, opposite-index, get-handler-point,
get-handler, handler->node, point-indices, handler-indices, next-node,
append-segment, points->content, closest-point, make-corner-point,
make-curve-point, split-segments, remove-nodes, merge-nodes, join-nodes,
and separate-nodes. Update all frontend callers to use path/ instead of
path.segment/ for these functions, removing the path.segment require
from helpers, drawing, edition, tools, curve, editor and debug.
Replace ad-hoc nil checks with impl/path-data coercion in all public
wrapper functions in app.common.types.path. The path-data helper
already handles nil by returning an empty PathData instance, which
provides uniform nil-safety across all content-accepting functions.
Update the path-get-points-nil-safe test to expect empty collection
instead of nil, matching the new coercion behavior.
* ♻️ Clean up path segment dead code and add missing tests
Remove dead code from segment.cljc: opposite-handler (duplicate of
calculate-opposite-handler) and path-closest-point-accuracy (unused
constant). Make update-handler and calculate-extremities private as
they are only used internally within segment.cljc.
Add missing tests for path/handler-indices, path/closest-point,
path/make-curve-point and path/merge-nodes. Update extremities tests
to use the local reference implementation instead of the now-private
calculate-extremities. Remove tests for deleted/privatized functions.
Add empty-content guard in path/closest-point wrapper to prevent
ArityException when reducing over zero segments.
* ✨ Expand interaction helper test coverage
Add coverage for interaction destination and flow helpers,
including nil handling and removal helpers. Document the
intent of the new assertions so future interaction changes
keep the helper contract explicit.
* ✨ Cover interaction validation edge cases
Exercise the remaining interaction guards and overlay
positioning edge cases, including invalid state
transitions and nested manual offsets. Keep the test
comments focused on why each branch matters for editor
behavior.
Return nil from get-prev-sibling when the shape is no longer present in
the parent ordering so delete undo generation falls back to index-based
restore instead of crashing on invalid vector access.
* 🐛 Fix crash in apply-text-modifier with nil selrect or modifier
Guard apply-text-modifier against nil text-modifier and nil selrect
to prevent the 'invalid arguments (on pointer constructor)' error
thrown by gpt/point when called with an invalid map.
- In text-wrapper: only call apply-text-modifier when text-modifier is
not nil (avoids unnecessary processing)
- In apply-text-modifier: handle nil text-modifier by returning shape
unchanged; guard selrect access before calling gpt/point
* 📚 Add tests for apply-text-modifier in workspace texts
Add exhaustive unit tests covering all paths of apply-text-modifier:
- nil modifier returns shape unchanged (identity)
- modifier with no recognised keys leaves shape unchanged
- :width / :height modifiers resize shape correctly
- nil :width / :height keys are skipped
- both dimensions applied simultaneously
- :position-data is set and nil-guarded
- position-data coordinates translated by delta on resize
- shape with nil selrect + nil modifier does not throw
- position-data-only modifier on shape without selrect is safe
- selrect origin preserved when no dimension changes
- result always carries required shape keys
* 🐛 Fix zero-dimension selrect crash in change-dimensions-modifiers
When a text shape is decoded from the server via map->Rect (which
bypasses make-rect's 0.01 minimum enforcement), its selrect can have
width or height of exactly 0. change-dimensions-modifiers and
change-size were dividing by these values, producing Infinity scale
factors that propagated through the transform pipeline until
calculate-selrect / center->rect returned nil, causing gpt/point to
throw 'invalid arguments (on pointer constructor)'.
Fix: before computing scale factors, guard sr-width / sr-height (and
old-width / old-height in change-size) against zero/negative and
non-finite values. When degenerate, fall back to the shape's own
top-level :width/:height so the denominator and proportion-lock base
remain consistent.
Also simplify apply-text-modifier's delta calculation now that the
transform pipeline is guaranteed to produce a valid selrect, and
update the test suite to test the exact degenerate-selrect scenario
that triggered the original crash.
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
* ♻️ Simplify change-dimensions-modifiers internal logic
- Remove the intermediate 'size' map ({:width sr-width :height sr-height})
that was built only to be assoc'd and immediately destructured back into
width/height; compute both values directly instead.
- Replace the double-negated condition 'if-not (and (not ignore-lock?) …)'
with a clear positive 'locked?' binding, and flatten the three-branch
if-not/if tree into two independent if expressions keyed on 'attr'.
- Call safe-size-rect once and reuse its result for both the fallback
sizes and the scale computation, eliminating a redundant call.
- Access :transform and :transform-inverse via direct map lookup rather
than destructuring in the function signature, consistent with how the
rest of the let-block reads shape keys.
- Clean up change-size to use the same destructuring style as the updated
function ({sr-width :width sr-height :height}).
- Fix typo in comment: 'havig' -> 'having'.
* ✨ Add tests for change-size and change-dimensions-modifiers
Cover the main behavioural contract of both functions:
change-size:
- Scales both axes to the requested target dimensions.
- Sets the resize origin to the shape's top-left point.
- Nil width/height each fall back to the current dimension (scale 1 on
that axis); both nil produces an identity resize that is optimised away.
- Propagates the shape's transform and transform-inverse matrices into the
resulting GeometricOperation.
change-dimensions-modifiers:
- Changing :width without proportion-lock only scales the x-axis (y
scale stays 1), and vice-versa for :height.
- With proportion-lock enabled, changing :width adjusts height via the
inverse proportion, and changing :height adjusts width via the
proportion.
- ignore-lock? true bypasses proportion-lock regardless of shape state.
- Values below 0.01 are clamped to 0.01 before computing the scale.
- End-to-end: applying the returned modifiers via gsh/transform-shape
yields the expected selrect dimensions.
* ✨ Harden safe-size-rect with additional fallbacks
The previous implementation could still return an invalid rect in several
edge cases. The new version tries four sources in order, accepting each
only if it passes a dedicated safe-size-rect? predicate:
1. :selrect – used when width and height are finite, positive
and within [-max-safe-int, max-safe-int].
2. points->rect – computed from the shape corner points; subject to
the same predicate.
3. Top-level shape fields (:x :y :width :height) – present on all rect,
frame, image, and component shape types.
4. grc/empty-rect – a 0,0 0.01×0.01 unit rect used as last resort so
callers always receive a usable, non-crashing value.
The out-of-range check (> max-safe-int) is new: it rejects coordinates
that pass d/num? (finite) but exceed the platform integer boundary defined
in app.common.schema, which previously slipped through undetected.
Tests cover all four fallback paths, including the NaN, zero-dimension,
and max-safe-int overflow cases.
* ⚡ Optimise safe-size-rect for ClojureScript performance
- Replace (when (some? rect) ...) with (and ^boolean (some? rect) ...)
to keep the entire predicate as a single boolean expression without
introducing an implicit conditional branch.
- Replace keyword access (:width rect) / (:height rect) with
dm/get-prop calls, consistent with the hot-path style used throughout
the rest of the namespace.
- Add ^boolean type hints to every sub-expression of the and chain in
safe-size-rect? (d/num?, pos?, <=) so the ClojureScript compiler emits
raw JS boolean operations instead of boxing the results through
cljs.core/truth_.
- Replace (when (safe-size-rect? ...) value) in safe-size-rect with
(and ^boolean (safe-size-rect? ...) value), avoiding an extra
conditional and keeping the or fallback chain free of allocated
intermediate objects.
* ✨ Use safe-size-rect in apply-text-modifier delta-move computation
safe-size-rect was already used inside change-dimensions-modifiers to
guard the resize scale computation. However, apply-text-modifier in
texts.cljs was still reading (:selrect shape) and (:selrect new-shape)
directly to build the delta-move vector via gpt/point.
gpt/point raises "invalid arguments (on pointer constructor)" when
given a nil value or a map with non-finite :x/:y, which can happen when
a shape's selrect is missing or degenerate (e.g. decoded from the server
via map->Rect, bypassing make-rect's 0.01 floor).
Changes:
- Promote safe-size-rect from defn- to defn in app.common.types.modifiers
so it can be reused by consumers outside the namespace.
- Replace the two raw (:selrect …) accesses in the delta-move computation
with (ctm/safe-size-rect …), which always returns a valid, finite rect
through the established four-step fallback chain.
- Add two frontend tests covering the delta-move path with a fully
degenerate (zero-dimension) selrect, ensuring neither a bare
position-data modifier nor a combined width+position-data modifier
throws.
* ♻️ Ensure all test shapes are proper Shape records in modifiers-test
All shapes in safe-size-rect-fallbacks tests now start from a proper
Shape record built by cts/setup-shape (via make-shape) instead of plain
hash-maps. Each test that mutates geometry fields (selrect, points,
width, height) does so via assoc on the already-initialised record,
which preserves the correct type while isolating the field under test.
A (cts/shape? shape) assertion is added to each fallback test to make
the type guarantee explicit and guard against regressions.
The unused shape-with-selrect helper (which built a bare map) is
removed.
* 🔥 Remove dead code and tighten visibility in app.common.types.modifiers
Dead functions removed (zero callers across the entire codebase):
- modifiers->transform-old: superseded by modifiers->transform; only
ever appeared in a commented-out dev/bench.cljs entry.
- change-recursive-property: no callers anywhere.
- move-parent-modifiers, resize-parent-modifiers: convenience wrappers
for the parent-geometry builder functions; never called.
- remove-children-modifiers, add-children-modifiers,
scale-content-modifiers: single-op convenience builders; never called.
- select-structure: projection helper; only referenced by
select-child-geometry-modifiers which is itself dead.
- select-child-geometry-modifiers: no callers anywhere.
Functions narrowed from defn to defn- (used only within this namespace):
- valid-vector?: assertion helper called only by move/resize builders.
- increase-order: called only by add-modifiers.
- transform-move!, transform-resize!, transform-rotate!, transform!:
steps of the modifiers->transform pipeline.
- modifiers->transform1: immediate helper for modifiers->transform; the
doc-string describing it as 'multiplatform' was also removed since it
is an implementation detail.
- transform-text-node, transform-paragraph-node: leaf helpers for
scale-text-content.
- update-text-content, scale-text-content, apply-scale-content: internal
scale-content pipeline; all called only by apply-modifier.
- remove-children-set: called only by apply-modifier.
- select-structure: demoted to defn- rather than deleted because it is
still called by select-child-structre-modifiers, which has external
callers.
* ✨ Add more tests for modifiers
---------
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
* 🐛 Fix nil deref on missing bounds in layout modifier propagation
When a parent shape has a child ID in its shapes vector that does
not exist in the objects map, the layout modifier code crashes
because it derefs nil from the bounds map.
The root cause is that children from the parent shapes list are
not validated against the objects map before being passed to the
layout modifier pipeline. Children with missing IDs pass through
unchecked and reach apply-modifiers where bounds lookup fails.
Fix by adding nil guards in apply-modifiers to skip children
without bounds, and changing map to keep to filter them out.
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
* 📎 Add tests for nil bounds in layout modifier propagation
Tests cover flex and grid layout scenarios where a parent
frame has child IDs in its shapes vector that do not exist
in the objects map, verifying that set-objects-modifiers
handles these gracefully without crashing.
Tests:
- Flex layout with normal children (baseline)
- Flex layout with non-existent child in shapes
- Flex layout with only non-existent children
- Grid layout with non-existent child in shapes
- Flex layout resize propagation with ghost children
- Nested flex layout with non-existent child in outer frame
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
---------
Signed-off-by: Andrey Antukh <niwi@niwi.nz>
