Fix subpixel positioning

examples/lorem-ipsum.rs

@@ -522,22 +522,29 @@ impl Renderer {
                 None => continue,
                 Some(glyph_index) => glyph_index,
             };
-            let glyph_bounds = outline_builder.glyph_bounds(glyph_index);
-            let uv_rect = atlas_builder.atlas_rect(glyph_index);
-            let (uv_bl, uv_tr) = (uv_rect.origin, uv_rect.bottom_right());
 
-            let left_pos = (position.x as f32 * pixels_per_unit).round() as i32;
+            let glyph_rect_i = outline_builder.glyph_pixel_bounds_i(glyph_index, point_size);
-            let top_pos = ((position.y as f32 - glyph_bounds.top as f32)
+
-                           * pixels_per_unit).round() as i32;
+            let uv_tl: Point2D<u32> = atlas_builder.atlas_origin(glyph_index)
-            let right_pos = left_pos + uv_rect.size.width as i32;
+                                                   .floor()
-            let bottom_pos = top_pos + uv_rect.size.height as i32;
+                                                   .cast()
+                                                   .unwrap();
+            let uv_br = uv_tl + glyph_rect_i.size().cast().unwrap();
+
+            let bearing_pos = (position.x as f32 * pixels_per_unit).round() as i32;
+            let baseline_pos = (position.y as f32 * pixels_per_unit).round() as i32;
+
+            let left_pos = bearing_pos + glyph_rect_i.left;
+            let top_pos = baseline_pos - glyph_rect_i.top;
+            let right_pos = bearing_pos + glyph_rect_i.right;
+            let bottom_pos = baseline_pos - glyph_rect_i.bottom;
 
             let first_index = vertices.len() as u16;
 
-            vertices.push(Vertex::new(left_pos,  bottom_pos, uv_bl.x, uv_tr.y));
+            vertices.push(Vertex::new(left_pos,  top_pos,    uv_tl.x, uv_tl.y));
-            vertices.push(Vertex::new(right_pos, bottom_pos, uv_tr.x, uv_tr.y));
+            vertices.push(Vertex::new(right_pos, top_pos,    uv_br.x, uv_tl.y));
-            vertices.push(Vertex::new(right_pos, top_pos,    uv_tr.x, uv_bl.y));
+            vertices.push(Vertex::new(right_pos, bottom_pos, uv_br.x, uv_br.y));
-            vertices.push(Vertex::new(left_pos,  top_pos,    uv_bl.x, uv_bl.y));
+            vertices.push(Vertex::new(left_pos,  bottom_pos, uv_tl.x, uv_br.y));
 
             indices.extend(RECT_INDICES.iter().map(|index| first_index + index));
         }

resources/shaders/draw.vs.glsl

@@ -33,7 +33,7 @@ layout(std140) uniform ubGlyphDescriptors {
 };
 
 layout(std140) uniform ubImageDescriptors {
-    uvec4 uImages[MAX_GLYPHS];
+    vec4 uImages[MAX_GLYPHS];
 };
 
 // The position of each vertex in glyph space.
@@ -50,13 +50,13 @@ flat out int vVertexID;
 void main() {
     vVertexID = gl_VertexID;
 
-    uvec4 image = uImages[aGlyphIndex];
+    vec4 image = uImages[aGlyphIndex];
     GlyphDescriptor glyph = uGlyphs[aGlyphIndex];
 
     vec2 glyphPos = vec2(aPosition.x - glyph.extents.x, glyph.extents.w - aPosition.y);
-    float pointSize = float(IMAGE_DESCRIPTOR_POINT_SIZE(image)) / 65536.0f;
+    float pointSize = IMAGE_DESCRIPTOR_POINT_SIZE(image);
     vec2 glyphPxPos = glyphPos * pointSize / GLYPH_DESCRIPTOR_UNITS_PER_EM(glyph);
-    vec2 atlasPos = glyphPxPos + vec2(IMAGE_DESCRIPTOR_ATLAS_POS(image));
+    vec2 atlasPos = glyphPxPos + IMAGE_DESCRIPTOR_ATLAS_POS(image);
 
     gl_Position = vec4(atlasPos, 0.0f, 1.0f);
 }

