Skip to content

Commit 7e2dfd5

Browse files
authored
Encode path_data and draw_data directly as u32 (#817)
We already assume these are word-aligned, and so this would catch a failure of that assumption very early, if it came up. A potential alternative is to store `path_data` as f32 instead, as all values stored are f32. I've not done that for pretty weak reasons, and it should be trivial to change. A future change would be to encode the full scene encoding as `u32`. That would be useful for #810, as the current code has an alignment hazard.
1 parent 9988360 commit 7e2dfd5

File tree

3 files changed

+51
-37
lines changed

3 files changed

+51
-37
lines changed

vello_encoding/src/encoding.rs

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,13 @@ pub struct Encoding {
2222
/// The path tag stream.
2323
pub path_tags: Vec<PathTag>,
2424
/// The path data stream.
25-
pub path_data: Vec<u8>,
25+
/// Stores all coordinates on paths.
26+
/// Stored as `u32` as all comparisons are performed bitwise.
27+
pub path_data: Vec<u32>,
2628
/// The draw tag stream.
2729
pub draw_tags: Vec<DrawTag>,
2830
/// The draw data stream.
29-
pub draw_data: Vec<u8>,
31+
pub draw_data: Vec<u32>,
3032
/// The transform stream.
3133
pub transforms: Vec<Transform>,
3234
/// The style stream
@@ -331,7 +333,8 @@ impl Encoding {
331333
pub fn encode_color(&mut self, color: impl Into<DrawColor>) {
332334
let color = color.into();
333335
self.draw_tags.push(DrawTag::COLOR);
334-
self.draw_data.extend_from_slice(bytemuck::bytes_of(&color));
336+
let DrawColor { rgba } = color;
337+
self.draw_data.push(rgba);
335338
}
336339

337340
/// Encodes a linear gradient brush.
@@ -350,7 +353,7 @@ impl Encoding {
350353
RampStops::Many => {
351354
self.draw_tags.push(DrawTag::LINEAR_GRADIENT);
352355
self.draw_data
353-
.extend_from_slice(bytemuck::bytes_of(&gradient));
356+
.extend_from_slice(bytemuck::cast_slice(bytemuck::bytes_of(&gradient)));
354357
}
355358
}
356359
}
@@ -375,7 +378,7 @@ impl Encoding {
375378
RampStops::Many => {
376379
self.draw_tags.push(DrawTag::RADIAL_GRADIENT);
377380
self.draw_data
378-
.extend_from_slice(bytemuck::bytes_of(&gradient));
381+
.extend_from_slice(bytemuck::cast_slice(bytemuck::bytes_of(&gradient)));
379382
}
380383
}
381384
}
@@ -399,7 +402,7 @@ impl Encoding {
399402
RampStops::Many => {
400403
self.draw_tags.push(DrawTag::SWEEP_GRADIENT);
401404
self.draw_data
402-
.extend_from_slice(bytemuck::bytes_of(&gradient));
405+
.extend_from_slice(bytemuck::cast_slice(bytemuck::bytes_of(&gradient)));
403406
}
404407
}
405408
}
@@ -416,14 +419,14 @@ impl Encoding {
416419
});
417420
self.draw_tags.push(DrawTag::IMAGE);
418421
self.draw_data
419-
.extend_from_slice(bytemuck::bytes_of(&DrawImage {
422+
.extend_from_slice(bytemuck::cast_slice(bytemuck::bytes_of(&DrawImage {
420423
xy: 0,
421424
width_height: (image.width << 16) | (image.height & 0xFFFF),
422425
sample_alpha: ((image.quality as u32) << 12)
423426
| ((image.x_extend as u32) << 10)
424427
| ((image.y_extend as u32) << 8)
425428
| alpha as u32,
426-
}));
429+
})));
427430
}
428431

