Skip to content

Commit c516fef

Browse files
relrelbrelrelb
committed
swf: Simplify read of PlaceObject tags
`tag_length` isn't really necessary since each tag is read using a dedicated `swf::Reader`, which keeps track of the tag boundary internally. As a result, `tag_len` can be avoided passed around many times in `movie_clip.rs`.
1 parent 8f214ff commit c516fef

File tree

2 files changed

+27
-81
lines changed

2 files changed

+27
-81
lines changed

‎core/src/display_object/movie_clip.rs‎

Lines changed: 20 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -1339,16 +1339,16 @@ impl<'gc> MovieClip<'gc> {
13391339
match tag_code {
13401340
TagCode::DoAction => self.do_action(context, reader, tag_len),
13411341
TagCode::PlaceObject if run_display_actions && !context.is_action_script_3() => {
1342-
self.place_object(context, reader, tag_len, 1)
1342+
self.place_object(context, reader, 1)
13431343
}
13441344
TagCode::PlaceObject2 if run_display_actions && !context.is_action_script_3() => {
1345-
self.place_object(context, reader, tag_len, 2)
1345+
self.place_object(context, reader, 2)
13461346
}
13471347
TagCode::PlaceObject3 if run_display_actions && !context.is_action_script_3() => {
1348-
self.place_object(context, reader, tag_len, 3)
1348+
self.place_object(context, reader, 3)
13491349
}
13501350
TagCode::PlaceObject4 if run_display_actions && !context.is_action_script_3() => {
1351-
self.place_object(context, reader, tag_len, 4)
1351+
self.place_object(context, reader, 4)
13521352
}
13531353
TagCode::RemoveObject if run_display_actions && !context.is_action_script_3() => {
13541354
self.remove_object(context, reader, 1)
@@ -1357,22 +1357,22 @@ impl<'gc> MovieClip<'gc> {
13571357
self.remove_object(context, reader, 2)
13581358
}
13591359
TagCode::PlaceObject if run_display_actions && context.is_action_script_3() => {
1360-
self.queue_place_object(context, reader, tag_len, 1)
1360+
self.queue_place_object(context, reader, 1)
13611361
}
13621362
TagCode::PlaceObject2 if run_display_actions && context.is_action_script_3() => {
1363-
self.queue_place_object(context, reader, tag_len, 2)
1363+
self.queue_place_object(context, reader, 2)
13641364
}
13651365
TagCode::PlaceObject3 if run_display_actions && context.is_action_script_3() => {
1366-
self.queue_place_object(context, reader, tag_len, 3)
1366+
self.queue_place_object(context, reader, 3)
13671367
}
13681368
TagCode::PlaceObject4 if run_display_actions && context.is_action_script_3() => {
1369-
self.queue_place_object(context, reader, tag_len, 4)
1369+
self.queue_place_object(context, reader, 4)
13701370
}
13711371
TagCode::RemoveObject if run_display_actions && context.is_action_script_3() => {
1372-
self.queue_remove_object(context, reader, tag_len, 1)
1372+
self.queue_remove_object(context, reader, 1)
13731373
}
13741374
TagCode::RemoveObject2 if run_display_actions && context.is_action_script_3() => {
1375-
self.queue_remove_object(context, reader, tag_len, 2)
1375+
self.queue_remove_object(context, reader, 2)
13761376
}
13771377
TagCode::SetBackgroundColor => self.set_background_color(context, reader),
13781378
TagCode::StartSound if run_sounds => self.start_sound_1(context, reader),
@@ -1675,60 +1675,28 @@ impl<'gc> MovieClip<'gc> {
16751675
self.0.write(context.gc_context).current_frame += 1;
16761676
frame_pos = reader.get_ref().as_ptr() as u64 - tag_stream_start;
16771677

1678-
use swf::TagCode;
1679-
let tag_callback = |reader: &mut _, tag_code, tag_len| {
1678+
let tag_callback = |reader: &mut _, tag_code, _tag_len| {
1679+
use swf::TagCode;
16801680
match tag_code {
16811681
TagCode::PlaceObject => {
16821682
index += 1;
16831683
let mut mc = self.0.write(context.gc_context);
1684-
1685-
mc.goto_place_object(
1686-
reader,
1687-
tag_len,
1688-
1,
1689-
&mut goto_commands,
1690-
is_rewind,
1691-
index,
1692-
)
1684+
mc.goto_place_object(reader, 1, &mut goto_commands, is_rewind, index)
16931685
}
16941686
TagCode::PlaceObject2 => {
16951687
index += 1;
16961688
let mut mc = self.0.write(context.gc_context);
1697-
1698-
mc.goto_place_object(
1699-
reader,
1700-
tag_len,
1701-
2,
1702-
&mut goto_commands,
1703-
is_rewind,
1704-
index,
1705-
)
1689+
mc.goto_place_object(reader, 2, &mut goto_commands, is_rewind, index)
17061690
}
17071691
TagCode::PlaceObject3 => {
17081692
index += 1;
17091693
let mut mc = self.0.write(context.gc_context);
1710-
1711-
mc.goto_place_object(
1712-
reader,
1713-
tag_len,
1714-
3,
1715-
&mut goto_commands,
1716-
is_rewind,
1717-
index,
1718-
)
1694+
mc.goto_place_object(reader, 3, &mut goto_commands, is_rewind, index)
17191695
}
17201696
TagCode::PlaceObject4 => {
17211697
index += 1;
17221698
let mut mc = self.0.write(context.gc_context);
1723-
1724-
mc.goto_place_object(
1725-
reader,
1726-
tag_len,
1727-
4,
1728-
&mut goto_commands,
1729-
is_rewind,
1730-
index,
1731-
)
1699+
mc.goto_place_object(reader, 4, &mut goto_commands, is_rewind, index)
17321700
}
17331701
TagCode::RemoveObject => self.goto_remove_object(
17341702
reader,
@@ -1808,7 +1776,6 @@ impl<'gc> MovieClip<'gc> {
18081776
let new_tag = QueuedTag {
18091777
tag_type: QueuedTagAction::Place(params.version),
18101778
tag_start: params.tag_start,
1811-
tag_len: params.tag_len,
18121779
};
18131780
let bucket = write
18141781
.queued_tags
@@ -2343,7 +2310,7 @@ impl<'gc> TDisplayObject<'gc> for MovieClip<'gc> {
23432310
_ => unreachable!(),
23442311
};
23452312

2346-
if let Err(e) = self.place_object(context, &mut reader, tag.tag_len, version) {
2313+
if let Err(e) = self.place_object(context, &mut reader, version) {
23472314
log::error!("Error running queued tag: {:?}, got {}", tag.tag_type, e);
23482315
}
23492316
}
@@ -2914,7 +2881,6 @@ impl<'gc> MovieClipData<'gc> {
29142881
fn goto_place_object<'a>(
29152882
&mut self,
29162883
reader: &mut SwfStream<'a>,
2917-
tag_len: usize,
29182884
version: u8,
29192885
goto_commands: &mut Vec<GotoPlaceObject<'a>>,
29202886
is_rewind: bool,
@@ -2923,7 +2889,7 @@ impl<'gc> MovieClipData<'gc> {
29232889
let tag_start =
29242890
reader.get_ref().as_ptr() as u64 - self.static_data.swf.as_ref().as_ptr() as u64;
29252891
let place_object = if version == 1 {
2926-
reader.read_place_object(tag_len)
2892+
reader.read_place_object()
29272893
} else {
29282894
reader.read_place_object_2_or_3(version)
29292895
}?;
@@ -2936,7 +2902,6 @@ impl<'gc> MovieClipData<'gc> {
29362902
is_rewind,
29372903
index,
29382904
tag_start,
2939-
tag_len,
29402905
version,
29412906
);
29422907
if let Some(i) = goto_commands.iter().position(|o| o.depth() == depth) {
@@ -3663,22 +3628,20 @@ impl<'gc, 'a> MovieClip<'gc> {
36633628
self,
36643629
context: &mut UpdateContext<'_, 'gc, '_>,
36653630
reader: &mut SwfStream<'a>,
3666-
tag_len: usize,
36673631
version: u8,
36683632
) -> Result<(), Error> {
36693633
let mut write = self.0.write(context.gc_context);
36703634
let tag_start =
36713635
reader.get_ref().as_ptr() as u64 - write.static_data.swf.as_ref().as_ptr() as u64;
36723636
let place_object = if version == 1 {
3673-
reader.read_place_object(tag_len)
3637+
reader.read_place_object()
36743638
} else {
36753639
reader.read_place_object_2_or_3(version)
36763640
}?;
36773641

36783642
let new_tag = QueuedTag {
36793643
tag_type: QueuedTagAction::Place(version),
36803644
tag_start,
3681-
tag_len,
36823645
};
36833646
let bucket = write
36843647
.queued_tags
@@ -3694,11 +3657,10 @@ impl<'gc, 'a> MovieClip<'gc> {
36943657
self,
36953658
context: &mut UpdateContext<'_, 'gc, '_>,
36963659
reader: &mut SwfStream<'a>,
3697-
tag_len: usize,
36983660
version: u8,
36993661
) -> Result<(), Error> {
37003662
let place_object = if version == 1 {
3701-
reader.read_place_object(tag_len)
3663+
reader.read_place_object()
37023664
} else {
37033665
reader.read_place_object_2_or_3(version)
37043666
}?;
@@ -3753,7 +3715,6 @@ impl<'gc, 'a> MovieClip<'gc> {
37533715
self,
37543716
context: &mut UpdateContext<'_, 'gc, '_>,
37553717
reader: &mut SwfStream<'a>,
3756-
tag_len: usize,
37573718
version: u8,
37583719
) -> Result<(), Error> {
37593720
let mut write = self.0.write(context.gc_context);
@@ -3768,7 +3729,6 @@ impl<'gc, 'a> MovieClip<'gc> {
37683729
let new_tag = QueuedTag {
37693730
tag_type: QueuedTagAction::Remove(version),
37703731
tag_start,
3771-
tag_len,
37723732
};
37733733
let bucket = write
37743734
.queued_tags
@@ -3998,9 +3958,6 @@ struct GotoPlaceObject<'a> {
39983958
/// not possible and we want to add children after the goto completes.
39993959
tag_start: u64,
40003960

4001-
/// The length of the PlaceObject tag at `tag_start`.
4002-
tag_len: usize,
4003-
40043961
/// The version of the PlaceObject tag at `tag_start`.
40053962
version: u8,
40063963
}
@@ -4012,7 +3969,6 @@ impl<'a> GotoPlaceObject<'a> {
40123969
is_rewind: bool,
40133970
index: usize,
40143971
tag_start: u64,
4015-
tag_len: usize,
40163972
version: u8,
40173973
) -> Self {
40183974
if is_rewind {
@@ -4050,7 +4006,6 @@ impl<'a> GotoPlaceObject<'a> {
40504006
place_object,
40514007
index,
40524008
tag_start,
4053-
tag_len,
40544009
version,
40554010
}
40564011
}
@@ -4177,7 +4132,6 @@ impl QueuedTagList {
41774132
pub struct QueuedTag {
41784133
pub tag_type: QueuedTagAction,
41794134
pub tag_start: u64,
4180-
pub tag_len: usize,
41814135
}
41824136

41834137
/// The type of queued tag.

‎swf/src/read.rs‎

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -591,9 +591,7 @@ impl<'a> Reader<'a> {
591591

592592
TagCode::DefineSprite => Tag::DefineSprite(tag_reader.read_define_sprite()?),
593593

594-
TagCode::PlaceObject => {
595-
Tag::PlaceObject(Box::new(tag_reader.read_place_object(length)?))
596-
}
594+
TagCode::PlaceObject => Tag::PlaceObject(Box::new(tag_reader.read_place_object()?)),
597595
TagCode::PlaceObject2 => {
598596
Tag::PlaceObject(Box::new(tag_reader.read_place_object_2_or_3(2)?))
599597
}
@@ -1834,20 +1832,14 @@ impl<'a> Reader<'a> {
18341832
Ok(exports)
18351833
}
18361834

1837-
pub fn read_place_object(&mut self, tag_length: usize) -> Result<PlaceObject<'a>> {
1838-
// TODO: What's a best way to know if the tag has a color transform?
1839-
// You only know if there is still data remaining after the matrix.
1840-
// This sucks.
1841-
let mut vector = [0; 128];
1842-
self.get_mut().read_exact(&mut vector[..tag_length])?;
1843-
let mut reader = Reader::new(&vector[..], self.version);
1835+
pub fn read_place_object(&mut self) -> Result<PlaceObject<'a>> {
18441836
Ok(PlaceObject {
18451837
version: 1,
1846-
action: PlaceObjectAction::Place(reader.read_u16()?),
1847-
depth: reader.read_u16()?,
1848-
matrix: Some(reader.read_matrix()?),
1849-
color_transform: if !reader.get_ref().is_empty() {
1850-
Some(reader.read_color_transform(false)?)
1838+
action: PlaceObjectAction::Place(self.read_u16()?),
1839+
depth: self.read_u16()?,
1840+
matrix: Some(self.read_matrix()?),
1841+
color_transform: if !self.get_ref().is_empty() {
1842+
Some(self.read_color_transform(false)?)
18511843
} else {
18521844
None
18531845
},

0 commit comments

Comments
 (0)