review(#1627): one UPDATEFIELD, honest counts, safe field boxes, one-pass host redraw

- UPDATEFIELD stays the command main already had (Select objects, undo,
  locale strings); it now takes the PR's reach — a block reference's
  attributes and block definition, a table's cell texts — and reports
  the fields that really changed. The PR's second implementation and its
  duplicate registration are gone.
- FIELDEVAL's range error goes through the locale catalog.
- Field boxes: no slicing inside a character, one layout per multi-line
  attribute, and TEXT boxes scale about the annotative reference point
  like the glyphs do. Comment and indentation put back where they belong.
- bump_text_hosts classifies every host in one pass over the drawing
  instead of scanning all layouts and blocks per host.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

src/app/commands/count.rs

@@ -542,60 +542,54 @@ impl OpenCADStudio {
     }
 
     /// UPDATEFIELD: every field the objects host is evaluated again and its
-    /// text written back. A table's fields are hosted by the cell texts of its
-    /// drawing (its `*T` block); fields a table keeps without such a text are
-    /// evaluated when drawn and are not counted here.
+    /// text written back, whatever FIELDEVAL says. A block reference brings
+    /// its attributes and the texts of its block definition; a table, the
+    /// cell texts of its drawing (its `*T` block).
     fn update_fields(&mut self, i: usize, handles: &[Handle]) {
         let doc = &self.tabs[i].scene.document;
-        let mut found = 0;
-        let mut updates: Vec<(Handle, String)> = Vec::new();
+        let mut hosts = handles.to_vec();
         for h in handles {
-            let Some(entity) = doc.get_entity(*h) else { continue };
-            let hosts: Vec<Handle> = match entity {
-                EntityType::Table(t) => t
+            let record = match doc.get_entity(*h) {
+                Some(EntityType::Insert(insert)) => {
+                    hosts.extend(insert.attributes.iter().map(|a| a.common.handle));
+                    doc.block_records.get(&insert.block_name)
+                }
+                Some(EntityType::Table(t)) => t
                     .block_record_handle
                     .and_then(|r| doc.block_records.iter().find(|b| b.handle == r))
-                    .or_else(|| doc.block_records.get(&t.block_name).filter(|_| !t.block_name.is_empty()))
-                    .map(|record| record.entity_handles.clone())
-                    .unwrap_or_default(),
-                _ => vec![*h],
+                    .or_else(|| doc.block_records.get(&t.block_name).filter(|_| !t.block_name.is_empty())),
+                _ => None,
             };
-            for host in hosts {
-                if !doc.get_entity(host).is_some_and(|e| crate::entities::field::hosts_field(doc, e)) {
-                    continue;
-                }
-                found += 1;
-                if let Some(text) = crate::entities::field::resolve(doc, host) {
-                    updates.push((host, text));
-                }
+            if let Some(record) = record {
+                hosts.extend(record.entity_handles.iter().copied());
             }
         }
+        let attributes: Vec<&codec::entities::EntityCommon> = handles
+            .iter()
+            .filter_map(|h| match doc.get_entity(*h) {
+                Some(EntityType::Insert(insert)) => Some(insert.attributes.iter().map(|a| &a.common)),
+                _ => None,
+            })
+            .flatten()
+            .collect();
+        let found = hosts
+            .iter()
+            .filter(|h| match doc.get_entity(**h) {
+                Some(entity) => crate::entities::field::hosts_field(doc, entity),
+                None => attributes.iter().any(|common| {
+                    common.handle == **h && crate::entities::field::common_hosts_field(doc, common)
+                }),
+            })
+            .count();
         self.command_line.push_output(&crate::tf!("{found} field(s) found."));
         if found == 0 {
             return;
         }
         self.push_undo_snapshot(i, "UPDATEFIELD");
-        let mut changed = Vec::new();
-        for (h, text) in updates {
-            if let Some(entity) = self.tabs[i].scene.document.get_entity_mut(h) {
-                match entity {
-                    EntityType::MText(m) => m.value = text,
-                    EntityType::Text(t) => t.value = text,
-                    EntityType::AttributeDefinition(a) => a.default_value = text,
-                    _ => continue,
-                }
-                changed.push((h, crate::scene::ChangeKind::Modified));
-            }
+        let updated = self.tabs[i].scene.update_fields(32, Some(&hosts));
+        if updated > 0 {
+            self.tabs[i].dirty = true;
         }
-        let updated = changed.len();
-        // Table cell texts live in a block definition.
-        let scene = &mut self.tabs[i].scene;
-        if scene.changes_touch_block_definition(&changed) {
-            scene.bump_geometry();
-        } else {
-            scene.bump_entities(&changed);
-        }
-        self.tabs[i].dirty = true;
         self.command_line.push_output(&crate::tf!("{updated} field(s) updated."));
     }
 

src/app/commands/field.rs

@@ -17,48 +17,6 @@ impl OpenCADStudio {
                 self.open_field_dialog(FieldTarget::NewText);
                 Some(Task::none())
             }
-            // UPDATEFIELD — re-evaluate the fields of the selected objects
-            // (a block reference's attributes included), whatever FIELDEVAL.
-            "UPDATEFIELD" => {
-                let selected: Vec<codec::Handle> =
-                    self.tabs[i].scene.selected_entities().into_iter().map(|(h, _)| h).collect();
-                if selected.is_empty() {
-                    use crate::modules::draw::select::SelectObjectsCommand;
-                    let cmd = SelectObjectsCommand::new("UPDATEFIELD");
-                    self.command_line.push_info(&cmd.prompt());
-                    self.tabs[i].active_cmd = Some(Box::new(cmd));
-                    return Some(Task::none());
-                }
-                // A block reference brings its attributes and the fields of its
-                // block definition; a table, the texts of its cells.
-                let document = &self.tabs[i].scene.document;
-                let mut hosts = selected.clone();
-                for handle in &selected {
-                    let record = match document.get_entity(*handle) {
-                        Some(codec::EntityType::Insert(insert)) => {
-                            hosts.extend(insert.attributes.iter().map(|a| a.common.handle));
-                            document.block_records.iter().find(|r| r.name.eq_ignore_ascii_case(&insert.block_name))
-                        }
-                        Some(codec::EntityType::Table(table)) => table
-                            .block_record_handle
-                            .and_then(|h| document.block_records.iter().find(|r| r.handle == h)),
-                        _ => None,
-                    };
-                    if let Some(record) = record {
-                        hosts.extend(record.entity_handles.iter().copied());
-                    }
-                }
-                self.push_undo_snapshot(i, "UPDATEFIELD");
-                let found = self.tabs[i].scene.update_fields(32, Some(&hosts));
-                self.tabs[i].scene.deselect_all();
-                self.refresh_properties();
-                if found > 0 {
-                    self.tabs[i].dirty = true;
-                }
-                self.command_line.push_output(crate::tf!("{} field(s) found.", found).as_ref());
-                self.command_line.push_output(crate::tf!("{} field(s) updated.", found).as_ref());
-                Some(Task::none())
-            }
             // FIELDEVAL — the events that update fields, kept in the drawing.
             cmd if cmd == "FIELDEVAL" || cmd.starts_with("FIELDEVAL ") || cmd.starts_with("SETVAR FIELDEVAL") => {
                 let value = cmd
@@ -85,7 +43,9 @@ impl OpenCADStudio {
                         }
                     }
                     _ => {
-                        self.command_line.push_error("Requires an integer between 0 and 31.");
+                        let (min, max) = (0, 31);
+                        self.command_line
+                            .push_error(&crate::tf!("Requires an integer between {min} and {max}."));
                         ask(self);
                     }
                 }

src/app/commands/mod.rs

@@ -709,7 +709,6 @@ inventory::submit!(crate::command::CommandRegistration {
         "UOSNAP",
         "FIELDDISPLAY",
         "FIELDEVAL",
-        "UPDATEFIELD",
         "PDFIMPORTMODE",
         "PDFIMPORTFILTER",
         "PDFIMPORTLAYERS",

src/entities/field.rs

@@ -327,7 +327,13 @@ pub const BACKGROUND: [f32; 4] = [203.0 / 255.0, 203.0 / 255.0, 203.0 / 255.0, 1
 
 /// Whether `entity` hosts a field: its extension dictionary has ACAD_FIELD.
 pub fn hosts_field(document: &CadDocument, entity: &codec::entities::EntityType) -> bool {
-    let Some(xdict) = entity.common().xdictionary_handle else {
+    common_hosts_field(document, entity.common())
+}
+
+/// [`hosts_field`] for an entity known only by its common data, such as an
+/// attribute of a block reference.
+pub fn common_hosts_field(document: &CadDocument, common: &codec::entities::EntityCommon) -> bool {
+    let Some(xdict) = common.xdictionary_handle else {
         return false;
     };
     matches!(

src/scene/convert/tessellate.rs

@@ -99,6 +99,7 @@ fn field_run_corner_groups(
     entity: &EntityType,
     stroke_groups: &[crate::scene::convert::acad_to_render::TextStroke],
     anno: f64,
+    ref_origin: [f64; 2],
 ) -> Option<Vec<[[f64; 2]; 4]>> {
     use crate::entities::text_support::{
         mtext_visible_count, resolve_dxf_special_chars, resolve_text_style, text_char_cells, GlyphBox,
@@ -137,6 +138,10 @@ fn field_run_corner_groups(
         }
         Some(())
     };
+    let multiline_attribute = match entity {
+        EntityType::AttributeEntity(a) => crate::entities::attribute::attribute_cells(a, document),
+        _ => None,
+    };
     match entity {
         EntityType::MText(m) => {
             let spans = codec::fields::field_spans(document, handle, &m.value)?;
@@ -151,9 +156,10 @@ fn field_run_corner_groups(
                 m.rotation + flip,
             )?;
         }
-        EntityType::AttributeEntity(a) if crate::entities::attribute::attribute_cells(a, document).is_some() => {
+        // A multi-line attribute is laid out like MTEXT.
+        EntityType::AttributeEntity(a) if multiline_attribute.is_some() => {
             let spans = codec::fields::field_spans(document, handle, &a.value)?;
-            let (cells, origin, rotation) = crate::entities::attribute::attribute_cells(a, document)?;
+            let (cells, origin, rotation) = multiline_attribute?;
             from_cells(&mut lines, &cells, &a.value, &spans, origin, rotation as f64)?;
         }
         EntityType::Text(_) | EntityType::AttributeEntity(_) => {
@@ -171,7 +177,9 @@ fn field_run_corner_groups(
                 let run = group.run.as_ref()?;
                 let resolved = resolve_dxf_special_chars(line_text);
                 let cells = text_char_cells(&run.font, &resolved, run.height, run.width_factor);
-                let char_at = |byte: usize| resolve_dxf_special_chars(&line_text[..byte]).chars().count();
+                let char_at = |byte: usize| {
+                    line_text.get(..byte).map_or(0, |head| resolve_dxf_special_chars(head).chars().count())
+                };
                 for span in &spans {
                     let (a, b) = (span.start.max(range.start), span.end.min(range.end));
                     if a >= b {
@@ -181,7 +189,12 @@ fn field_run_corner_groups(
                     let l = picked.iter().map(|c| c.0).fold(f32::INFINITY, f32::min) as f64;
                     let r = picked.iter().map(|c| c.1).fold(f32::NEG_INFINITY, f32::max) as f64;
                     if l.is_finite() && r.is_finite() {
-                        lines.push((group.origin, run.rotation as f64, [l * anno, 0.0, r * anno, run.height as f64 * anno]));
+                        // Glyphs scale about the reference origin.
+                        let origin = [
+                            (group.origin[0] - ref_origin[0]) * anno + ref_origin[0],
+                            (group.origin[1] - ref_origin[1]) * anno + ref_origin[1],
+                        ];
+                        lines.push((origin, run.rotation as f64, [l * anno, 0.0, r * anno, run.height as f64 * anno]));
                     }
                 }
             }
@@ -1551,66 +1564,66 @@ pub fn tessellate(
                     WireModel::UNBOUNDED_AABB
                 };
 
+                // FIELDDISPLAY: a field shows on a gray box behind its glyphs,
+                // on screen only — for SDF glyphs and shaped stroke text alike.
+                let mut field_background: Option<WireModel> = None;
+                if crate::entities::field::display()
+                    && crate::entities::field::hosts_field(document, entity)
+                {
+                    let runs = field_run_corner_groups(document, entity, &stroke_groups, anno, ref_origin);
+                    let corner_groups = match entity {
+                        _ if runs.is_some() => runs.unwrap_or_default(),
+                        _ if text_aabb == WireModel::UNBOUNDED_AABB => Vec::new(),
+                        EntityType::MText(m) => {
+                            let rotation = stroke_groups
+                                .iter()
+                                .find_map(|group| group.run.as_ref().map(|run| run.rotation as f64))
+                                .unwrap_or(m.rotation);
+                            oriented_mtext_corner_groups(&sdf_verts, m, rotation, 0.0, anno)
+                        }
+                        EntityType::Text(t) => vec![oriented_text_corners(
+                            &sdf_verts,
+                            [t.insertion_point.x, t.insertion_point.y],
+                            t.rotation,
+                            0.0,
+                        )],
+                        EntityType::AttributeEntity(a) => vec![oriented_text_corners(
+                            &sdf_verts,
+                            [a.insertion_point.x, a.insertion_point.y],
+                            a.rotation,
+                            0.0,
+                        )],
+                        _ => Vec::new(),
+                    };
+                    let mut ft = Vec::with_capacity(6 * corner_groups.len());
+                    let mut ftl = Vec::with_capacity(6 * corner_groups.len());
+                    for corners in &corner_groups {
+                        for &k in &[0usize, 1, 2, 0, 2, 3] {
+                            let (h, lo) = split_ds_xyz(corners[k][0], corners[k][1], elev_v);
+                            ft.push(h);
+                            ftl.push(lo);
+                        }
+                    }
+                    if !ft.is_empty() {
+                        field_background = Some(WireModel {
+                            display_visible: true,
+                            plot_visible: false,
+                            name: name.clone(),
+                            color: crate::entities::field::BACKGROUND,
+                            selected,
+                            line_weight_px,
+                            aabb: WireModel::UNBOUNDED_AABB,
+                            plinegen: true,
+                            fill_tris: ft,
+                            fill_tris_low: ftl,
+                            ..Default::default()
+                        });
+                    }
+                }
                 // Empty input (no glyphs) → emit a single empty wire so the
                 // entity still has a hit-test target via snap_pts. With SDF on
                 // this is the normal path (strokes suppressed) and the wire
                 // also carries the glyph quads built above.
-                // FIELDDISPLAY: a field shows on a gray box behind its glyphs,
-                // on screen only — for SDF glyphs and shaped stroke text alike.
-                let mut field_background: Option<WireModel> = None;
-                    if crate::entities::field::display()
-                        && crate::entities::field::hosts_field(document, entity)
-                    {
-                        let runs = field_run_corner_groups(document, entity, &stroke_groups, anno);
-                        let corner_groups = match entity {
-                            _ if runs.is_some() => runs.unwrap_or_default(),
-                            _ if text_aabb == WireModel::UNBOUNDED_AABB => Vec::new(),
-                            EntityType::MText(m) => {
-                                let rotation = stroke_groups
-                                    .iter()
-                                    .find_map(|group| group.run.as_ref().map(|run| run.rotation as f64))
-                                    .unwrap_or(m.rotation);
-                                oriented_mtext_corner_groups(&sdf_verts, m, rotation, 0.0, anno)
-                            }
-                            EntityType::Text(t) => vec![oriented_text_corners(
-                                &sdf_verts,
-                                [t.insertion_point.x, t.insertion_point.y],
-                                t.rotation,
-                                0.0,
-                            )],
-                            EntityType::AttributeEntity(a) => vec![oriented_text_corners(
-                                &sdf_verts,
-                                [a.insertion_point.x, a.insertion_point.y],
-                                a.rotation,
-                                0.0,
-                            )],
-                            _ => Vec::new(),
-                        };
-                        let mut ft = Vec::with_capacity(6 * corner_groups.len());
-                        let mut ftl = Vec::with_capacity(6 * corner_groups.len());
-                        for corners in &corner_groups {
-                            for &k in &[0usize, 1, 2, 0, 2, 3] {
-                                let (h, lo) = split_ds_xyz(corners[k][0], corners[k][1], elev_v);
-                                ft.push(h);
-                                ftl.push(lo);
-                            }
-                        }
-                        if !ft.is_empty() {
-                            field_background = Some(WireModel {
-                                display_visible: true,
-                                plot_visible: false,
-                                name: name.clone(),
-                                color: crate::entities::field::BACKGROUND,
-                                selected,
-                                line_weight_px,
-                                aabb: WireModel::UNBOUNDED_AABB,
-                                plinegen: true,
-                                fill_tris: ft,
-                                fill_tris_low: ftl,
-                                ..Default::default()
-                            });
-                        }
-                    }
                 if bins.is_empty() {
                     let mut wires: Vec<WireModel> = Vec::new();
                     wires.extend(field_background.take());

src/scene/mod.rs

@@ -3415,11 +3415,11 @@ impl Scene {
     /// [`crate::entities::field::update_fields`]) and redraw the hosts whose
     /// text changed. Returns the number of fields the hosts hold.
     pub fn update_fields(&mut self, event: i32, hosts: Option<&[Handle]>) -> usize {
-        let (changed, found) = crate::entities::field::update_fields(&mut self.document, event, hosts);
+        let changed = crate::entities::field::update_fields(&mut self.document, event, hosts).0;
         if !changed.is_empty() {
             self.bump_text_hosts(&changed);
         }
-        found
+        changed.len()
     }
 
     /// Redraw text hosts whose stored text changed (re-evaluated fields). An
@@ -3427,43 +3427,82 @@ impl Scene {
     /// by every reference to that block, so those redraw instead of the host;
     /// nothing else re-tessellates.
     pub fn bump_text_hosts(&mut self, hosts: &[Handle]) {
+        if hosts.is_empty() {
+            return;
+        }
         if self.dependency_index_cache.borrow().is_none() {
             *self.dependency_index_cache.borrow_mut() = Some(self.rebuild_dependency_index());
         }
+        // One pass over the drawing for all hosts: which block definition
+        // holds an entity, which INSERT draws an attribute, which tables
+        // draw from a block.
+        let layout_blocks: HashSet<Handle> = self
+            .document
+            .objects
+            .values()
+            .filter_map(|object| match object {
+                ObjectType::Layout(layout) if !layout.block_record.is_null() => Some(layout.block_record),
+                _ => None,
+            })
+            .collect();
+        let mut definitions: HashMap<Handle, (Handle, String)> = HashMap::default();
+        let mut members: HashMap<Handle, Handle> = HashMap::default();
+        for record in self.document.block_records.iter() {
+            let name = record.name.to_ascii_uppercase();
+            if layout_blocks.contains(&record.handle)
+                || name.starts_with("*MODEL_SPACE")
+                || name.starts_with("*PAPER_SPACE")
+            {
+                continue;
+            }
+            definitions.insert(record.handle, (record.handle, normalize_name(&record.name)));
+            for member in &record.entity_handles {
+                members.insert(*member, record.handle);
+            }
+        }
+        let mut attribute_owner: HashMap<Handle, Handle> = HashMap::default();
+        let mut tables_of: HashMap<Handle, Vec<Handle>> = HashMap::default();
+        for entity in self.document.entities() {
+            match entity {
+                EntityType::Insert(insert) => {
+                    for attribute in &insert.attributes {
+                        attribute_owner.insert(attribute.common.handle, insert.common.handle);
+                    }
+                }
+                EntityType::Table(table) => {
+                    if let Some(record) = table.block_record_handle {
+                        tables_of.entry(record).or_default().push(table.common.handle);
+                    }
+                }
+                _ => {}
+            }
+        }
         let mut targets = DependencyTargets::default();
         for &host in hosts {
-            let drawn = match self.document.get_entity(host) {
-                Some(entity) => entity,
-                None => match self.document.entities().find(|e| {
-                    matches!(e, EntityType::Insert(i) if i.attributes.iter().any(|a| a.common.handle == host))
-                }) {
-                    Some(insert) => insert,
+            // An attribute is drawn by its INSERT.
+            let handle = match self.document.get_entity(host) {
+                Some(_) => host,
+                None => match attribute_owner.get(&host) {
+                    Some(insert) => *insert,
                     None => continue,
                 },
             };
-            let handle = drawn.common().handle;
-            let owner = drawn.common().owner_handle;
-            if !self.changes_touch_block_definition(&[(handle, ChangeKind::Modified)]) {
+            let owner = self.document.get_entity(handle).map(|e| e.common().owner_handle);
+            let record = owner
+                .and_then(|owner| definitions.get(&owner))
+                .or_else(|| members.get(&handle).and_then(|record| definitions.get(record)));
+            let Some((record_handle, name)) = record else {
                 targets.render_handles.insert(handle);
                 continue;
-            }
-            let Some(record) = self.document.block_records.iter().find(|r| {
-                r.handle == owner || r.entity_handles.contains(&handle)
-            }) else {
-                continue;
             };
-            let name = normalize_name(&record.name);
+            // A host inside a block definition is drawn by every reference
+            // to that block, and a table by its own block of cell texts.
             if let Some(index) = self.dependency_index_cache.borrow().as_ref() {
-                if let Some(users) = index.block_roots.get(&name) {
+                if let Some(users) = index.block_roots.get(name) {
                     targets.render_handles.extend(users.iter().copied());
                 }
             }
-            // A table draws its cells from its own block of cell texts.
-            let record_handle = record.handle;
-            targets.render_handles.extend(self.document.entities().filter_map(|e| match e {
-                EntityType::Table(t) if t.block_record_handle == Some(record_handle) => Some(t.common.handle),
-                _ => None,
-            }));
+            targets.render_handles.extend(tables_of.get(record_handle).into_iter().flatten().copied());
             targets.touches_block_definition = true;
         }
         self.invalidate_dependency_targets(targets);