1b-agent/OneCAD-Draft
1
0 Fork 0
forked from 1b-agent/opencadstudio-upstream
Code Pull Requests Activity

fix: keep exact snapping, source drawing data and adaptive options layout

Browse Source 
Resolve curve intersections through cadkernel, retain empty exact results, gather analytic candidates independently of tessellation, and bound material uploads. Preserve large MINSERT data while limiting rendering work; remove obsolete review narratives.
This commit is contained in:
Hakan Seven committed 2026-09-09 13:17:30 +03:00
1 parent 4bb6aac0b6
commit 973af87d58
16 files changed
+57 -724
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

src/scene/entity.rs
-4
View File
@@ -298,10 +298,6 @@ impl Scene {
};
if !handle.is_null() {
// New geometry drawn during REFEDIT belongs to the edited block,
// so it must stay bright along with the copied block entities.
// The remaining model-space entities are only visible as faded
// context.
if let Some(keep) = self.refedit_keep.as_mut() {
keep.insert(handle);
}
src/scene/group_layer.rs
-12
View File
@@ -183,14 +183,6 @@ impl Scene {
handles: &[Handle],
) -> HashSet<Handle> {
let mut expanded: HashSet<Handle> = handles.iter().copied().collect();
// Asked the other way round, this was quadratic: for every group, scan
// every selected handle, and `Group::contains` is a linear scan of the
// group's own entity list. A box selection of 185 096 entities spent
// seconds here.
//
// The membership set is the *input* handles, not `expanded` — a group
// must match because something the user selected is in it, not because
// an earlier group already contributed one of its members.
let wanted: HashSet<Handle> = handles.iter().copied().collect();
for obj in self.document.objects.values() {
if let ObjectType::Group(g) = obj {
@@ -218,10 +210,6 @@ mod group_expansion_tests {
use super::*;
use crate::scene::Scene;
/// Selecting one member of a selectable group selects the whole group, and
/// a group nobody touched stays out. The lookup was inverted for speed —
/// it asked every group about every selected handle, and `Group::contains`
/// scans the group's own list — so the answer has to be shown unchanged.
#[test]
fn group_expansion_pulls_in_siblings_and_nothing_else() {
use acadrust::entities::{EntityType, Line};
src/scene/mod.rs
-12
View File
@@ -1877,12 +1877,6 @@ pub struct Scene {
entity_block_map_cache: RefCell<Option<(u64, HashMap<Handle, Handle>)>>,
/// Candidate handles per block, in document order and keyed by geometry epoch.
block_members_cache: RefCell<Option<BlockMembers>>,
/// Annotation handle -> the LEADERs that point at it, resolved once per
/// `geometry_epoch`.
///
/// Selecting an entity has to pull in a LEADER whose annotation it is, and
/// that used to be answered by walking the whole document — per selected
/// handle. Selecting N entities cost N document walks.
leaders_by_annotation_cache: RefCell<Option<(u64, HashMap<Handle, Vec<Handle>>)>>,
/// Insert/Viewport/Block/BlockEnd handles omitted by the spatial index.
unindexable_cache: RefCell<Option<(u64, Vec<Handle>)>>,
@@ -7865,12 +7859,6 @@ impl Scene {
(Arc::new(wires), base_slots, changed_slots)
}
/// `area_only` skips the snap-only categories and gathers the rest without
/// sorting; see [`InteractionIndex::query_xy_area`]. Both the overlay path
/// and the plain indexed path honour it — the overlay runs whenever the
/// drawing has been edited since the index was built, which is most of a
/// working session, so a saving that stopped there would stop the first
/// time the user drew something.
fn indexed_interaction_candidates_xy(
&self,
wires: Arc<Vec<WireModel>>,
src/scene/pick/hit_test.rs
-41
View File
@@ -1271,28 +1271,6 @@ fn indexed_box_crossing_hits<'a, W: WireSource + ?Sized>(
segment_hits(a, b).then_some(wire.name.as_str())
};
// A crossing selection over a whole drawing spends its time here, not in
// the window branch of `box_hit`: every measured `select-commit` line read
// `crossing=true`.
//
// The scan stops projecting a wire once that wire is in. A circle or a
// polyline contributes one segment per tessellated span, and every span
// after the first that hits can only re-add a name already present — at 24
// spans per wire that is 23 projection pairs out of 24 skipped, which took
// a 4.4 M-segment scan from ~1 s to 42 ms. The wire is keyed by index, an
// integer to hash rather than a string, and the name set still runs behind
// it because one entity can produce several wires sharing a name.
//
// Fanning this out across cores was tried and removed: with the early-out
// the remaining work is small enough that per-chunk sets, which cannot see
// wires already hit in another chunk, measured *slower* than one pass
// (45.6 ms against 42.4 ms at 186 468 wires).
// A bitmap over wire indices rather than a hash set: this is consulted once
// per segment, triangle and glyph inside the box — millions of times on a
// whole-drawing selection — and an array index beats hashing a `u32`, let
// alone hashing the wire's name, which is what the primitive loops below
// used to do. `seen` stays for the names, because several wires can share
// one entity's name and the output dedups by name.
let mut wire_hit: Vec<bool> = Vec::new();
fn mark(wire_hit: &mut Vec<bool>, wire: u32) {
let index = wire as usize;
@@ -1377,10 +1355,6 @@ fn indexed_box_crossing_hits<'a, W: WireSource + ?Sized>(
// Degenerate point-only wires have no indexed segment or surface primitive.
for wire in wires.iter() {
// Structural tests first: `||` short-circuits, so putting the name
// lookup last means a wire with two or more points — very nearly all
// of them — is dropped without hashing its name. This loop walks every
// candidate, so that was one string hash per wire in the box.
if wire.points.len() >= 2
|| !wire.fill_tris.is_empty()
|| !wire.pick_tris.is_empty()
@@ -1591,10 +1565,6 @@ fn indexed_polygon_crossing_hits<'a, W: WireSource + ?Sized>(
}
}
for wire in wires.iter() {
// Structural tests first: `||` short-circuits, so putting the name
// lookup last means a wire with two or more points — very nearly all
// of them — is dropped without hashing its name. This loop walks every
// candidate, so that was one string hash per wire in the box.
if wire.points.len() >= 2
|| !wire.fill_tris.is_empty()
|| !wire.pick_tris.is_empty()
@@ -2773,13 +2743,6 @@ mod parallel_selection_tests {
use crate::scene::pick::interaction_index::{InteractionCandidates, InteractionIndex};
use std::sync::Arc;
// Enough wires to clear the 4096 default several times over, laid on a
// diagonal so the selection box catches a known prefix of them rather than
// all or nothing — an "everything hits" case would not notice a reordering.
// A drawing is not made of single segments: a circle or an arc tessellates
// into dozens of spans, and a polyline carries as many as it has vertices.
// A one-segment-per-wire sample makes the per-wire early-out invisible and
// the whole measurement optimistic, so the benchmark uses `spans` per wire.
fn many_wires_with_spans(count: usize, spans: usize) -> Vec<WireModel> {
(0..count)
.map(|i| {
@@ -2804,10 +2767,6 @@ mod parallel_selection_tests {
.collect()
}
// Not a correctness test — a measurement, at the size the user's drawing
// actually is. Run it deliberately:
//
// cargo test --release --lib selection_scaling -- --ignored --nocapture
#[test]
#[ignore = "measurement, not a check"]
fn selection_scaling_numbers() {
src/scene/pick/interaction_index.rs
-71
View File
@@ -458,20 +458,6 @@ impl SpatialGrid {
}
}
/// The same set as [`SpatialGrid::query`], in cell order and with
/// duplicates left in.
///
/// `query` sorts twice over the result — once to dedup the entry indices an
/// entry gets for spanning several cells, once to dedup the values — which
/// over a box covering the drawing is two O(n log n) passes across 4.4 M
/// elements to remove a handful of repeats. Everything an area selection
/// does with the result is order-independent and dedups anyway: the
/// crossing scan keys wires by index and names by string, and the box test
/// on a segment does not care when it runs.
///
/// Click picking keeps [`SpatialGrid::query`]: it resolves ties between
/// overlapping geometry by scan order, so its order is part of which entity
/// a click selects.
fn query_unordered<T: Copy>(&self, entries: &[Entry3<T>], query: [f64; 4]) -> Vec<T> {
let mut indices = Vec::new();
self.oversized.query(entries, query, &mut indices);
@@ -1211,13 +1197,6 @@ impl InteractionIndex {
.collect()
}
/// [`InteractionIndex::query_remapped_xy`] for an area selection.
///
/// The overlay path runs whenever the drawing has been edited since the
/// interaction index was built — that is, for most of a working session —
/// so leaving it on the full query would have meant the area selection got
/// its saving only until the user drew something. Same five categories,
/// same unordered gather.
pub(crate) fn query_remapped_xy_area(
&self,
wires: Arc<Vec<WireModel>>,
@@ -1397,34 +1376,11 @@ impl InteractionIndex {
}
}
/// Candidates for an area selection — box, crossing or fence.
///
/// `query_xy` runs eight spatial queries because hover and snapping need
/// them all. An area selection needs five: the wires and their segments,
/// and — because `indexed_box_crossing_hits` reads them to catch hatches
/// and text inside the box — the fill triangles, pick triangles and
/// glyphs. Only the three that exist for snapping (`snap_points`,
/// `key_vertices`, `key_segments`, read solely by `pick::snap::snap`) go
/// unread, so only those are skipped.
///
/// Over a box covering the whole drawing each query gathers nearly
/// everything, which is why this is worth splitting: gathering candidates
/// was 407 ms of an 825 ms selection of 186 468 entities.
///
/// The skipped categories stay `None`. That is safe *only* for consumers
/// that never read them — `snap` would silently stop snapping rather than
/// fail, since the accessors treat `None` as "no index". Route anything
/// that snaps through `query_xy`.
pub fn query_xy_area(
&self,
wires: Arc<Vec<WireModel>>,
aabb: [f64; 4],
) -> InteractionCandidates {
// The five are independent tree walks over the same box, and over a box
// covering the drawing each one gathers nearly everything — 259 ms
// together. Running them concurrently costs the largest instead of the
// sum. Each closure still calls the same query, so the results are
// identical to the sequential ones, order included.
let ((wire_indices, segments), ((fill_triangles, pick_triangles), glyphs)) =
crate::par::join(
|| {
@@ -1703,16 +1659,6 @@ impl InteractionCandidates {
.map(|(rect, (view, eye, bounds))| (rect, view, eye, bounds))
}
/// [`InteractionCandidates::extend_indexed`] for an area selection.
///
/// `extend_indexed` sorts and dedups every category it merges, which over a
/// box covering the drawing would put back exactly the sorts the unordered
/// query exists to avoid.
///
/// `wire_indices` keeps its dedup, and must: the window branch of `box_hit`
/// walks it directly, so a wire listed twice would be selected twice. The
/// other categories are read by scans that dedup by wire index and by name
/// already, so a repeat there costs one extra test and changes nothing.
pub(crate) fn extend_indexed_area(&mut self, other: Self) {
debug_assert!(Arc::ptr_eq(&self.wires, &other.wires));
@@ -2081,11 +2027,6 @@ mod area_query_tests {
]
}
// The area query drops the three snap-only categories and must leave every
// category an area selection reads untouched. `unwrap_or_default()` in
// `indexed_box_crossing_hits` would turn a wrongly-dropped category into a
// silent miss — a hatch or a text that stops being selectable — rather than
// a failure, so the equivalence is asserted rather than assumed.
#[test]
fn area_query_matches_full_query_where_it_is_read() {
let wires = sample();
@@ -2096,13 +2037,6 @@ mod area_query_tests {
let full = index.query_xy(Arc::clone(&arc), aabb);
let area = index.query_xy_area(Arc::clone(&arc), aabb);
// The area query skips the sort and the dedup `query` pays for, so it
// returns the same *set* in cell order, with an entry repeated once per
// cell it spans. Every consumer on this path dedups already — the
// crossing scan by wire index and by name — so the set is the contract,
// and comparing sequences here would only pin down an order nothing
// reads. `wire_indices` is the exception: `query_xy_area` sorts and
// dedups it before returning, so it must match exactly.
fn normalise<T: Copy + Ord>(v: &Option<Vec<T>>) -> Option<Vec<T>> {
v.as_ref().map(|items| {
let mut items = items.clone();
@@ -2137,11 +2071,6 @@ mod area_query_tests {
assert!(full.snap_points.is_some(), "the full query still snaps");
}
// The overlay merge drops the sort on every category but one. That one
// matters: the window branch of `box_hit` walks `wire_indices` directly, so
// a wire left in twice would be selected twice — a duplicate handle rather
// than an error, which is the kind of thing that surfaces as a wrong count
// somewhere far away.
#[test]
fn the_area_merge_dedups_wire_indices_and_nothing_else() {
let arc = Arc::new(sample());
src/scene/pipeline/mesh_gpu.rs
+9 -11
View File
@@ -568,12 +568,15 @@ struct MeshSurfaceParams {
flags: [u32; 4],
}
/// Downscale a decoded RGBA buffer so neither dimension exceeds `limit`,
/// preserving aspect ratio (scaled by whichever dimension overshoots more).
/// `None` only for genuinely malformed input (`rgba`'s length doesn't match
/// `width * height * 4`) — the caller falls back to the 1x1 material color
/// in that case, same as it already does for a missing image entirely.
fn valid_rgba_size(width: u32, height: u32, pixels: &[u8]) -> bool {
width > 0 && height > 0
&& (width as usize).checked_mul(height as usize).and_then(|size| size.checked_mul(4)) == Some(pixels.len())
}
fn downscale_rgba_to_limit(width: u32, height: u32, rgba: &[u8], limit: u32) -> Option<(u32, u32, Vec<u8>)> {
if limit == 0 || !valid_rgba_size(width, height, rgba) {
return None;
}
let buffer = image::RgbaImage::from_raw(width, height, rgba.to_vec())?;
let scale = (limit as f64 / width.max(height) as f64).min(1.0);
let new_width = ((width as f64 * scale).round() as u32).clamp(1, limit);
@@ -590,14 +593,9 @@ fn upload_rgba_texture(
fallback: [u8; 4],
srgb: bool,
) -> wgpu::TextureView {
// A material map's decoded dimensions come straight from the source file
// (`material_model::load_map_image`), uncapped — an oversized image would
// otherwise fail `wgpu` texture-size validation and leave the material
// unusable (audit: raster-image rendering already tiles to this same
// limit in `image_gpu.rs`; a mesh material sampled by UV can't use that
// per-quad tiling trick, so downscale instead of tiling).
let limit = device.limits().max_texture_dimension_2d.max(1);
let resized;
let image = image.filter(|image| valid_rgba_size(image.width, image.height, &image.rgba));
let (width, height, pixels): (u32, u32, &[u8]) = match image {
Some(image) if image.width > limit || image.height > limit => {
match downscale_rgba_to_limit(image.width, image.height, &image.rgba, limit) {
src/scene/pipeline/mod.rs
-38
View File
@@ -2701,13 +2701,6 @@ impl Pipeline {
// O(highlighted), no per-wire string parse or deep geometry clone.
let mut selected_wires: Vec<&WireModel> = Vec::new();
let mut hover_wires: Vec<&WireModel> = Vec::new();
// The index is built by walking the wires in order, so each handle's
// slots come out ascending, and `patch_handle_index` only shifts them
// by a constant, which keeps that. Cloning and re-sorting per handle
// was one allocation and one sort for every selected entity — 186 468
// of each on a whole-drawing selection. The assert holds the invariant
// in place: if it ever breaks, a debug build says so here rather than
// drawing the highlight in the wrong order.
for h in selected {
if let Some(idxs) = self.wire_handle_index.get(&h.value()) {
debug_assert!(
@@ -2747,25 +2740,6 @@ impl Pipeline {
}
}
let t_gather = perf_started.map(|t| t.elapsed().as_secs_f64() * 1000.0);
// One extraction pass per wire, where there used to be four.
//
// The old shape asked each extractor twice: once at the draw depth to
// build the instances, then once more at depth 0.0 purely to ask
// *whether* the wire is a circle or an ellipse — allocating a vector
// each time and dropping it. Neither extractor's `Some`/`None` answer
// depends on the depth: every `return None` in them turns on the
// geometry (which `TangentGeom` variant, the radius, the axis lengths,
// the parameters), and the depth is only written into the instance,
// never read to decide. So the extraction that builds the instances
// already answers the classification, and the second pair is pure
// waste — 116 ms of shapes plus 57 ms of splitting, out of a 299 ms
// highlight at 186 468 entities.
//
// A wire with a `render_instance` is a block wire, and both extractors
// reject those outright, so it skips extraction altogether.
// Selected keeps its colour unless a tint is configured; hover is
// always recoloured. Both vectors carry selected first, then hover,
// exactly as the four separate loops produced them.
let (mut selected_circles, mut selected_ellipses, selected_regular, selected_blocks) =
Self::classify_highlight_wires(
&selected_wires,
@@ -2857,12 +2831,6 @@ impl Pipeline {
if let Some(started) = perf_started {
let elapsed_ms = started.elapsed().as_secs_f64() * 1000.0;
if elapsed_ms >= 1.0 {
// Cumulative marks, differenced here: gathering the wires
// from the handle index, classifying them and extracting their
// analytic instances, uploading those, building the regular
// wire instances, and the block ones. A one-time cost when the
// selection changes, which puts it directly between the user's
// second click and seeing the selection.
let gather = t_gather.unwrap_or(0.0);
let classify = t_shapes.unwrap_or(0.0);
let analytic = t_split.unwrap_or(0.0);
@@ -5725,12 +5693,6 @@ mod highlight_classification_tests {
wire
}
// The classification used to be a second pair of extractions at depth 0.0,
// run purely to ask which bucket a wire belongs in. It now rides the
// extraction that builds the instances. Nothing renders in a test, so a
// wrong answer here would show up only as circles drawn wrong when
// selected — this asserts the buckets directly, against the predicate the
// old code used.
#[test]
fn each_wire_lands_in_the_bucket_the_old_predicate_chose() {
let wires = vec![plain("1"), circle("2"), ellipse("3"), plain("4")];
src/scene/pipeline/wire_gpu.rs
-5
View File
@@ -1148,11 +1148,6 @@ impl WireGpu {
let _ = const_bgl;
let max_packed_instances = super::gpu_budget::max_elements::<PackedWireInstance>(device);
// Same fan-out the native path above uses: emitting a wire reads that
// wire and the depth map and writes nothing shared, and a selection
// large enough to be slow is exactly where it pays. `collect` on an
// indexed parallel iterator keeps the order, which the instance indices
// depend on.
use crate::par::prelude::*;
let per: Vec<Vec<PackedWireInstance>> = wires
.par_iter()
src/scene/render_graph.rs
-27
View File
@@ -1042,15 +1042,6 @@ pub fn document_block_uses(document: &CadDocument) -> Vec<BlockUse> {
uses
}
/// A MINSERT's row/column counts are parsed `u16`s whose unchecked product
/// (`Insert::instance_count`) would otherwise drive this function's
/// allocation and a full per-instance render-graph walk after it — a corrupt
/// or adversarial 65535x65535 pair requests over four billion instances (tens
/// of GiB) long before any GPU-side chunking limit could help. The primary
/// defense is the load-time filter in `io::is_entity_corrupt` (kept in sync
/// with this constant), which drops such an insert before it ever reaches a
/// `Scene`; this is the failsafe for an insert built or edited some other way
/// (API/scripting) that skips that path.
const MAX_MINSERT_INSTANCES: usize = 20_000;
pub fn array_offsets(insert: &Insert) -> Vec<[f64; 3]> {
@@ -1093,16 +1084,6 @@ pub fn insert_instance_transform(
if offset == [0.0; 3] {
transform
} else {
// Row/column spacing is defined in the insert's rotated/OCS-oriented
// space but is NOT subject to the block's x/y/z scale — this matches
// the pinned CAD model's own `Insert::array_points`, which rotates
// the offset but never scales it. Composing the raw offset ahead of
// the full (scaled) transform, as this used to do, let the block
// scale multiply the spacing too (e.g. an x-scale-2 insert with
// column spacing 10 would render 20 units between columns). Instead,
// orient the offset by rotation + OCS only, then translate the
// already-composed (scaled) instance transform by that world-space
// vector, applied after — never scaled.
let orientation = Transform::from_matrix(Matrix4::rotation_z(insert.rotation))
.then(&Transform::from_matrix(Matrix4::from_matrix3(Matrix3::arbitrary_axis(insert.normal))));
let world_offset = orientation.apply_rotation(Vector3::new(offset[0], offset[1], offset[2]));
@@ -1201,10 +1182,6 @@ mod tests {
#[test]
fn array_offsets_clamps_pathological_minsert_counts() {
// A corrupt or adversarial row/column pair (parsed as u16, so this is
// reachable from a malformed file) must not drive a multi-billion-
// element allocation; the primary defense is the load-time filter in
// `io::is_entity_corrupt`, this is the render-graph-side failsafe.
let mut insert = Insert::new("BLOCK", Vector3::ZERO);
insert.row_count = u16::MAX;
insert.column_count = u16::MAX;
@@ -1230,10 +1207,6 @@ mod tests {
#[test]
fn insert_instance_transform_does_not_scale_the_row_column_spacing() {
// github discussion-derived report: an INSERT with x-scale 2 and
// column spacing 10 was rendering 20 drawing units between columns
// instead of 10, because the offset translation was composed ahead
// of (and thus multiplied by) the block scale.
let document = CadDocument::new();
let mut insert = Insert::new("BLOCK", Vector3::new(100.0, 0.0, 0.0));
insert.set_x_scale(2.0);
src/scene/selection.rs
-28
View File
@@ -37,12 +37,6 @@ impl Scene {
})
}
/// The handles plus their leader/annotation partners, in the order given.
///
/// Kept separate from the sorted, deduplicated form because a bulk
/// selection has to preserve pick order — `selected_handles_in_order`
/// reports it — and because sorting per handle is what made selecting
/// 186 000 entities one call at a time expensive.
fn expanded_with_leaders(&self, handles: &[Handle]) -> Vec<Handle> {
let leaders = self.leaders_by_annotation();
let mut expanded = Vec::with_capacity(handles.len());
@@ -62,11 +56,6 @@ impl Scene {
expanded
}
/// Select many handles in one pass.
///
/// The single-handle path allocates, sorts and deduplicates for every
/// entity, and a box selection called it once per entity — most of the
/// remaining 786 ms of a 186 460-entity selection.
pub fn select_entities(&mut self, handles: &[Handle]) {
if handles.is_empty() {
return;
@@ -84,10 +73,6 @@ impl Scene {
}
}
/// Deselect many handles in one pass.
///
/// `deselect_entity` rebuilds `selected_order` with a `retain` per handle,
/// so removing N of M selected entities walked the order list N times.
pub fn deselect_entities(&mut self, handles: &[Handle]) {
if handles.is_empty() {
return;
@@ -1082,10 +1067,6 @@ impl Scene {
mod tests {
use super::*;
/// A box selection used to call `select_entity` once per entity. The bulk
/// path has to land on the same selection, in the same pick order —
/// `selected_handles_in_order` is read by commands such as DIM, where the
/// order the user picked things in decides what they mean.
#[test]
fn bulk_selection_matches_selecting_one_at_a_time() {
use acadrust::entities::Line;
@@ -1141,11 +1122,6 @@ mod tests {
);
}
/// Selecting an entity pulls in any LEADER pointing at it, and that was
/// answered by walking the whole document once per selected handle —
/// selecting 471 583 entities in a 585 786-entity drawing did not finish
/// inside eight minutes. The reverse index has to give exactly what the
/// walk gave.
#[test]
fn the_leader_index_matches_a_document_walk() {
use acadrust::entities::Leader;
@@ -1207,10 +1183,6 @@ mod tests {
);
}
/// The reason the cache carries a set as well as a list: drawing into a
/// drawing that already has that type must not rebuild either, because the
/// rebuild walks every entity the layout owns — 444 937 of them on the
/// reproducer, ~100 ms, on every single drawn point.
#[test]
fn adding_a_type_already_present_reuses_the_list() {
use acadrust::entities::{Circle, EntityType, Line};