diff --git a/crates/render/src/table.rs b/crates/render/src/table.rs index dd2630f63..ce6f9a864 100644 --- a/crates/render/src/table.rs +++ b/crates/render/src/table.rs @@ -153,18 +153,28 @@ pub fn look(ctx: &Ctx, t: &Table, r: usize, c: usize) -> CellLook { let band_c = t.band_col && c.is_multiple_of(2); let tint = |k: i32| deckcraft_color::apply(a, &[ColorTransform::Tint(k)]); let white = Rgba::WHITE; - let dark_text = ctx.color(&ColorRef::scheme(SchemeSlot::Tx1), None); + let text_color = ctx.color(&ColorRef::scheme(SchemeSlot::Tx1), None); + // Text on a tinted cell never inherits the text colour, which a dark theme makes light and + // unreadable on the light tint: PowerPoint's medium styles set it to the theme's dark colour + // (dk1). A dark theme may keep that in dk1 and map tx1 away from it, or (Nocturne) put it in + // lt1, so of the two the one contrasting more with the cell's fill is used. + let dk1 = ctx.color(&ColorRef::scheme(SchemeSlot::Dk1), None); + let lt1 = ctx.color(&ColorRef::scheme(SchemeSlot::Lt1), None); + let on = |fill: Rgba| if dk1.contrast(fill) >= lt1.contrast(fill) { dk1 } else { lt1 }; match kind { "none" => CellLook { fill: None, text: None, bold: false, border: None, sides: [None; 4], paint: None }, - "grid" => CellLook { fill: None, text: None, bold: false, border: Some((dark_text, 0.75)), sides: [None; 4], paint: None }, - "light1" => CellLook { - fill: if band || band_c { Some(tint(20000)) } else { None }, - text: None, - bold: header || total || first_col || last_col, - border: if header { Some((a, 1.0)) } else { None }, - sides: [None; 4], - paint: None, - }, + "grid" => CellLook { fill: None, text: None, bold: false, border: Some((text_color, 0.75)), sides: [None; 4], paint: None }, + "light1" => { + let fill = (band || band_c).then(|| tint(20000)); + CellLook { + fill, + text: fill.map(on), + bold: header || total || first_col || last_col, + border: if header { Some((a, 1.0)) } else { None }, + sides: [None; 4], + paint: None, + } + } "light2" => CellLook { fill: if header { Some(a) } else { None }, text: if header { Some(white) } else { None }, @@ -187,29 +197,36 @@ pub fn look(ctx: &Ctx, t: &Table, r: usize, c: usize) -> CellLook { sides: [None; 4], paint: None, }, - "medium4" => CellLook { - fill: Some(if header || band || band_c { tint(40000) } else { tint(20000) }), - text: None, - bold: header || total || first_col || last_col, - border: Some((a, 0.75)), - sides: [None; 4], - paint: None, - }, - _ => CellLook { + "medium4" => { + let fill = if header || band || band_c { tint(40000) } else { tint(20000) }; + CellLook { + fill: Some(fill), + text: Some(on(fill)), + bold: header || total || first_col || last_col, + border: Some((a, 0.75)), + sides: [None; 4], + paint: None, + } + } + _ => { // medium2: strong header, banded light tints, white grid. - fill: Some(if header || total || first_col || last_col { + let strong = header || total || first_col || last_col; + let fill = if strong { a } else if band || band_c { tint(40000) } else { tint(20000) - }), - text: if header || total || first_col || last_col { Some(white) } else { None }, - bold: header || total || first_col || last_col, - border: Some((white, 1.0)), - sides: [None; 4], - paint: None, - }, + }; + CellLook { + fill: Some(fill), + text: Some(if strong { white } else { on(fill) }), + bold: strong, + border: Some((white, 1.0)), + sides: [None; 4], + paint: None, + } + } } } diff --git a/crates/render/src/tests.rs b/crates/render/src/tests.rs index b65098b0e..871bbccb3 100644 --- a/crates/render/src/tests.rs +++ b/crates/render/src/tests.rs @@ -1336,6 +1336,41 @@ fn a_table_fill_shows_behind_cells_without_their_own() { assert_eq!(img.pixel(50, 50)[..3], [255, 255, 255]); } +#[test] +fn table_body_text_is_dark_on_the_tinted_cells_of_a_dark_theme() { + // The default style (medium2) tints body cells towards white, so their text must be the + // theme's dark colour, as PowerPoint's medium styles set it (dk1): inheriting the text colour + // leaves it light, and unreadable, on a dark theme such as Nocturne. + let fill_and_darkest = |theme: &str| { + let mut t = Table::new(2, 2, 200.0, 40.0); + for (r, c) in [(0, 0), (0, 1), (1, 0), (1, 1)] { + t.cell_mut(r, c).unwrap().text = TextBody::from_text("Body"); + } + let tbl = Shape { id: ShapeId(796), xfrm: Some(Xfrm::new(100.0, 100.0, 200.0, 80.0)), kind: ShapeKind::Table(t), ..Default::default() }; + let mut p = deck_with(vec![tbl]); + let th = deckcraft_model::theme::builtin_themes().into_iter().find(|th| th.name == theme).unwrap(); + std::sync::Arc::make_mut(&mut p.masters[0]).theme = th; + let img = render_slide(&p, 0, &RenderOpts { scale: 1.0, ..Default::default() }); + let lum = |x, y| { + let px = img.pixel(x, y); + deckcraft_color::Rgba::rgb(px[0], px[1], px[2]).luminance() + }; + // Body cell (1, 1) covers (200, 140)–(300, 180); its top-left corner is clear of the text + // (cell margins) and of the white grid line. + let fill = lum(203, 143); + let darkest = (143..177).flat_map(|y| (203..297).map(move |x| (x, y))).map(|(x, y)| lum(x, y)).fold(1.0_f64, f64::min); + (fill, darkest) + }; + for theme in ["Nocturne", "Harbor"] { + let (fill, darkest) = fill_and_darkest(theme); + assert!(fill > 0.4, "{theme}: the body cell's fill is a light tint, luminance {fill}"); + assert!( + darkest < 0.15 && darkest < fill / 3.0, + "{theme}: darkest pixel in the body cell has luminance {darkest} on a fill of {fill}: its text is not dark" + ); + } +} + /// A white 280x200 pt rectangle at (20, 20) with `effects`, rendered at 1 px/pt. fn effect_box(effects: deckcraft_model::Effects) -> Image { use deckcraft_model::style::{Fill, Line};