github.com/HakanSeven12/OpenCADStudio discussion #1052: constructing a
3-4-5 right triangle by drawing two circles (radius 3 and 4, centers 5
apart) and using OSNAP Intersection to find their crossing points gave
measured side lengths that didn't match the entered radii.
OSNAP's Intersection snap found candidate points by sweeping line
segments from each wire's *tessellated* polyline approximation, never
the entity's real geometry. Circles feeding the resident wire set
always tessellate at a fixed 48 segments (Scene::resident_wires_for
passes wpp=None deliberately, to avoid re-tessellating on every camera
pan/zoom) regardless of radius or zoom — so every circle/arc/ellipse
intersection snap was already off by construction, not just at extreme
zoom levels. Arcs, ellipses, and splines have the same problem via
their own fixed-angle tessellation.
Added an exact intersection path that bypasses tessellation entirely,
used by both call sites in the Intersection-snap code (the indexed/
local fast path and the brute-force wire-pair fallback), falling back
to the original tessellated sweep for anything it can't resolve:
- exact_circle_intersections: closed-form circle/circle intersection
for any pair of Circle/Arc entities (frame-independent — each side's
own arc sweep is checked against its own native axis, so two arcs in
differently rotated coordinate frames still resolve correctly).
- exact_curve_intersections: a more general path for anything
involving a line or an ellipse — embeds both entities into a shared
working plane and dispatches to cadkernel::geom2d::intersect (an
exact/tolerance-converged solver already vendored in this repo's own
dependency but previously unused for this purpose). Handles a
reflection hazard the naive version would miss: when an entity's own
normal is anti-parallel to the shared embedding frame's normal, the
mapping mirrors the parametrization and reverses which physical arc
a [start, end] range refers to — both the Arc and PlanarEllipse
branches detect this and swap start/end to compensate.
The only remaining case still going through the tessellated sweep is a
compound polyline chain with a bulge segment (not a single recognized
curve — it would need its own per-segment bookkeeping this pass
doesn't add).
12 new tests, including the reported 3-4-5 case verified against the
exact expected crossing points, arcs in differently rotated frames,
and the anti-parallel-normal reflection case for both Arc and Ellipse
(independently verified via a Python script for exact precision).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Material map images (diffuse/specular/reflection/opacity/bump/
refraction/normal) were decoded at their original file dimensions and
uploaded to wgpu without checking device.limits().max_texture_dimension_2d,
unlike raster-image rendering, which already tiles to that same limit.
An oversized material map produced a wgpu validation error and left
the material unusable; the global error handler prevented a process
abort, but never produced a valid texture either.
A mesh material is sampled by UV across a surface, so it can't use the
same per-quad tiling trick raster images use. Instead,
upload_rgba_texture now checks the decoded image against the device's
texture-size limit and downscales it (aspect-preserving, via
image::imageops::resize) before upload when it's oversized, falling
back to the existing 1x1 material color if the decoded buffer is
malformed (its length doesn't match its reported dimensions).
3 new tests cover the resize math (aspect ratio preserved, both
dimensions respect the limit) and the malformed-input fallback.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Automation's "open" op called io::load_bytes directly and installed
the result as-is, skipping everything the canonical path-based loader
(load_file_for_open / finalize_loaded_outcome) runs after parsing:
block-origin normalization for legacy non-zero-origin blocks,
relative raster/material path resolution, source_path assignment,
and the corrupt-entity purge the UI open path always applies. The
same file opened two ways behaved differently, and headless/automation
use was more exposed to malformed geometry with no filter at all.
Added io::load_bytes_finalized(path, bytes), which runs load_bytes
plus that same normalization and purge using the real path (not just
its basename, which the old code passed to load_bytes as the "name"
parameter — losing the directory needed for raster-path resolution
and source_path). automation.rs's "open" handler now uses it and
reports the purge count as a "purged" field in the response.
load_bytes itself is unchanged, since the byte round-trip tests
(sketch_persist.rs, named_parameters_persist.rs) deliberately want
the simpler byte-only path with no real filesystem path behind it.
New test constructs a document with one normal and one corrupt
(zero-radius) entity, saves it, opens it through automation, and
checks the corrupt entity was purged and source_path was set —
proving the automation path now matches the UI path.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two independent MINSERT (array INSERT) bugs found while auditing
render_graph.rs:
1. array_offsets() reserved and filled one Vec element per array
instance, with row_count/column_count parsed as unchecked u16s.
A corrupt or adversarial 65535x65535 pair requests 4,294,836,225
instances (~96 GiB) before any GPU-side chunking limit can help,
and before the render graph even reaches the GPU. Fixed with a
documented instance budget (20,000) enforced at both the load-time
corrupt-entity filter (is_entity_corrupt, matching the existing
polyline/spline vertex-count guards) and a failsafe clamp in
array_offsets itself, for an insert built or edited some other way.
2. insert_instance_transform composed the per-instance row/column
offset translation *before* the INSERT's own transform, so the
block's x/y scale multiplied the spacing along with the block
contents — an INSERT with x-scale 2 and column spacing 10 rendered
20 units between columns instead of 10. The pinned CAD model's own
Insert::array_points() confirms the correct behavior: spacing
rotates with the insert but is never scaled. Fixed by orienting the
offset with rotation + OCS only, then translating the already-
composed (scaled) instance transform by that world-space vector.
7 new tests cover both: pathological/reasonable instance counts at
the load-time filter and the render-graph failsafe, and instance
transforms for scaled and rotated arrays.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Handling a document-open Apple Event (a warm "Open With" while the app
is already running) silently promotes this process from
NSApplicationActivationPolicyAccessory to a normal foreground app —
confirmed with lsappinfo, not assumed: its own entry flips from
type="UIElement" to type="Foreground" the instant it handles a second
Apple Event (a cold launch's own first event doesn't trigger this).
That gave the launcher its own, contentless Dock icon sitting next to
the real editor's, both apparently "the same app" to the user but
actually two separate Dock entries.
Re-set Accessory explicitly right after relaying, in the one place
every open (cold fallback, document open, and reopen-while-running)
already funnels through.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Quitting the real GUI binary (Cmd+Q) leaves the launcher itself still
running — that's by design, so it can keep relaying opens for the
session. But reopening the app afterward (Dock icon, double-clicking
it in Finder again) reactivates that same long-lived launcher process
rather than triggering a fresh cold launch, since Launch Services
still considers the bundle running. Neither of the delegate methods
already handled covered that: applicationDidFinishLaunching: only
fires once, at the launcher's own original cold start, and
application:openURLs: only fires when a document was named. The net
effect: the Dock icon would appear, but no window ever showed.
applicationShouldHandleReopen:hasVisibleWindows: is exactly the
callback AppKit fires for this reactivation case. Reproduced and
verified the fix against the real sequence (cold launch, kill only the
GUI process to simulate Cmd+Q, reopen) rather than assuming it from
the delegate method's docs alone.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The previous commit's Apple Event handler only ever catches an open
sent to an already-running process. A cold double-click (app not
running yet) still failed: every registration point tried inside the
real binary — before app::run(), on the Subscription's first poll,
even the literal first statement of main() — reliably caught a warm
open but never the original cold-launch event. A raw
AEInstallEventHandler in a separate, minimal, non-AppKit process (no
winit/iced, just a manual CFRunLoopRunInMode pump) still missed it
too, ruling out "the real binary's heavy dependency graph loading
before main() wins the race" as the explanation.
What does reliably catch a cold launch, verified against real
Finder-equivalent opens: a from-scratch NSApplication with its own
delegate implementing application:openURLs: — in a process where we
own setDelegate: from the start, with no contention over the single
delegate slot winit already claims. That has to be a separate process
from the real GUI binary (only one -[NSApplication run] call can own a
process), so src/bin/ocs_launcher.rs is now the .app's actual
CFBundleExecutable: a small, dependency-light binary (no iced, wgpu,
or cadkernel) whose only job is to catch the open and relay it.
Relaying by replacing this process's image (exec, same PID) or by
spawning a child and exiting turned out to both be dead ends: Launch
Services tracks "the running instance of this bundle" by the
Application Serial Number it handed out to whichever process it
actually launched — this launcher, not whatever it execs into or
spawns. Once the launcher's own ASN stopped pointing at a live,
listening process, every later open Finder tried to deliver silently
went nowhere (measured with lsappinfo against a real second "Open
With", not assumed). So the launcher now stays running for the app's
session — invisible, via NSApplicationActivationPolicyAccessory, since
it owns no window — and relays every open (cold or warm) over the same
channel a second CLI invocation already uses: io::single_instance.
That relay only worked once single_instance's rendezvous key stopped
being computed from the running executable's own path. That was fine
when one binary always did the job, but the launcher and the real
binary are different executables and independently computed different
ports, so they could never find each other. Anchor the key on the
enclosing .app bundle instead, shared by both.
packaging/build_macos_signed.sh and .github/workflows/release.yml now
build ocs_launcher and place it at Contents/MacOS/OpenCADStudio (the
CFBundleExecutable Info.plist already names), with the real GUI binary
renamed to Contents/MacOS/OpenCADStudio-App alongside it.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Double-click / "Open With" never puts the file on argv — Finder always
delivers it as a kAEOpenDocuments Apple Event instead, which nothing in
this app listened for, so it just showed "cannot open files in this
format" even though the file association itself was registered
correctly.
The standard fix, implementing application:openURLs: on this app's own
NSApplicationDelegate, isn't available here: winit already installs
its own delegate to own window/lifecycle events, NSApplication has
exactly one delegate slot, and winit's class doesn't implement the
open-document method.
This bypasses the delegate protocol and registers directly with the
lower-level Apple Event Manager (AEInstallEventHandler) instead — the
same mechanism AppKit's delegate forwarding is itself built on, and
independent of who owns the delegate. Registration happens on the
iced Subscription's first poll rather than in main() before app::run()
— registering earlier gets silently overwritten by winit's own
NSApplication startup, confirmed by testing against a real `open -a`
rather than assumed from Apple's docs.
This handles a file opened while the app is already running. A cold
launch (app not running yet) needs a different mechanism, in the
following commit.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
