mirror of
https://github.com/storytold/effectcraft.git
synced 2026-10-30 13:08:07 -06:00
Review: short "d" falls back to the font ascent; tracking unit comment; Lottie hunk out
- first_ascent: a "d" shorter than 0.3 x the font ascent (a symbol or icon font) no longer puts box text's first baseline almost at the box top; the font ascent is used, as before the "d" rule (CodeRabbit on #1875). Test: a_short_d_falls_back_to_the_font_ascent. - CharXf::track_before / track_after are pixels, not 1/1000 em (CodeRabbit). - The Lottie export of style-run tracking is back to what it was: the pixel conversion was not checked against a player, so it is left for a separate, verified change. - The Subtract clamp in Mode::combine is intended for every animator property: the selectors combine into one selection per character for the whole animator, as in After Effects; the read-back observed it through Tracking (now said in the code). - Per-character 3D rotation notes that #1876 adds Mat4::rotation_xyz for the same order. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
8b64074ce3
commit
daedbd5ca1
@@ -227,8 +227,8 @@ fn style_run_animators(ex: &mut Ex, d: &TextDoc, label: &str) -> Vec<Json> {
|
||||
props.insert("s".into(), k(json!([p, p, 100.0])));
|
||||
}
|
||||
if (s.tracking - base.tracking).abs() > 1e-9 {
|
||||
// An animator's tracking is in pixels; the Character panel's in 1/1000 em.
|
||||
props.insert("t".into(), k(json!((s.tracking - base.tracking) / 1000.0 * s.size)));
|
||||
// Lottie tracking is in 1/1000 em like the Character panel's.
|
||||
props.insert("t".into(), k(json!(s.tracking - base.tracking)));
|
||||
}
|
||||
if (s.baseline_shift - base.baseline_shift).abs() > 1e-9 {
|
||||
props.insert("p".into(), k(json!([0.0, -(s.baseline_shift - base.baseline_shift), 0.0])));
|
||||
|
||||
@@ -42,7 +42,8 @@ pub struct CharXf {
|
||||
pub stroke_hsb: [f64; 3],
|
||||
pub stroke_opacity: f64,
|
||||
pub stroke_width: f64,
|
||||
/// Tracking (1/1000 em) added before / after the character.
|
||||
/// Tracking (pixels: an animator's Tracking Amount, as After Effects) added before / after
|
||||
/// the character.
|
||||
pub track_before: f64,
|
||||
pub track_after: f64,
|
||||
/// Line Anchor (percent) and the selection weight it was set with.
|
||||
@@ -752,6 +753,9 @@ pub fn text_geom(ctx: &EvalCtx, layer: &Layer) -> Option<TextGeom> {
|
||||
};
|
||||
// Outline relative to the pivot.
|
||||
let local = kurbo::Affine::translate((o[0] - pivot[0], o[1] - pivot[1])) * outline;
|
||||
// Rx · Ry · Rz (Z turns first): the order After Effects gives a 3D layer's X / Y / Z
|
||||
// Rotation too. #1876 moves layers to it with a helper, `Mat4::rotation_xyz`; once both
|
||||
// land, this should call that helper (one convention, one place).
|
||||
let char_m = Mat4::rotate_x(x.rotation_x)
|
||||
* Mat4::rotate_y(x.rotation_y)
|
||||
* Mat4::rotate_z(x.rotation)
|
||||
|
||||
@@ -577,8 +577,20 @@ fn first_ascent(face: FaceId, px: f32) -> Option<f32> {
|
||||
let units = crate::outline_units(face, gid)?;
|
||||
let top = kurbo::Shape::bounding_box(&*units).y1 as f32;
|
||||
let em = (top / f.units_per_em() * 1000.0).round() / 1000.0;
|
||||
plausible_first(em, px, f.metrics(px).ascent)
|
||||
}
|
||||
|
||||
/// A `d` at least this fraction of the font ascent is a Latin lowercase ascender; a shorter one
|
||||
/// (a symbol or icon font's `d`) would put the first baseline almost at the box top, so the font
|
||||
/// ascent is used instead (the rule before the `d` height).
|
||||
const MIN_D_OF_ASCENT: f32 = 0.3;
|
||||
|
||||
/// The first baseline `em * px` below the box top, if `em` (the `d` height in em) is plausible:
|
||||
/// finite, positive, under 4 em and at least [`MIN_D_OF_ASCENT`] of the font `ascent` (px).
|
||||
fn plausible_first(em: f32, px: f32, ascent: f32) -> Option<f32> {
|
||||
let h = em * px;
|
||||
(h.is_finite() && h > 0.0 && em < 4.0).then_some(h)
|
||||
let tall_enough = !(ascent.is_finite() && ascent > 0.0) || h >= MIN_D_OF_ASCENT * ascent;
|
||||
(h.is_finite() && h > 0.0 && em < 4.0 && tall_enough).then_some(h)
|
||||
}
|
||||
|
||||
struct ParaLine {
|
||||
@@ -1284,6 +1296,29 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// CodeRabbit on #1875: a face whose `d` is very short (a symbol or icon font) would put the
|
||||
/// first baseline almost at the box top; below 0.3 of the ascent the font ascent is used.
|
||||
#[test]
|
||||
fn a_short_d_falls_back_to_the_font_ascent() {
|
||||
// Latin faces: the `d` is about 0.7 em, the ascent about 0.9 em.
|
||||
let near = |v: Option<f32>, want: f32| v.is_some_and(|v| (v - want).abs() < 1e-3);
|
||||
assert!(near(plausible_first(0.7, 100.0, 90.0), 70.0));
|
||||
// A `d` a tenth of the ascent: not a lowercase ascender.
|
||||
assert_eq!(plausible_first(0.09, 100.0, 90.0), None);
|
||||
assert_eq!(plausible_first(0.26, 100.0, 90.0), None);
|
||||
assert!(near(plausible_first(0.28, 100.0, 90.0), 28.0));
|
||||
// Broken values stay rejected; an unusable ascent does not reject a sane `d`.
|
||||
for (em, px) in [(0.0, 100.0), (-0.5, 100.0), (f32::NAN, 100.0), (5.0, 100.0), (0.7, f32::INFINITY)] {
|
||||
assert_eq!(plausible_first(em, px, 90.0), None, "{em} {px}");
|
||||
}
|
||||
assert!(near(plausible_first(0.7, 100.0, f32::NAN), 70.0));
|
||||
// The bundled faces keep the `d` rule.
|
||||
for family in ["Inter", "Noto Serif"] {
|
||||
let r = fonts::resolve(family, "Regular");
|
||||
assert!(first_ascent(r.face, 100.0).is_some(), "{family}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn newlines_make_paragraphs_and_leading_adds() {
|
||||
let l = layout("one\ntwo\n\nfour", &st(30.0), &ParagraphStyle::default());
|
||||
|
||||
@@ -74,6 +74,11 @@ impl Mode {
|
||||
// Subtracting more than is selected leaves nothing, not a negative selection (After
|
||||
// Effects 2026: a Subtract selector over unselected characters leaves their
|
||||
// Tracking at 0, EXPORTER read-back t8).
|
||||
// Intended for every animator property, not only Tracking: selectors combine into one
|
||||
// selection per character for the whole animator (`render::text::animator_selection`),
|
||||
// which then scales each of its properties, as in After Effects, where selectors
|
||||
// belong to the animator. The read-back observed that selection through Tracking;
|
||||
// Opacity, Position and the rest were not measured separately.
|
||||
Mode::Subtract if acc >= 0.0 => (acc - v).max(0.0),
|
||||
Mode::Subtract => acc - v,
|
||||
Mode::Intersect => acc * v,
|
||||
|
||||
Reference in New Issue
Block a user