429432
// Encodes a blurred rounded rectangle brush.
@@ -437,21 +440,25 @@ impl Encoding {
437440
) {
438441
self.draw_tags.push(DrawTag::BLUR_RECT);
439442
self.draw_data
440-
.extend_from_slice(bytemuck::bytes_of(&DrawBlurRoundedRect {
441-
color: color.into(),
442-
width,
443-
height,
444-
radius,
445-
std_dev,
446-
}));
443+
.extend_from_slice(bytemuck::cast_slice(bytemuck::bytes_of(
444+
&DrawBlurRoundedRect {
445+
color: color.into(),
446+
width,
447+
height,
448+
radius,
449+
std_dev,
450+
},
451+
)));
447452
}
448453

449454
/// Encodes a begin clip command.
450455
pub fn encode_begin_clip(&mut self, blend_mode: BlendMode, alpha: f32) {
451456
use super::DrawBeginClip;
452457
self.draw_tags.push(DrawTag::BEGIN_CLIP);
453458
self.draw_data
454-
.extend_from_slice(bytemuck::bytes_of(&DrawBeginClip::new(blend_mode, alpha)));
459+
.extend_from_slice(bytemuck::cast_slice(bytemuck::bytes_of(
460+
&DrawBeginClip::new(blend_mode, alpha),
461+
)));
455462
self.n_clips += 1;
456463
self.n_open_clips += 1;
457464
}

