List item improvements (#136)

* Go back to just using parley layouts reglardless of single glyph or string

* cleanup warnings

packages/blitz/src/renderer/render.rs

@@ -7,7 +7,8 @@ use crate::{
     util::{GradientSlice, StyloGradient, ToVelloColor},
 };
 use blitz_dom::node::{
-    ListItemLayout, ListItemLayoutPosition, NodeData, TextBrush, TextInputData, TextNodeData,
+    ListItemLayout, ListItemLayoutPosition, Marker, NodeData, TextBrush, TextInputData,
+    TextNodeData,
 };
 use blitz_dom::{local_name, Document, Node};
 
@@ -46,11 +47,8 @@ use style::values::generics::image::{
 };
 use style::values::specified::percentage::ToPercentage;
 use taffy::prelude::Layout;
-use vello::glyph::skrifa::prelude::{NormalizedCoord, Size};
-use vello::glyph::skrifa::{FontRef, GlyphId};
 use vello::kurbo::{BezPath, Cap, Join};
 use vello::peniko::Gradient;
-use vello::skrifa::MetadataProvider;
 use vello::{
     kurbo::{Affine, Point, Rect, Shape, Stroke, Vec2},
     peniko::{self, Brush, Color, Fill, Mix},
@@ -410,58 +408,37 @@ impl<'dom> VelloSceneGenerator<'dom> {
                 );
             }
         } else if let Some(ListItemLayout {
-            marker: _,
-            position,
+            marker,
+            position: ListItemLayoutPosition::Outside(layout),
         }) = cx.list_item
         {
-            match position {
-                ListItemLayoutPosition::OutsideGlyph {
-                    font,
-                    glyph_id,
-                    font_size_px,
-                    line_height_px,
-                    color,
-                } => {
-                    let font_ref = FontRef::from_index(font.data.data(), font.index).unwrap();
-                    let variations: &[(&str, f32)] = &[];
-                    let location = font_ref.axes().location(variations);
-                    let glyph_metrics = font_ref.glyph_metrics(Size::new(*font_size_px), &location);
-                    let glyph_width = glyph_metrics
-                        .advance_width(GlyphId::new(*glyph_id))
-                        .unwrap();
-                    let metrics = font_ref.metrics(Size::new(*font_size_px), &location);
-                    let coords = location.coords();
-                    let pos = Point {
-                        // Right align the glyph, and add some gap between the glyph and the following list item text
-                        x: pos.x - glyph_width as f64 - 8.0,
-                        // Center the glyph on the line
-                        y: pos.y
-                            + (*line_height_px as f64 + metrics.ascent as f64
-                                - metrics.descent as f64)
-                                / 2.0,
-                    };
-                    cx.stroke_glyph(
-                        scene,
-                        Glyph {
-                            glyph_id: *glyph_id,
-                            font,
-                            font_size: *font_size_px,
-                            coords,
-                            brush: &Brush::Solid(*color),
-                        },
-                        pos,
-                    );
-                }
-                ListItemLayoutPosition::OutsideString { layout } => {
-                    //Right align and pad the bullet when rendering outside
-                    let pos = Point {
-                        x: pos.x - (layout.full_width() / layout.scale()) as f64,
-                        y: pos.y,
-                    };
-                    cx.stroke_text(scene, layout, pos);
+            //Right align and pad the bullet when rendering outside
+            let x_padding = match marker {
+                Marker::Char(_) => 8.0,
+                Marker::String(_) => 0.0,
+            };
+            let x_offset = -(layout.full_width() / layout.scale() + x_padding);
+            //Align the marker with the baseline of the first line of text in the list item
+            let y_offset = if let Some(text_layout) = &element
+                .raw_dom_data
+                .downcast_element()
+                .and_then(|el| el.inline_layout_data.as_ref())
+            {
+                if let Some(first_text_line) = text_layout.layout.lines().next() {
+                    (first_text_line.metrics().baseline
+                        - layout.lines().next().unwrap().metrics().baseline)
+                        / layout.scale()
+                } else {
+                    0.0
                 }
-                ListItemLayoutPosition::Inside => {}
-            }
+            } else {
+                0.0
+            };
+            let pos = Point {
+                x: pos.x + x_offset as f64,
+                y: pos.y + y_offset as f64,
+            };
+            cx.stroke_text(scene, layout, pos);
         }
 
         if element.is_inline_root {
@@ -566,16 +543,6 @@ impl<'dom> VelloSceneGenerator<'dom> {
     }
 }
 
-/// A glyph to be rendered by an ELementCx
-#[derive(Debug)]
-struct Glyph<'a> {
-    glyph_id: u16,
-    font: &'a parley::Font,
-    font_size: f32,
-    coords: &'a [NormalizedCoord],
-    brush: &'a Brush,
-}
-
 /// A context of loaded and hot data to draw the element from
 struct ElementCx<'a> {
     frame: ElementFrame,
