From f1b81456a66d2ca274a78439612e006687955414 Mon Sep 17 00:00:00 2001 From: Dustin Carlino Date: Wed, 23 Jan 2019 13:46:05 -0800 Subject: [PATCH] calculating line height from the font --- docs/TODO_ux.md | 11 ++-- editor/src/plugins/view/turn_cycler.rs | 4 +- ezgui/src/canvas.rs | 18 +++++-- ezgui/src/lib.rs | 4 -- ezgui/src/log_scroller.rs | 2 +- ezgui/src/menu.rs | 3 +- ezgui/src/runner.rs | 7 ++- ezgui/src/scrolling_menu.rs | 2 +- ezgui/src/text.rs | 73 ++++++++++++++------------ ezgui/src/top_menu.rs | 9 ++-- 10 files changed, 75 insertions(+), 58 deletions(-) diff --git a/docs/TODO_ux.md b/docs/TODO_ux.md index 1f048291a4..750f5e5a3a 100644 --- a/docs/TODO_ux.md +++ b/docs/TODO_ux.md @@ -66,9 +66,9 @@ ## Switch to OpenGL (for speed) - no bugs - - text entry needs to draw the cursor differently anyway. - - top menu is very buggy - - need padding around text; and selecting stuff from menus is weird. LINE_HEIGHT. + - text entry needs to draw the cursor differently + - top menu is very buggy. MAX_CHAR_WIDTH is weird + - need padding around text; and selecting stuff from menus is weird - forking is buggy (traffic signal diagram) - arrows - some colors are wrong @@ -76,4 +76,7 @@ - make polygon store points and indices efficiently - change ezgui API to allow uploading geometry once - undo the y inversion hacks at last! -- probably use f32, not f64 everywhere +- refactoring + - pass canvas to text module, make it do the glyph borrowing? + - pass dims to draw_text_bubble; all callers have it anyway, right? + - probably use f32, not f64 everywhere diff --git a/editor/src/plugins/view/turn_cycler.rs b/editor/src/plugins/view/turn_cycler.rs index 5b0076d246..d1fa650c62 100644 --- a/editor/src/plugins/view/turn_cycler.rs +++ b/editor/src/plugins/view/turn_cycler.rs @@ -2,7 +2,7 @@ use crate::objects::{Ctx, ID}; use crate::plugins::{Plugin, PluginCtx}; use crate::render::{draw_signal_diagram, DrawTurn}; use dimensioned::si; -use ezgui::{Color, GfxCtx, Key, TOP_MENU_HEIGHT}; +use ezgui::{Color, GfxCtx, Key}; use map_model::{IntersectionID, LaneID, TurnType}; pub struct TurnCyclerState { @@ -101,7 +101,7 @@ impl Plugin for TurnCyclerState { i, cycle.idx, Some(time_left), - TOP_MENU_HEIGHT + 10.0, + ctx.canvas.top_menu_height() + 10.0, g, ctx, ); diff --git a/ezgui/src/canvas.rs b/ezgui/src/canvas.rs index cc5a101700..d8c8c9ba0e 100644 --- a/ezgui/src/canvas.rs +++ b/ezgui/src/canvas.rs @@ -25,6 +25,7 @@ pub struct Canvas { // TODO Super gross, but we can't create this immediately. pub(crate) glyphs: RefCell>>, + pub(crate) line_height: f64, // TODO Bit weird and hacky to mutate inside of draw() calls. covered_areas: RefCell>, @@ -46,6 +47,7 @@ impl Canvas { window_height: f64::from(initial_height), glyphs: RefCell::new(None), + line_height: 0.0, covered_areas: RefCell::new(Vec::new()), } } @@ -95,7 +97,7 @@ impl Canvas { pub fn draw_mouse_tooltip(&self, g: &mut GfxCtx, txt: Text) { let mut glyphs = self.glyphs.borrow_mut(); - let (width, height) = txt.dims(glyphs.as_mut().unwrap()); + let (width, height) = txt.dims(glyphs.as_mut().unwrap(), self); let x1 = self.cursor_x - (width / 2.0); let y1 = self.cursor_y - (height / 2.0); // No need to cover the tooltip; this tooltip follows the mouse anyway. @@ -111,7 +113,7 @@ impl Canvas { // TODO Rename these draw_nonblocking_text_* pub fn draw_text_at(&self, g: &mut GfxCtx, txt: Text, map_pt: Pt2D) { let mut glyphs = self.glyphs.borrow_mut(); - let (width, height) = txt.dims(glyphs.as_mut().unwrap()); + let (width, height) = txt.dims(glyphs.as_mut().unwrap(), self); let pt = self.map_to_screen(map_pt); text::draw_text_bubble( g, @@ -147,7 +149,7 @@ impl Canvas { return; } let mut glyphs = self.glyphs.borrow_mut(); - let (width, height) = txt.dims(glyphs.as_mut().unwrap()); + let (width, height) = txt.dims(glyphs.as_mut().unwrap(), self); let x1 = match horiz { HorizontalAlignment::Left => 0.0, HorizontalAlignment::Center => (self.window_width - width) / 2.0, @@ -155,7 +157,7 @@ impl Canvas { }; let y1 = match vert { VerticalAlignment::Top => 0.0, - VerticalAlignment::BelowTopMenu => text::LINE_HEIGHT, + VerticalAlignment::BelowTopMenu => self.line_height, VerticalAlignment::Center => (self.window_height - height) / 2.0, VerticalAlignment::Bottom => self.window_height - height, }; @@ -169,7 +171,7 @@ impl Canvas { } pub fn text_dims(&self, txt: &Text) -> (f64, f64) { - txt.dims(self.glyphs.borrow_mut().as_mut().unwrap()) + txt.dims(self.glyphs.borrow_mut().as_mut().unwrap(), self) } fn zoom_towards_mouse(&mut self, delta_zoom: f64) { @@ -237,6 +239,12 @@ impl Canvas { b.update(self.screen_to_map(ScreenPt::new(self.window_width, self.window_height))); b } + + // TODO Not super happy about exposing this; fork_screenspace for external callers should be + // smarter. + pub fn top_menu_height(&self) -> f64 { + self.line_height + } } pub enum HorizontalAlignment { diff --git a/ezgui/src/lib.rs b/ezgui/src/lib.rs index aa4166d1ff..5a58077429 100644 --- a/ezgui/src/lib.rs +++ b/ezgui/src/lib.rs @@ -27,10 +27,6 @@ pub use crate::wizard::{Wizard, WrappedWizard}; use geom::{Angle, Circle, Line, Polygon, Pt2D, Triangle}; use glium::{implement_vertex, uniform, Surface}; -// TODO Not super happy about exposing this; fork_screenspace for external callers should be -// smarter. -pub const TOP_MENU_HEIGHT: f64 = text::LINE_HEIGHT; - pub struct ToggleableLayer { layer_name: String, // If None, never automatically enable at a certain zoom level. diff --git a/ezgui/src/log_scroller.rs b/ezgui/src/log_scroller.rs index 851d1a72d6..2b9b8ff624 100644 --- a/ezgui/src/log_scroller.rs +++ b/ezgui/src/log_scroller.rs @@ -66,7 +66,7 @@ impl LogScroller { let can_fit = { // Subtract 1 for the title, and an additional TODO hacky // few to avoid the bottom OSD and stuff. - let n = (canvas.window_height / text::LINE_HEIGHT).floor() as isize - 1 - 6; + let n = (canvas.window_height / canvas.line_height).floor() as isize - 1 - 6; if n <= 0 { 0 } else { diff --git a/ezgui/src/menu.rs b/ezgui/src/menu.rs index f0435a3ce0..ebcd3bfe37 100644 --- a/ezgui/src/menu.rs +++ b/ezgui/src/menu.rs @@ -1,5 +1,4 @@ use crate::screen_geom::ScreenRectangle; -use crate::text::LINE_HEIGHT; use crate::{text, Canvas, Event, GfxCtx, InputResult, Key, ScreenPt, Text}; // Stores some associated data with each choice @@ -66,7 +65,7 @@ impl Menu { } else { total_width }; - ScreenPt::new(canvas.window_width - w, LINE_HEIGHT) + ScreenPt::new(canvas.window_width - w, canvas.line_height) } }; diff --git a/ezgui/src/runner.rs b/ezgui/src/runner.rs index 6d1c94de34..8f958f03cf 100644 --- a/ezgui/src/runner.rs +++ b/ezgui/src/runner.rs @@ -1,8 +1,9 @@ use crate::input::{ContextMenu, ModalMenuState}; -use crate::{Canvas, Event, GfxCtx, ModalMenu, TopMenu, UserInput}; +use crate::{text, Canvas, Event, GfxCtx, ModalMenu, TopMenu, UserInput}; use abstutil::Timer; use glium::glutin; use glium_glyph::glyph_brush::rusttype::Font; +use glium_glyph::glyph_brush::rusttype::Scale; use glium_glyph::GlyphBrush; use std::cell::RefCell; use std::io::Write; @@ -275,6 +276,10 @@ pub fn run>(mut gui: G, window_title: &str) { let dejavu: &[u8] = include_bytes!("DejaVuSans.ttf"); let fonts = vec![Font::from_bytes(dejavu).unwrap()]; + let vmetrics = fonts[0].v_metrics(Scale::uniform(text::FONT_SIZE)); + // TODO This works for this font, but could be more paranoid with abs() + gui.get_mut_canvas().line_height = + (vmetrics.ascent - vmetrics.descent + vmetrics.line_gap) as f64; gui.get_mut_canvas().glyphs = RefCell::new(Some(GlyphBrush::new(&display, fonts))); let mut state = State { diff --git a/ezgui/src/scrolling_menu.rs b/ezgui/src/scrolling_menu.rs index 7d56ca8c05..faa86ff44f 100644 --- a/ezgui/src/scrolling_menu.rs +++ b/ezgui/src/scrolling_menu.rs @@ -59,7 +59,7 @@ impl ScrollingMenu { let can_fit = { // Subtract 1 for the prompt, and an additional TODO hacky // few to avoid the bottom OSD and stuff. - let n = (canvas.window_height / text::LINE_HEIGHT).floor() as isize - 1 - 6; + let n = (canvas.window_height / canvas.line_height).floor() as isize - 1 - 6; if n <= 0 { // Weird small window, just display the prompt and bail out. canvas.draw_blocking_text(g, txt, CENTERED); diff --git a/ezgui/src/text.rs b/ezgui/src/text.rs index 5734a24fd7..222b5d3c4e 100644 --- a/ezgui/src/text.rs +++ b/ezgui/src/text.rs @@ -14,10 +14,8 @@ pub const SELECTED_COLOR: Color = Color::RED; pub const HOTKEY_COLOR: Color = Color::GREEN; pub const INACTIVE_CHOICE_COLOR: Color = Color::grey(0.4); -const FONT_SIZE: f32 = 24.0; -// TODO These are dependent on FONT_SIZE, but hand-tuned. Glyphs all have 0 as their height, and -// they need adjustments to their positioning. -pub const LINE_HEIGHT: f64 = 32.0; +pub const FONT_SIZE: f32 = 24.0; +// TODO Don't do this! const MAX_CHAR_WIDTH: f64 = 25.0; #[derive(Debug, Clone)] @@ -117,34 +115,36 @@ impl Text { self.lines.is_empty() } - pub(crate) fn dims(&self, glyphs: &mut GlyphBrush<'static, 'static>) -> (f64, f64) { - let mut widths: Vec = Vec::new(); - let mut total_height: i32 = 0; + pub(crate) fn dims( + &self, + glyphs: &mut GlyphBrush<'static, 'static>, + canvas: &Canvas, + ) -> (f64, f64) { + let width = self + .lines + .iter() + .map(|(_, l)| { + let full_line = l.iter().fold(String::new(), |mut so_far, span| { + so_far.push_str(&span.text); + so_far + }); + if let Some(rect) = glyphs.pixel_bounds(Section { + text: &full_line, + scale: Scale::uniform(FONT_SIZE), + ..Section::default() + }) { + rect.width() + } else { + // TODO Sometimes we want to space something like " ", but no drawn glyphs + // means pixel_bounds fails. Hack? + (MAX_CHAR_WIDTH * (full_line.len() as f64)) as i32 + } + }) + .max() + .unwrap() as f64; - for (_, l) in &self.lines { - let full_line = l.iter().fold(String::new(), |mut so_far, span| { - so_far.push_str(&span.text); - so_far - }); - if let Some(rect) = glyphs.pixel_bounds(Section { - text: &full_line, - scale: Scale::uniform(FONT_SIZE), - ..Section::default() - }) { - widths.push(rect.width()); - total_height += rect.height(); - } else { - // TODO Sometimes we want to space something like " ", but no drawn glyphs - // means pixel_bounds fails. Hack? - widths.push((MAX_CHAR_WIDTH * (full_line.len() as f64)) as i32); - total_height += LINE_HEIGHT as i32; - } - } - - ( - widths.into_iter().max().unwrap() as f64, - total_height as f64, - ) + // Always use the max height, since other stuff like menus assume a fixed height. + (width, (self.lines.len() as f64) * canvas.line_height) } } @@ -158,7 +158,7 @@ pub fn draw_text_bubble( // TODO Is it expensive to constantly change uniforms and the shader program? g.fork_screenspace(canvas); - let (total_width, total_height) = txt.dims(glyphs); + let (total_width, total_height) = txt.dims(glyphs, canvas); if let Some(c) = txt.bg_color { g.draw_polygon( c, @@ -185,16 +185,19 @@ pub fn draw_text_bubble( .collect(), ..VariedSection::default() }; - let height = glyphs.pixel_bounds(section.clone()).unwrap().height() as f64; if let Some(c) = line_color { g.draw_polygon( *c, - &Polygon::rectangle_topleft(Pt2D::new(top_left.x, y), total_width, height), + &Polygon::rectangle_topleft( + Pt2D::new(top_left.x, y), + total_width, + canvas.line_height, + ), ); } - y += height; + y += canvas.line_height; glyphs.queue(section); } diff --git a/ezgui/src/top_menu.rs b/ezgui/src/top_menu.rs index 783ddd1bbd..22abe5dfe4 100644 --- a/ezgui/src/top_menu.rs +++ b/ezgui/src/top_menu.rs @@ -1,7 +1,6 @@ use crate::menu::{Menu, Position}; use crate::screen_geom::ScreenRectangle; use crate::text; -use crate::TOP_MENU_HEIGHT; use crate::{Canvas, GfxCtx, InputResult, Key, ScreenPt, Text, UserInput}; use geom::{Polygon, Pt2D}; use std::collections::{HashMap, HashSet}; @@ -132,13 +131,17 @@ impl TopMenu { x1: 0.0, y1: 0.0, x2: canvas.window_width, - y2: TOP_MENU_HEIGHT, + y2: canvas.line_height, }); g.fork_screenspace(canvas); g.draw_polygon( text::BG_COLOR, - &Polygon::rectangle_topleft(Pt2D::new(0.0, 0.0), canvas.window_width, TOP_MENU_HEIGHT), + &Polygon::rectangle_topleft( + Pt2D::new(0.0, 0.0), + canvas.window_width, + canvas.line_height, + ), ); if let Some(idx) = self.highlighted {