vello_encoding/src/path.rs

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,7 @@ pub struct Tile {
418418
/// Encoder for path segments.
419419
pub struct PathEncoder<'a> {
420420
tags: &'a mut Vec<PathTag>,
421-
data: &'a mut Vec<u8>,
421+
data: &'a mut Vec<u32>,
422422
n_segments: &'a mut u32,
423423
n_paths: &'a mut u32,
424424
first_point: [f32; 2],
@@ -474,7 +474,7 @@ impl<'a> PathEncoder<'a> {
474474
/// line-to), the thread draws nothing.
475475
pub fn new(
476476
tags: &'a mut Vec<PathTag>,
477-
data: &'a mut Vec<u8>,
477+
data: &'a mut Vec<u32>,
478478
n_segments: &'a mut u32,
479479
n_paths: &'a mut u32,
480480
is_fill: bool,
@@ -498,9 +498,9 @@ impl<'a> PathEncoder<'a> {
498498
self.close();
499499
}
500500
let buf = [x, y];
501-
let bytes = bytemuck::bytes_of(&buf);
501+
let bytes = bytemuck::cast_slice(&buf);
502502
if self.state == PathState::MoveTo {
503-
let new_len = self.data.len() - 8;
503+
let new_len = self.data.len() - 2;
504504
self.data.truncate(new_len);
505505
} else if self.state == PathState::NonemptySubpath {
506506
if !self.is_fill {
@@ -538,7 +538,7 @@ impl<'a> PathEncoder<'a> {
538538
return;
539539
}
540540
let buf = [x, y];
541-
let bytes = bytemuck::bytes_of(&buf);
541+
let bytes = bytemuck::cast_slice(&buf);
542542
self.data.extend_from_slice(bytes);
543543
self.tags.push(PathTag::LINE_TO_F32);
544544
self.state = PathState::NonemptySubpath;
@@ -566,7 +566,7 @@ impl<'a> PathEncoder<'a> {
566566
return;
567567
}
568568
let buf = [x1, y1, x2, y2];
569-
let bytes = bytemuck::bytes_of(&buf);
569+
let bytes = bytemuck::cast_slice(&buf);
570570
self.data.extend_from_slice(bytes);
571571
self.tags.push(PathTag::QUAD_TO_F32);
572572
self.state = PathState::NonemptySubpath;
@@ -594,7 +594,7 @@ impl<'a> PathEncoder<'a> {
594594
return;
595595
}
596596
let buf = [x1, y1, x2, y2, x3, y3];
597-
let bytes = bytemuck::bytes_of(&buf);
597+
let bytes = bytemuck::cast_slice(&buf);
598598
self.data.extend_from_slice(bytes);
599599
self.tags.push(PathTag::CUBIC_TO_F32);
600600
self.state = PathState::NonemptySubpath;
@@ -604,7 +604,7 @@ impl<'a> PathEncoder<'a> {
604604
/// Encodes an empty path (as placeholder for begin clip).
605605
pub(crate) fn empty_path(&mut self) {
606606
let coords = [0.0_f32, 0., 0., 0.];
607-
let bytes = bytemuck::bytes_of(&coords);
607+
let bytes = bytemuck::cast_slice(&coords);
608608
self.data.extend_from_slice(bytes);
609609
self.tags.push(PathTag::LINE_TO_F32);
610610
self.n_encoded_segments += 1;
@@ -615,20 +615,23 @@ impl<'a> PathEncoder<'a> {
615615
match self.state {
616616
PathState::Start => return,
617617
PathState::MoveTo => {
618-
let new_len = self.data.len() - 8;
618+
// If we close a new-opened path, delete it.
619+
let new_len = self.data.len() - 2;
619620
self.data.truncate(new_len);
620621
self.state = PathState::Start;
621622
return;
622623
}
623624
PathState::NonemptySubpath => (),
624625
}
625626
let len = self.data.len();
626-
if len < 8 {
627-
// can't happen
627+
if len < 2 {
628+
if cfg!(debug_assertions) {
629+
unreachable!("There is an open path, so there must be data.")
630+
}
628631
return;
629632
}
630-
let first_bytes = bytemuck::bytes_of(&self.first_point);
631-
if &self.data[len - 8..len] != first_bytes {
633+
let first_bytes = bytemuck::cast_slice(&self.first_point);
634+
if &self.data[len - 2..len] != first_bytes {
632635
self.data.extend_from_slice(first_bytes);
633636
self.tags.push(PathTag::LINE_TO_F32);
634637
self.n_encoded_segments += 1;
@@ -722,12 +725,12 @@ impl<'a> PathEncoder<'a> {
722725

723726
fn last_point(&self) -> Option<(f32, f32)> {
724727
let len = self.data.len();
725-
if len < 8 {
728+
if len < 2 {
726729
return None;
727730
}
728731
Some((
729-
bytemuck::pod_read_unaligned::<f32>(&self.data[len - 8..len - 4]),
730-
bytemuck::pod_read_unaligned::<f32>(&self.data[len - 4..len]),
732+
bytemuck::cast(self.data[len - 2]),
733+
bytemuck::cast(self.data[len - 1]),
731734
))
732735
}
733736

vello_encoding/src/resolve.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -270,31 +270,35 @@ impl Resolver {
270270
extend,
271271
} => {
272272
if pos < *draw_data_offset {
273-
data.extend_from_slice(&encoding.draw_data[pos..*draw_data_offset]);
273+
data.extend_from_slice(bytemuck::cast_slice(
274+
&encoding.draw_data[pos..*draw_data_offset],
275+
));
274276
}
275277
let index_mode = (ramp_id << 2) | *extend as u32;
276278
data.extend_from_slice(bytemuck::bytes_of(&index_mode));
277-
pos = *draw_data_offset + 4;
279+
pos = *draw_data_offset + 1;
278280
}
279281
ResolvedPatch::GlyphRun { .. } => {}
280282
ResolvedPatch::Image {
281283
index,
282284
draw_data_offset,
283285
} => {
284286
if pos < *draw_data_offset {
285-
data.extend_from_slice(&encoding.draw_data[pos..*draw_data_offset]);
287+
data.extend_from_slice(bytemuck::cast_slice(
288+
&encoding.draw_data[pos..*draw_data_offset],
289+
));
286290
}
287291
if let Some((x, y)) = self.pending_images[*index].xy {
288292
let xy = (x << 16) | y;
289293
data.extend_from_slice(bytemuck::bytes_of(&xy));
290-
pos = *draw_data_offset + 4;
294+
pos = *draw_data_offset + 1;
291295
} else {
292296
// If we get here, we failed to allocate a slot for this image in the atlas.
293297
// In this case, let's zero out the dimensions so we don't attempt to render
294298
// anything.
295299
// TODO: a better strategy: texture array? downsample large images?
296300
data.extend_from_slice(&[0_u8; 8]);
297-
pos = *draw_data_offset + 8;
301+
pos = *draw_data_offset + 2;
298302
}
299303
}
300304
}

0 commit comments

Comments
 (0)