AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Revert "[skottie] Visual-only text valign"

Browse Source 
This reverts commit dcafc5d2bc.

Reason for revert: too disruptive for existing g3 asssets

Original change's description:
> [skottie] Visual-only text valign
>
> Historically, Skottie started with vertical alignment based on the
> typographic bounding box.  This was meant to account for empty
> leading/trailing lines.
>
> At some point [1], the strategy was changed to also take the visual
> bounding box into account (union of typographic and visual bounds).
>
> It turns out this is still suboptimal: aligning based on font metrics
> yields poor results in practice, and pretty much everyone expects
> visual-only alignment.
>
> This CL is an attempt to fix things:
>
> 1) update kVisualTop/kVisualCenter/kVisualBottom to use visual bounds
>    only (as their name implies)
> 2) introduce kDeprecatedVisualCenter to preserves the old behavior
>    for compatibility, and use it for the legacy sk_vj flags
>
> The latter is done to minimize disruption for clients which have
> adjusted for the current misalignment: luckily they're mostly using the
> old sk_vj flag instead of explicit resize/valign policies, and they can
> continue to do so without change, while new clients can opt into the
> new/improved valign modes.
>
> The change is guarded by a build flag for g3 staging.
>
> [1] https://skia-review.googlesource.com/c/skia/+/224188
>
> Change-Id: I334c1713ce32635e3649711f072a3dcdf6b12244
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/501016
> Reviewed-by: Ben Wagner <bungeman@google.com>
> Reviewed-by: Jorge Betancourt <jmbetancourt@google.com>
> Commit-Queue: Florin Malita <fmalita@google.com>

Change-Id: I633dd54cd04727617e845d24a35e5e8cef64f861
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/506177
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Florin Malita <fmalita@google.com>
This commit is contained in:
Florin Malita 2022-02-09 13:59:07 +00:00 committed by SkCQ
parent 3e6e29cf2e
commit e5d7a8ec89
3 changed files with 29 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
modules/skottie/src/text/SkottieShaper.cpp
View File

@ -146,24 +146,27 @@ public:
const auto ascent = this->ascent();
auto extent_box = [&](bool include_typographical_extent = false) {
// For visual VAlign modes, we use a hybrid extent box computed as the union of
// actual visual bounds and the vertical typographical extent.
//
// This ensures that
//
// a) text doesn't visually overflow the alignment boundaries
//
// b) leading/trailing empty lines are still taken into account for alignment purposes
auto extent_box = [&]() {
auto box = fResult.computeVisualBounds();
#ifdef SKOTTIE_DEPRECATED_VERTICAL_ALIGNMENT
include_typographical_extent = true;
#endif
// Deprecated visual alignment mode, based on typographical extent.
if (include_typographical_extent) {
// By default, first line is vertically-aligned on a baseline of 0.
// The typographical height considered for vertical alignment is the distance
// between the first line top (ascent) to the last line bottom (descent).
const auto typographical_top = fBox.fTop + ascent,
typographical_bottom = fBox.fTop + fLastLineDescent +
fDesc.fLineHeight*(fLineCount > 0 ? fLineCount - 1 : 0ul);
// By default, first line is vertically-aligned on a baseline of 0.
// The typographical height considered for vertical alignment is the distance between
// the first line top (ascent) to the last line bottom (descent).
const auto typographical_top = fBox.fTop + ascent,
typographical_bottom = fBox.fTop + fLastLineDescent + fDesc.fLineHeight *
(fLineCount > 0 ? fLineCount - 1 : 0ul);
box.fTop = std::min(box.fTop, typographical_top);
box.fBottom = std::max(box.fBottom, typographical_bottom);
}
box.fTop = std::min(box.fTop, typographical_top);
box.fBottom = std::max(box.fBottom, typographical_bottom);
return box;
};
@ -186,8 +189,7 @@ public:
v_offset += fBox.fTop - ebox->fTop;
break;
case Shaper::VAlign::kVisualCenter:
case Shaper::VAlign::kDeprecatedVisualCenter:
ebox.init(extent_box(fDesc.fVAlign == Shaper::VAlign::kDeprecatedVisualCenter));
ebox.init(extent_box());
v_offset += fBox.centerY() - ebox->centerY();
break;
case Shaper::VAlign::kVisualBottom:

17
modules/skottie/src/text/SkottieShaper.h
View File

@ -47,14 +47,7 @@ public:
// Align the first line typographical baseline with the text box top (AE point text).
kTopBaseline,
// Skottie vertical alignment extensions:
kVisualTop, // visual top -> text box top
kVisualCenter, // visual center -> text box center
kVisualBottom, // visual bottom -> text box bottom
// Deprecated/historical alignment mode. Do not use.
//
// Based on an extent box defined (in Y) as
// Skottie vertical alignment extensions: these are based on an extent box defined (in Y) as
//
// ------------------------------------------------------
// MIN(visual_top_extent , typographical_top_extent )
@ -63,7 +56,13 @@ public:
//
// MAX(visual_bottom_extent, typographical_bottom_extent)
// ------------------------------------------------------
kDeprecatedVisualCenter, // extent box center -> text box center
// extent box top -> text box top
kVisualTop,
// extent box center -> text box center
kVisualCenter,
// extent box bottom -> text box bottom
kVisualBottom,
};
enum class ResizePolicy : uint8_t {

4
modules/skottie/src/text/TextValue.cpp
View File

@ -121,12 +121,12 @@ bool Parse(const skjson::Value& jv, const internal::AnimationBuilder& abuilder,
switch (vj) {
case 3:
// 'sk_vj': 3 -> kVisualCenter/kScaleToFit
v->fVAlign = Shaper::VAlign::kDeprecatedVisualCenter;
v->fVAlign = Shaper::VAlign::kVisualCenter;
v->fResize = Shaper::ResizePolicy::kScaleToFit;
break;
case 4:
// 'sk_vj': 4 -> kVisualCenter/kDownscaleToFit
v->fVAlign = Shaper::VAlign::kDeprecatedVisualCenter;
v->fVAlign = Shaper::VAlign::kVisualCenter;
v->fResize = Shaper::ResizePolicy::kDownscaleToFit;
break;
default: