diff --git a/src/object_pool/object.rs b/src/object_pool/object.rs index ab831d7..c4817a9 100644 --- a/src/object_pool/object.rs +++ b/src/object_pool/object.rs @@ -291,16 +291,16 @@ impl Object { Object::Key(o) => extend_object_refs(&mut refs, &o.object_refs), Object::Button(o) => extend_object_refs(&mut refs, &o.object_refs), Object::InputBoolean(o) => { - refs.push(o.foreground_colour); + push_nullable_id(&mut refs, &o.foreground_colour); push_nullable_id(&mut refs, &o.variable_reference); } Object::InputString(o) => { - refs.push(o.font_attributes); + push_nullable_id(&mut refs, &o.font_attributes); push_nullable_id(&mut refs, &o.input_attributes); push_nullable_id(&mut refs, &o.variable_reference); } Object::InputNumber(o) => { - refs.push(o.font_attributes); + push_nullable_id(&mut refs, &o.font_attributes); push_nullable_id(&mut refs, &o.variable_reference); } Object::InputList(o) => { @@ -308,28 +308,28 @@ impl Object { extend_nullable_ids(&mut refs, &o.list_items); } Object::OutputString(o) => { - refs.push(o.font_attributes); + push_nullable_id(&mut refs, &o.font_attributes); push_nullable_id(&mut refs, &o.variable_reference); } Object::OutputNumber(o) => { - refs.push(o.font_attributes); + push_nullable_id(&mut refs, &o.font_attributes); push_nullable_id(&mut refs, &o.variable_reference); } Object::OutputList(o) => { push_nullable_id(&mut refs, &o.variable_reference); extend_nullable_ids(&mut refs, &o.list_items); } - Object::OutputLine(o) => refs.push(o.line_attributes), + Object::OutputLine(o) => push_nullable_id(&mut refs, &o.line_attributes), Object::OutputRectangle(o) => { - refs.push(o.line_attributes); + push_nullable_id(&mut refs, &o.line_attributes); push_nullable_id(&mut refs, &o.fill_attributes); } Object::OutputEllipse(o) => { - refs.push(o.line_attributes); + push_nullable_id(&mut refs, &o.line_attributes); push_nullable_id(&mut refs, &o.fill_attributes); } Object::OutputPolygon(o) => { - refs.push(o.line_attributes); + push_nullable_id(&mut refs, &o.line_attributes); push_nullable_id(&mut refs, &o.fill_attributes); } Object::OutputMeter(o) => push_nullable_id(&mut refs, &o.variable_reference), @@ -527,7 +527,7 @@ pub struct InputBoolean { pub id: ObjectId, pub background_colour: u8, pub width: u16, - pub foreground_colour: ObjectId, + pub foreground_colour: NullableObjectId, pub variable_reference: NullableObjectId, pub value: bool, pub enabled: bool, @@ -550,7 +550,7 @@ pub struct InputString { pub width: u16, pub height: u16, pub background_colour: u8, - pub font_attributes: ObjectId, + pub font_attributes: NullableObjectId, pub input_attributes: NullableObjectId, pub options: InputStringOptions, pub variable_reference: NullableObjectId, @@ -576,7 +576,7 @@ pub struct InputNumber { pub width: u16, pub height: u16, pub background_colour: u8, - pub font_attributes: ObjectId, + pub font_attributes: NullableObjectId, pub options: NumberOptions, pub variable_reference: NullableObjectId, pub value: u32, @@ -629,7 +629,7 @@ pub struct OutputString { pub width: u16, pub height: u16, pub background_colour: u8, - pub font_attributes: ObjectId, + pub font_attributes: NullableObjectId, pub options: OutputStringOptions, pub variable_reference: NullableObjectId, pub justification: Alignment, @@ -653,7 +653,7 @@ pub struct OutputNumber { pub width: u16, pub height: u16, pub background_colour: u8, - pub font_attributes: ObjectId, + pub font_attributes: NullableObjectId, pub options: NumberOptions, pub variable_reference: NullableObjectId, pub value: u32, @@ -699,7 +699,7 @@ impl SizedObject for OutputList { #[derive(Debug, PartialEq, Clone)] pub struct OutputLine { pub id: ObjectId, - pub line_attributes: ObjectId, + pub line_attributes: NullableObjectId, pub width: u16, pub height: u16, pub line_direction: LineDirection, @@ -719,7 +719,7 @@ impl SizedObject for OutputLine { #[derive(Debug, PartialEq, Clone)] pub struct OutputRectangle { pub id: ObjectId, - pub line_attributes: ObjectId, + pub line_attributes: NullableObjectId, pub width: u16, pub height: u16, pub line_suppression: u8, @@ -740,7 +740,7 @@ impl SizedObject for OutputRectangle { #[derive(Debug, PartialEq, Clone)] pub struct OutputEllipse { pub id: ObjectId, - pub line_attributes: ObjectId, + pub line_attributes: NullableObjectId, pub width: u16, pub height: u16, pub ellipse_type: u8, @@ -765,7 +765,7 @@ pub struct OutputPolygon { pub id: ObjectId, pub width: u16, pub height: u16, - pub line_attributes: ObjectId, + pub line_attributes: NullableObjectId, pub fill_attributes: NullableObjectId, pub polygon_type: u8, pub points: Vec>, diff --git a/src/object_pool/object_pool.rs b/src/object_pool/object_pool.rs index b321d84..83441bb 100644 --- a/src/object_pool/object_pool.rs +++ b/src/object_pool/object_pool.rs @@ -7,6 +7,7 @@ use crate::object_pool::object::{ use crate::object_pool::object_id::ObjectId; use crate::object_pool::vt_version::VtVersion; use crate::object_pool::ObjectType; +use crate::object_pool::ParseError; #[derive(Debug, Clone, PartialEq)] pub struct ObjectPool { @@ -76,18 +77,50 @@ impl ObjectPool { op } + /// Like [`from_iop`](Self::from_iop), but returns the first parse error + /// instead of silently keeping only the objects read before it. + /// + /// `from_iop` cannot tell a pool that ended from a pool it could not + /// finish reading: both just stop. For a pool whose Working Set is not the + /// first object, an unreadable object anywhere before it makes the + /// lenient parse come back with no Working Set at all and no indication + /// why. Use this when that difference matters. + pub fn try_from_iop(data: I) -> Result + where + I: IntoIterator, + { + let mut op = Self::new(); + op.try_extend_with_iop(data)?; + Ok(op) + } + pub fn extend_with_iop(&mut self, data: I) where I: IntoIterator, { - let mut data = data.into_iter(); + // Lenient by design: keep whatever was readable, as this always has. + let _ = self.try_extend_with_iop(data); + } + + /// Strict counterpart of [`extend_with_iop`](Self::extend_with_iop): stops + /// at, and returns, the first object that cannot be read. Objects read + /// before the failure have already been added. + pub fn try_extend_with_iop(&mut self, data: I) -> Result<(), ParseError> + where + I: IntoIterator, + { + let mut data = data.into_iter().peekable(); - while let Ok(o) = Object::read(&mut data) { + // Running out of data *between* objects is the normal end of a pool; + // running out inside one is a truncated object and is reported. + while data.peek().is_some() { + let o = Object::read(&mut data)?; // By the standard, if there already is an object with the same ID, the new object // replaces the old one self.objects.retain(|x| x.id() != o.id()); self.objects.push(o); } + Ok(()) } pub fn as_iop(&self) -> Vec { diff --git a/src/object_pool/reader.rs b/src/object_pool/reader.rs index 84feec5..08194f1 100644 --- a/src/object_pool/reader.rs +++ b/src/object_pool/reader.rs @@ -497,7 +497,7 @@ impl Object { id, background_colour: Self::read_u8(data)?.into(), width: Self::read_u16(data)?, - foreground_colour: Self::read_u16(data)?.try_into()?, + foreground_colour: Self::read_u16(data)?.into(), variable_reference: Self::read_u16(data)?.into(), value: Self::read_bool(data)?, enabled: Self::read_bool(data)?, @@ -519,7 +519,7 @@ impl Object { width: Self::read_u16(data)?, height: Self::read_u16(data)?, background_colour: Self::read_u8(data)?.into(), - font_attributes: Self::read_u16(data)?.try_into()?, + font_attributes: Self::read_u16(data)?.into(), input_attributes: Self::read_u16(data)?.into(), options: Self::read_u8(data)?.into(), variable_reference: Self::read_u16(data)?.into(), @@ -544,7 +544,7 @@ impl Object { width: Self::read_u16(data)?, height: Self::read_u16(data)?, background_colour: Self::read_u8(data)?.into(), - font_attributes: Self::read_u16(data)?.try_into()?, + font_attributes: Self::read_u16(data)?.into(), options: Self::read_u8(data)?.into(), variable_reference: Self::read_u16(data)?.into(), value: Self::read_u32(data)?, @@ -599,7 +599,7 @@ impl Object { width: Self::read_u16(data)?, height: Self::read_u16(data)?, background_colour: Self::read_u8(data)?.into(), - font_attributes: Self::read_u16(data)?.try_into()?, + font_attributes: Self::read_u16(data)?.into(), options: Self::read_u8(data)?.into(), variable_reference: Self::read_u16(data)?.into(), justification: Self::read_u8(data)?.into(), @@ -622,7 +622,7 @@ impl Object { width: Self::read_u16(data)?, height: Self::read_u16(data)?, background_colour: Self::read_u8(data)?.into(), - font_attributes: Self::read_u16(data)?.try_into()?, + font_attributes: Self::read_u16(data)?.into(), options: Self::read_u8(data)?.into(), variable_reference: Self::read_u16(data)?.into(), value: Self::read_u32(data)?, @@ -646,7 +646,7 @@ impl Object { ) -> Result { let mut o = OutputLine { id, - line_attributes: Self::read_u16(data)?.try_into()?, + line_attributes: Self::read_u16(data)?.into(), width: Self::read_u16(data)?, height: Self::read_u16(data)?, line_direction: Self::read_u8(data)?.into(), @@ -665,7 +665,7 @@ impl Object { ) -> Result { let mut o = OutputRectangle { id, - line_attributes: Self::read_u16(data)?.try_into()?, + line_attributes: Self::read_u16(data)?.into(), width: Self::read_u16(data)?, height: Self::read_u16(data)?, line_suppression: Self::read_u8(data)?, @@ -685,7 +685,7 @@ impl Object { ) -> Result { let mut o = OutputEllipse { id, - line_attributes: Self::read_u16(data)?.try_into()?, + line_attributes: Self::read_u16(data)?.into(), width: Self::read_u16(data)?, height: Self::read_u16(data)?, ellipse_type: Self::read_u8(data)?, @@ -709,7 +709,7 @@ impl Object { id, width: Self::read_u16(data)?, height: Self::read_u16(data)?, - line_attributes: Self::read_u16(data)?.try_into()?, + line_attributes: Self::read_u16(data)?.into(), fill_attributes: Self::read_u16(data)?.into(), polygon_type: Self::read_u8(data)?, points: Vec::with_capacity(Self::read_u8(data)?.into()), @@ -1325,7 +1325,7 @@ mod tests { use crate::object_pool::object::{Object, WorkingSet}; use crate::object_pool::object_attributes::{ObjectRef, Point}; use crate::object_pool::object_id::ObjectId; - use crate::object_pool::{Colour, ObjectPool, ObjectType}; + use crate::object_pool::{Colour, NullableObjectId, ObjectPool, ObjectType, ParseError}; use std::vec::IntoIter; fn read_id_type(data: &mut dyn Iterator) -> ObjectId { @@ -1382,7 +1382,7 @@ mod tests { let _working_set_exp = WorkingSet { id, - background_colour: Colour::new_by_id(0xF0), + background_colour: Colour::new_by_id(0xF0).into(), selectable: true, active_mask: ObjectId::default(), object_refs: vec![ @@ -1401,4 +1401,118 @@ mod tests { assert_eq!(*_working_set_act, _working_set_exp); } + + // ISO 11783-6 allows the Line Attributes reference of the output shape + // objects, the Font Attributes reference of the string/number objects and + // the Foreground Colour reference of Input Boolean to be the NULL object + // ID (0xFFFF). Real pools use it: a John Deere StarFire receiver's pool + // nulls line attributes on 213 shapes and foreground colour on every one + // of its Input Booleans. Rejecting it made `Object::read` fail partway + // through such pools. + + #[test] + fn read_output_line_with_null_line_attributes_test() { + let mut data: IntoIter = vec![ + 0x01, 0x00, // Object ID + 0x0D, // Type: Output Line + 0xFF, 0xFF, // Line attributes: NULL + 0x70, 0x00, // Width + 0x01, 0x00, // Height + 0x00, // Line direction + 0x00, // Number of macro references + ] + .into_iter(); + + match Object::read(&mut data) { + Ok(Object::OutputLine(o)) => assert_eq!(o.line_attributes, NullableObjectId::NULL), + other => panic!("expected an Output Line, got {:?}", other), + } + } + + #[test] + fn read_output_string_with_null_font_attributes_test() { + let mut data: IntoIter = vec![ + 0x02, 0x00, // Object ID + 0x0B, // Type: Output String + 0x10, 0x00, // Width + 0x08, 0x00, // Height + 0x01, // Background colour + 0xFF, 0xFF, // Font attributes: NULL + 0x00, // Options + 0xFF, 0xFF, // Variable reference: NULL + 0x00, // Justification + 0x00, 0x00, // Length + 0x00, // Number of macro references + ] + .into_iter(); + + match Object::read(&mut data) { + Ok(Object::OutputString(o)) => assert_eq!(o.font_attributes, NullableObjectId::NULL), + other => panic!("expected an Output String, got {:?}", other), + } + } + + #[test] + fn read_input_boolean_with_null_foreground_colour_test() { + let mut data: IntoIter = vec![ + 0x03, 0x00, // Object ID + 0x07, // Type: Input Boolean + 0x01, // Background colour + 0x10, 0x00, // Width + 0xFF, 0xFF, // Foreground colour: NULL + 0xFF, 0xFF, // Variable reference: NULL + 0x00, // Value + 0x01, // Enabled + 0x00, // Number of macro references + ] + .into_iter(); + + match Object::read(&mut data) { + Ok(Object::InputBoolean(o)) => assert_eq!(o.foreground_colour, NullableObjectId::NULL), + other => panic!("expected an Input Boolean, got {:?}", other), + } + } + + // The pool-level consequence of the above: `from_iop` stops at the first + // object it cannot read and keeps what came before, so one NULL reference + // early in a pool used to drop every object after it -- including the + // Working Set when a vendor places it last, as John Deere does. + #[test] + fn from_iop_keeps_objects_after_null_line_attributes_test() { + let iop: Vec = vec![ + // Output Line, id 1, line attributes NULL + 0x01, 0x00, 0x0D, 0xFF, 0xFF, 0x70, 0x00, 0x01, 0x00, 0x00, 0x00, + // Working Set, id 0, active mask 2, one language "en" + 0x00, 0x00, 0x00, 0xF0, 0x01, 0x02, 0x00, 0x00, 0x00, 0x01, b'e', b'n', + // Data Mask, id 2, no soft key mask + 0x02, 0x00, 0x01, 0xF0, 0xFF, 0xFF, 0x00, 0x00, + ]; + + let pool = ObjectPool::from_iop(iop.clone()); + + assert_eq!(pool.objects().len(), 3); + assert!(pool.working_set_object().is_some()); + assert_eq!( + pool.as_iop(), + iop, + "NULL references must round-trip as 0xFFFF" + ); + + let strict = ObjectPool::try_from_iop(iop) + .unwrap_or_else(|why| panic!("strict parse should succeed: {:?}", why)); + assert_eq!(strict.objects().len(), 3); + } + + #[test] + fn try_from_iop_reports_truncated_object_test() { + // An Output Line cut off after its type byte. + let iop: Vec = vec![0x01, 0x00, 0x0D]; + + assert!(matches!( + ObjectPool::try_from_iop(iop.clone()), + Err(ParseError::DataEmpty) + )); + // The lenient reader keeps its existing behaviour: no objects, no error. + assert_eq!(ObjectPool::from_iop(iop).objects().len(), 0); + } }