@@ -592,24 +559,6 @@ struct ElementCx<'a> {
 }
 
 impl ElementCx<'_> {
-    fn stroke_glyph(&self, scene: &mut Scene, glyph: Glyph, pos: Point) {
-        let transform = Affine::translate((pos.x * self.scale, pos.y * self.scale));
-        scene
-            .draw_glyphs(glyph.font)
-            .font_size(glyph.font_size * self.scale as f32)
-            .normalized_coords(glyph.coords)
-            .brush(glyph.brush)
-            .transform(transform)
-            .draw(
-                Fill::NonZero,
-                std::iter::once(vello::glyph::Glyph {
-                    id: glyph.glyph_id as u32,
-                    x: 0.0,
-                    y: 0.0,
-                }),
-            )
-    }
-
     fn stroke_text(&self, scene: &mut Scene, text_layout: &parley::Layout<TextBrush>, pos: Point) {
         let transform = Affine::translate((pos.x * self.scale, pos.y * self.scale));
 

packages/dom/src/document.rs

@@ -3,7 +3,6 @@ use crate::node::{NodeSpecificData, TextBrush};
 use crate::{ElementNodeData, Node, NodeData, TextNodeData, Viewport};
 use app_units::Au;
 use html5ever::local_name;
-use parley::fontique::FamilyId;
 use peniko::kurbo;
 // use quadtree_rs::Quadtree;
 use parley::editor::{PointerButton, TextEvent};
@@ -105,8 +104,6 @@ pub struct Document {
     /// A Parley font context
     pub(crate) font_ctx: parley::FontContext,
 
-    pub(crate) bullet_font_id: FamilyId,
-
     /// A Parley layout context
     pub(crate) layout_ctx: parley::LayoutContext<TextBrush>,
 
@@ -223,7 +220,7 @@ impl Document {
         style_config::set_bool("layout.legacy_layout", true);
         style_config::set_bool("layout.columns.enabled", true);
 
-        let (font_ctx, bullet_font_id) = Self::create_font_context();
+        let font_ctx = Self::create_font_context();
 
         let mut doc = Self {
             guard,
@@ -237,7 +234,6 @@ impl Document {
             // quadtree: Quadtree::new(20),
             stylesheets: HashMap::new(),
             font_ctx,
-            bullet_font_id,
             layout_ctx: parley::LayoutContext::new(),
 
             hover_node_id: None,
@@ -251,12 +247,11 @@ impl Document {
         doc
     }
 
-    fn create_font_context() -> (parley::FontContext, FamilyId) {
+    fn create_font_context() -> parley::FontContext {
         let mut ctx = parley::FontContext::default();
-        let names = ctx
-            .collection
+        ctx.collection
             .register_fonts(include_bytes!("moz-bullet-font.otf").to_vec());
-        (ctx, names[0].0)
+        ctx
     }
 
     /// Set base url for resolving linked resources (stylesheets, images, fonts, etc)

packages/dom/src/layout/construct.rs

@@ -5,10 +5,8 @@ use html5ever::{local_name, namespace_url, ns, QualName};
 use parley::{
     builder::TreeBuilder,
     style::{FontStack, WhiteSpaceCollapse},
-    swash::FontRef,
     InlineBox,
 };
-use peniko::Font;
 use slab::Slab;
 use style::{
     data::ElementData,
@@ -18,7 +16,7 @@ use style::{
     },
     shared_lock::StylesheetGuards,
     values::{
-        computed::{Display, LineHeight},
+        computed::Display,
         specified::box_::{DisplayInside, DisplayOutside},
     },
 };
@@ -28,9 +26,7 @@ use crate::{
         ListItemLayout, ListItemLayoutPosition, Marker, NodeKind, NodeSpecificData, TextBrush,
         TextInputData, TextLayout,
     },
-    stylo_to_parley,
-    util::ToPenikoColor,
-    Document, ElementNodeData, Node, NodeData,
+    stylo_to_parley, Document, ElementNodeData, Node, NodeData,
 };
 
 use super::table::build_table_context;
@@ -269,35 +265,9 @@ fn node_list_item_child(doc: &mut Document, child: usize, index: usize) -> Optio
 
     let marker = marker_for_style(list_style_type, index)?;
 
-    let position = match (list_style_position, &marker) {
-        (ListStylePosition::Inside, _) => ListItemLayoutPosition::Inside,
-        (ListStylePosition::Outside, Marker::Char(char)) => {
-            let family_info = doc.font_ctx.collection.family(doc.bullet_font_id).unwrap();
-            let bullet_font = doc
-                .font_ctx
-                .source_cache
-                .get(family_info.default_font().unwrap().source())
-                .unwrap();
-            let font_ref =
-                FontRef::from_index(bullet_font.data(), family_info.default_font_index()).unwrap();
-            let font_size_px = styles.get_font().font_size.used_size.px();
-            let color = styles.get_inherited_text().color.as_peniko();
-            let glyph_id = font_ref.charmap().map(*char);
-
-            let line_height_px: f32 = match styles.clone_line_height() {
-                LineHeight::Normal => font_size_px * 1.2,
-                LineHeight::Number(num) => font_size_px * num.0,
-                LineHeight::Length(value) => value.0.px(),
-            };
-            ListItemLayoutPosition::OutsideGlyph {
-                font: Font::new(bullet_font, family_info.default_font().unwrap().index()),
-                glyph_id,
-                font_size_px,
-                line_height_px,
-                color,
-            }
-        }
-        (ListStylePosition::Outside, Marker::String(str)) => {
+    let position = match list_style_position {
+        ListStylePosition::Inside => ListItemLayoutPosition::Inside,
+        ListStylePosition::Outside => {
             let mut parley_style = stylo_to_parley::style(&styles);
 
             if let Some(font_stack) = font_for_bullet_style(list_style_type) {
@@ -309,15 +279,16 @@ fn node_list_item_child(doc: &mut Document, child: usize, index: usize) -> Optio
                 doc.layout_ctx
                     .tree_builder(&mut doc.font_ctx, doc.viewport.scale(), &parley_style);
 
-            builder.push_text(str);
+            match &marker {
+                Marker::Char(char) => builder.push_text(&char.to_string()),
+                Marker::String(str) => builder.push_text(str),
+            };
 
             let mut layout = builder.build().0;
 
             layout.break_all_lines(Some(0.0));
 
-            ListItemLayoutPosition::OutsideString {
-                layout: Box::new(layout),
-            }
+            ListItemLayoutPosition::Outside(Box::new(layout))
         }
     };
 

packages/dom/src/node.rs

@@ -1,7 +1,7 @@
 use atomic_refcell::{AtomicRef, AtomicRefCell};
 use html5ever::{local_name, LocalName, QualName};
 use image::DynamicImage;
-use peniko::{kurbo, Font};
+use peniko::kurbo;
 use selectors::matching::QuirksMode;
 use slab::Slab;
 use std::cell::RefCell;
@@ -557,16 +557,7 @@ pub enum Marker {
 #[derive(Clone)]
 pub enum ListItemLayoutPosition {
     Inside,
-    OutsideGlyph {
-        font: Font,
-        glyph_id: parley::swash::GlyphId,
-        font_size_px: f32,
-        line_height_px: f32,
-        color: peniko::Color,
-    },
-    OutsideString {
-        layout: Box<parley::Layout<TextBrush>>,
-    },
+    Outside(Box<parley::Layout<TextBrush>>),
 }
 
 impl std::fmt::Debug for ListItemLayout {