forked from 1b-agent/opencadstudio-upstream
fix(constraints): split large XRecord Chunk data across entries for DWG
The DWG writer truncates an XRecord Chunk to u8::MAX bytes, so a sketch's constraint set or named-parameter table silently lost every entry on the next DWG open once serialization passed ~255 bytes (found live: a 23-constraint demo round-tripped fine through DXF but came back empty through DWG). Split the blob across multiple same-code 310 entries on save and reassemble them on load, entirely on the OCS side. Co-Authored-By: Claude Sonnet 5 <[email protected]>
This commit is contained in:
1 parent
9f7972ee0d
commit
d27eb625ca
3 files changed
+72
-19
No files matched your search
@@ -6,6 +6,23 @@ pushed.
|
||||
|
||||
## Unreleased (since `3d0a41b6`)
|
||||
|
||||
- **Fix:** a sketch's constraint set (or the named-parameter table) silently
|
||||
lost *all* of its entries on the next DWG open once its serialized size
|
||||
passed 255 bytes — roughly 5-6 constraints, easily reached by a real
|
||||
sketch. Root cause: the vendored DWG writer packs an `XRecord` binary
|
||||
`Chunk` entry's length as a single `u8` and truncates anything longer
|
||||
(`cadcodec`'s `encode_xrecord_entries`), so the truncated bytes then
|
||||
failed to `bincode::deserialize` on load and the loader silently dropped
|
||||
the whole scope rather than erroring. DXF was unaffected (its writer has
|
||||
no such cap), which is what made this findable — the exact same document
|
||||
round-tripped constraint-for-constraint through DXF and came back empty
|
||||
through DWG. Fixed entirely on the OCS side, no dependency patch needed:
|
||||
the blob is now split across as many same-code `310` entries as it takes
|
||||
(below 255 bytes each) on save, and every entry for that key is
|
||||
concatenated back together on load, instead of just the first one.
|
||||
([sketch_persist.rs](src/scene/sketch_persist.rs),
|
||||
[named_parameters_persist.rs](src/scene/named_parameters_persist.rs))
|
||||
|
||||
- **Fix:** none of the 17 sketch-constraint commands (`HCONSTRAINT`,
|
||||
`VCONSTRAINT`, `PCONSTRAINT`, `QCONSTRAINT`, `ECONSTRAINT`, `TCONSTRAINT`,
|
||||
`NCONSTRAINT`, `NRCONSTRAINT`, `LCONSTRAINT`, `FXCONSTRAINT`,
|
||||
|
||||
@@ -48,6 +48,12 @@ use super::Scene;
|
||||
|
||||
const XRECORD_KEY: &str = "OCS_NAMED_PARAMETERS";
|
||||
|
||||
/// See `sketch_persist::MAX_CHUNK_BYTES`'s doc comment: the vendored DWG
|
||||
/// writer truncates any single `XRecordValue::Chunk` entry to `u8::MAX`
|
||||
/// bytes, so a table past a handful of parameters must be split across
|
||||
/// multiple same-code `310` entries on write and concatenated back on read.
|
||||
const MAX_CHUNK_BYTES: usize = u8::MAX as usize;
|
||||
|
||||
/// Prefixed onto the serialized blob so a future schema change can be
|
||||
/// detected and gracefully skipped rather than silently misreading bytes —
|
||||
/// same call-out as `sketch_persist::FORMAT_VERSION`, same caveat that the
|
||||
@@ -91,11 +97,13 @@ impl Scene {
|
||||
self.document.ensure_xrecord(owner, XRECORD_KEY);
|
||||
if let Some(record) = self.document.xrecord_mut(owner, XRECORD_KEY) {
|
||||
// Overwrite, not append: a resave must replace the prior blob,
|
||||
// not accumulate one more Chunk entry every time.
|
||||
// not accumulate more Chunk entries every time.
|
||||
record.entries.clear();
|
||||
record
|
||||
.entries
|
||||
.push(acadrust::objects::XRecordEntry::new(310, acadrust::objects::XRecordValue::Chunk(bytes)));
|
||||
for chunk in bytes.chunks(MAX_CHUNK_BYTES) {
|
||||
record
|
||||
.entries
|
||||
.push(acadrust::objects::XRecordEntry::new(310, acadrust::objects::XRecordValue::Chunk(chunk.to_vec())));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -114,12 +122,17 @@ impl Scene {
|
||||
return;
|
||||
}
|
||||
let Some(record) = self.document.xrecord(owner, XRECORD_KEY) else { return };
|
||||
let Some(bytes) = record.entries.iter().find_map(|entry| match &entry.value {
|
||||
acadrust::objects::XRecordValue::Chunk(bytes) => Some(bytes.clone()),
|
||||
_ => None,
|
||||
}) else {
|
||||
// Concatenate every Chunk entry in order, not just the first — see
|
||||
// `MAX_CHUNK_BYTES`'s doc comment.
|
||||
let mut bytes = Vec::new();
|
||||
for entry in &record.entries {
|
||||
if let acadrust::objects::XRecordValue::Chunk(chunk) = &entry.value {
|
||||
bytes.extend_from_slice(chunk);
|
||||
}
|
||||
}
|
||||
if bytes.is_empty() {
|
||||
return;
|
||||
};
|
||||
}
|
||||
if let Some(table) = decode(&bytes) {
|
||||
self.named_parameters = table;
|
||||
}
|
||||
|
||||
+33
-10
@@ -21,6 +21,21 @@ use acadrust::{CadDocument, Handle};
|
||||
|
||||
const XRECORD_KEY: &str = "OCS_SKETCH_CONSTRAINTS";
|
||||
|
||||
/// The vendored DWG writer's `XRecordValue::Chunk` encoder packs a single
|
||||
/// `u8` length prefix per entry and silently truncates anything longer
|
||||
/// (`cadcodec`'s `io/dwg/dwg_stream_writers/object_writer/objects.rs`,
|
||||
/// `encode_xrecord_entries`) — unlike the DXF writer, which has no such
|
||||
/// limit. A constraint set past a handful of entries serializes past 255
|
||||
/// bytes easily, so a single `XRecordEntry` silently loses the tail and
|
||||
/// `bincode::deserialize` then fails on the truncated bytes, dropping the
|
||||
/// *entire* scope's constraints on the next DWG load — reproduced and
|
||||
/// root-caused live (not from a change log): a 5-entry set round-tripped
|
||||
/// clean, a 6-entry set of the same shapes lost everything. Splitting the
|
||||
/// blob across as many same-code `310` entries as needed (below) works
|
||||
/// around it without touching the vendored crate — `CadDocument::xrecord`'s
|
||||
/// `entries: Vec<XRecordEntry>` already supports repeating a code.
|
||||
const MAX_CHUNK_BYTES: usize = u8::MAX as usize;
|
||||
|
||||
/// Removes a named XRecord from `owner`'s extension dictionary, if present.
|
||||
/// The dictionary itself is left in place even if now empty — other code may
|
||||
/// already reference it via `xdictionary_handle`, and an empty extension
|
||||
@@ -107,12 +122,14 @@ impl Scene {
|
||||
self.document.ensure_xrecord(owner, XRECORD_KEY);
|
||||
if let Some(record) = self.document.xrecord_mut(owner, XRECORD_KEY) {
|
||||
// Overwrite, not append: a resave must replace the prior
|
||||
// blob, not accumulate one more Chunk entry every time.
|
||||
// blob, not accumulate more Chunk entries every time.
|
||||
record.entries.clear();
|
||||
record.entries.push(acadrust::objects::XRecordEntry::new(
|
||||
310,
|
||||
acadrust::objects::XRecordValue::Chunk(bytes),
|
||||
));
|
||||
for chunk in bytes.chunks(MAX_CHUNK_BYTES) {
|
||||
record.entries.push(acadrust::objects::XRecordEntry::new(
|
||||
310,
|
||||
acadrust::objects::XRecordValue::Chunk(chunk.to_vec()),
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -132,12 +149,18 @@ impl Scene {
|
||||
self.document.block_records.iter().map(|record| record.handle).collect();
|
||||
for owner in owners {
|
||||
let Some(record) = self.document.xrecord(owner, XRECORD_KEY) else { continue };
|
||||
let Some(bytes) = record.entries.iter().find_map(|entry| match &entry.value {
|
||||
acadrust::objects::XRecordValue::Chunk(bytes) => Some(bytes.clone()),
|
||||
_ => None,
|
||||
}) else {
|
||||
// Concatenate every Chunk entry in order, not just the first —
|
||||
// a set materialized past `MAX_CHUNK_BYTES` spans more than one
|
||||
// same-code `310` entry (see that constant's doc comment).
|
||||
let mut bytes = Vec::new();
|
||||
for entry in &record.entries {
|
||||
if let acadrust::objects::XRecordValue::Chunk(chunk) = &entry.value {
|
||||
bytes.extend_from_slice(chunk);
|
||||
}
|
||||
}
|
||||
if bytes.is_empty() {
|
||||
continue;
|
||||
};
|
||||
}
|
||||
if let Some(set) = decode(&bytes) {
|
||||
self.sketch_constraints.push(set);
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user