src/atlas.rs

@@ -43,18 +43,12 @@ impl AtlasBuilder {
                       glyph_index: u32,
                       point_size: f32)
                       -> Result<(), ()> {
-        // FIXME(pcwalton): I think this will check for negative values and panic, which is
+        let pixel_bounds_f = outline_builder.glyph_pixel_bounds_f(glyph_index, point_size);
-        // unnecessary.
+        let pixel_bounds_i = outline_builder.glyph_pixel_bounds_i(glyph_index, point_size);
-        let pixel_size = outline_builder.glyph_pixel_bounds(glyph_index, point_size)
+
-                                        .size
+        let atlas_origin = try!(self.rect_packer.pack(&pixel_bounds_i.size().cast().unwrap()));
-                                        .ceil()
-                                        .cast()
-                                        .unwrap();
 
         let glyph_id = outline_builder.glyph_id(glyph_index);
-
-        let atlas_origin = try!(self.rect_packer.pack(&pixel_size));
-
         let glyph_index = self.image_descriptors.len() as u32;
 
         while self.image_descriptors.len() < glyph_index as usize + 1 {
@@ -62,10 +56,10 @@ impl AtlasBuilder {
         }
 
         self.image_descriptors[glyph_index as usize] = ImageDescriptor {
-            atlas_x: atlas_origin.x,
+            atlas_x: atlas_origin.x as f32 + pixel_bounds_f.left.fract(),
-            atlas_y: atlas_origin.y,
+            atlas_y: atlas_origin.y as f32 + (1.0 - pixel_bounds_f.top.fract()),
-            point_size: (point_size * 65536.0) as u32,
+            point_size: point_size,
-            glyph_index: glyph_index,
+            glyph_index: glyph_index as f32,
         };
 
         while self.image_metadata.len() < glyph_index as usize + 1 {
@@ -73,8 +67,6 @@ impl AtlasBuilder {
         }
 
         self.image_metadata[glyph_index as usize] = ImageMetadata {
-            atlas_width: pixel_size.width,
-            atlas_height: pixel_size.height,
             glyph_index: glyph_index,
             glyph_id: glyph_id,
         };
@@ -143,11 +135,9 @@ impl AtlasBuilder {
     }
 
     #[inline]
-    pub fn atlas_rect(&self, glyph_index: u32) -> Rect<u32> {
+    pub fn atlas_origin(&self, glyph_index: u32) -> Point2D<f32> {
         let descriptor = &self.image_descriptors[glyph_index as usize];
-        let metadata = &self.image_metadata[glyph_index as usize];
+        Point2D::new(descriptor.atlas_x, descriptor.atlas_y)
-        Rect::new(Point2D::new(descriptor.atlas_x, descriptor.atlas_y),
-                  Size2D::new(metadata.atlas_width, metadata.atlas_height))
     }
 }
 
@@ -188,17 +178,15 @@ impl Atlas {
 #[repr(C)]
 #[derive(Clone, Copy, Default, Debug)]
 pub struct ImageDescriptor {
-    atlas_x: u32,
+    atlas_x: f32,
-    atlas_y: u32,
+    atlas_y: f32,
-    point_size: u32,
+    point_size: f32,
-    glyph_index: u32,
+    glyph_index: f32,
 }
 
 /// Information about each image that we keep around ourselves.
 #[derive(Clone, Copy, Default, Debug)]
 pub struct ImageMetadata {
-    atlas_width: u32,
-    atlas_height: u32,
     glyph_index: u32,
     glyph_id: u16,
 }

src/otf/glyf.rs

@@ -13,7 +13,7 @@ use euclid::Point2D;
 use otf::head::HeadTable;
 use otf::loca::LocaTable;
 use otf::{Error, FontTable};
-use outline::GlyphBounds;
+use outline::GlyphBoundsI;
 use std::mem;
 use std::ops::Mul;
 use util::Jump;
@@ -273,13 +273,13 @@ impl<'a> GlyfTable<'a> {
     }
 
     pub fn glyph_bounds(&self, head_table: &HeadTable, loca_table: &LocaTable, glyph_id: u16)
-                        -> Result<GlyphBounds, Error> {
+                        -> Result<GlyphBoundsI, Error> {
         let mut reader = self.table.bytes;
 
         match try!(loca_table.location_of(head_table, glyph_id)) {
             None => {
                 // No outlines.
-                return Ok(GlyphBounds {
+                return Ok(GlyphBoundsI {
                     left: 0,
                     bottom: 0,
                     right: 0,
@@ -296,7 +296,7 @@ impl<'a> GlyfTable<'a> {
         let y_min = try!(reader.read_i16::<BigEndian>().map_err(Error::eof));
         let x_max = try!(reader.read_i16::<BigEndian>().map_err(Error::eof));
         let y_max = try!(reader.read_i16::<BigEndian>().map_err(Error::eof));
-        Ok(GlyphBounds {
+        Ok(GlyphBoundsI {
             left: x_min as i32,
             bottom: y_min as i32,
             right: x_max as i32,

src/otf/head.rs

@@ -10,7 +10,7 @@
 
 use byteorder::{BigEndian, ReadBytesExt};
 use otf::{Error, FontTable};
-use outline::GlyphBounds;
+use outline::GlyphBoundsI;
 use std::mem;
 use util::Jump;
 
@@ -20,7 +20,7 @@ const MAGIC_NUMBER: u32 = 0x5f0f3cf5;
 pub struct HeadTable {
     pub units_per_em: u16,
     pub index_to_loc_format: i16,
-    pub max_glyph_bounds: GlyphBounds,
+    pub max_glyph_bounds: GlyphBoundsI,
 }
 
 impl HeadTable {
@@ -51,7 +51,7 @@ impl HeadTable {
         let y_min = try!(reader.read_i16::<BigEndian>().map_err(Error::eof));
         let x_max = try!(reader.read_i16::<BigEndian>().map_err(Error::eof));
         let y_max = try!(reader.read_i16::<BigEndian>().map_err(Error::eof));
-        let max_glyph_bounds = GlyphBounds {
+        let max_glyph_bounds = GlyphBoundsI {
             left: x_min as i32,
             bottom: y_min as i32,
             right: x_max as i32,

src/otf/mod.rs

@@ -17,7 +17,7 @@ use otf::head::HeadTable;
 use otf::hhea::HheaTable;
 use otf::hmtx::{HmtxTable, HorizontalMetrics};
 use otf::loca::LocaTable;
-use outline::GlyphBounds;
+use outline::GlyphBoundsI;
 use std::mem;
 use std::u16;
 use util::Jump;
@@ -270,7 +270,7 @@ impl<'a> Font<'a> {
     }
 
     #[inline]
-    pub fn glyph_bounds(&self, glyph_id: u16) -> Result<GlyphBounds, Error> {
+    pub fn glyph_bounds(&self, glyph_id: u16) -> Result<GlyphBoundsI, Error> {
         match self.glyf {
             Some(glyf) => {
                 let loca = match self.loca {
@@ -286,8 +286,12 @@ impl<'a> Font<'a> {
 
     #[inline]
     pub fn shelf_height(&self, point_size: f32) -> u32 {
-        let pixel_rect = self.head.max_glyph_bounds.pixel_rect(self.head.units_per_em, point_size);
+        self.head
-        pixel_rect.round_out().size.height as u32
+            .max_glyph_bounds
+            .pixel_rect_f(self.head.units_per_em, point_size)
+            .to_i()
+            .size()
+            .height as u32
     }
 
     #[inline]

src/outline.rs

@@ -80,14 +80,20 @@ impl OutlineBuilder {
 
     /// Returns the glyph rectangle in units.
     #[inline]
-    pub fn glyph_bounds(&self, glyph_index: u32) -> GlyphBounds {
+    pub fn glyph_bounds(&self, glyph_index: u32) -> GlyphBoundsI {
         self.descriptors[glyph_index as usize].bounds
     }
 
-    /// Returns the glyph rectangle in pixels.
+    /// Returns the glyph rectangle in fractional pixels.
     #[inline]
-    pub fn glyph_pixel_bounds(&self, glyph_index: u32, point_size: f32) -> Rect<f32> {
+    pub fn glyph_pixel_bounds_f(&self, glyph_index: u32, point_size: f32) -> GlyphBoundsF {
-        self.descriptors[glyph_index as usize].pixel_rect(point_size)
+        self.descriptors[glyph_index as usize].pixel_rect_f(point_size)
+    }
+
+    /// Returns the glyph rectangle, rounded out to the nearest pixel.
+    #[inline]
+    pub fn glyph_pixel_bounds_i(&self, glyph_index: u32, point_size: f32) -> GlyphBoundsI {
+        self.descriptors[glyph_index as usize].pixel_rect_i(point_size)
     }
 
     /// Returns the ID of the glyph with the given index.
@@ -152,7 +158,7 @@ impl Drop for OutlineBuffers {
 #[repr(C)]
 #[derive(Clone, Copy, Debug)]
 pub struct GlyphDescriptor {
-    pub bounds: GlyphBounds,
+    pub bounds: GlyphBoundsI,
     pub units_per_em: u32,
     pub start_point: u32,
     pub start_index: u32,
@@ -161,8 +167,13 @@ pub struct GlyphDescriptor {
 
 impl GlyphDescriptor {
     #[inline]
-    fn pixel_rect(&self, point_size: f32) -> Rect<f32> {
+    fn pixel_rect_f(&self, point_size: f32) -> GlyphBoundsF {
-        self.bounds.pixel_rect(self.units_per_em as u16, point_size)
+        self.bounds.pixel_rect_f(self.units_per_em as u16, point_size)
+    }
+
+    #[inline]
+    fn pixel_rect_i(&self, point_size: f32) -> GlyphBoundsI {
+        self.bounds.pixel_rect_f(self.units_per_em as u16, point_size).to_i()
     }
 }
 
@@ -177,20 +188,53 @@ pub struct Vertex {
 }
 
 #[derive(Copy, Clone, Debug)]
-pub struct GlyphBounds {
+pub struct GlyphBoundsF {
+    pub left: f32,
+    pub bottom: f32,
+    pub right: f32,
+    pub top: f32,
+}
+
+impl GlyphBoundsF {
+    #[inline]
+    pub fn to_i(&self) -> GlyphBoundsI {
+        GlyphBoundsI {
+            left: self.left.floor() as i32,
+            bottom: self.bottom.floor() as i32,
+            right: self.right.ceil() as i32,
+            top: self.top.ceil() as i32,
+        }
+    }
+
+    #[inline]
+    pub fn size(&self) -> Size2D<f32> {
+        Size2D::new(self.right - self.left, self.top - self.bottom)
+    }
+}
+
+#[derive(Copy, Clone, Debug)]
+pub struct GlyphBoundsI {
     pub left: i32,
     pub bottom: i32,
     pub right: i32,
     pub top: i32,
 }
 
-impl GlyphBounds {
+impl GlyphBoundsI {
     #[inline]
-    pub fn pixel_rect(&self, units_per_em: u16, point_size: f32) -> Rect<f32> {
+    pub fn pixel_rect_f(&self, units_per_em: u16, point_size: f32) -> GlyphBoundsF {
         let pixels_per_unit = point_size / units_per_em as f32;
-        Rect::new(Point2D::new(self.left as f32, self.bottom as f32),
+        GlyphBoundsF {
-                  Size2D::new((self.right - self.left) as f32, (self.top - self.bottom) as f32)) *
+            left: self.left as f32 * pixels_per_unit,
-            pixels_per_unit
+            bottom: self.bottom as f32 * pixels_per_unit,
+            right: self.right as f32 * pixels_per_unit,
+            top: self.top as f32 * pixels_per_unit,
+        }
+    }
+
+    #[inline]
+    pub fn size(&self) -> Size2D<i32> {
+        Size2D::new(self.right - self.left, self.top - self.bottom)
     }
 }