From 824ae50de4254667108c2f7da8deaef835d4e32c Mon Sep 17 00:00:00 2001 From: Jonathan Kew Date: Fri, 27 Oct 2017 18:57:45 +0100 Subject: [PATCH] Bug 1395926 - Fix up handling of glyph offsets for text shaped in vertical mode. r=jrmuizel --- gfx/thebes/gfxHarfBuzzShaper.cpp | 43 +++++++++-------------- layout/reftests/writing-mode/reftest.list | 2 +- 2 files changed, 17 insertions(+), 28 deletions(-) diff --git a/gfx/thebes/gfxHarfBuzzShaper.cpp b/gfx/thebes/gfxHarfBuzzShaper.cpp index c070f04caa764..f9bb2be94c6bf 100644 --- a/gfx/thebes/gfxHarfBuzzShaper.cpp +++ b/gfx/thebes/gfxHarfBuzzShaper.cpp @@ -421,9 +421,6 @@ gfxHarfBuzzShaper::HBGetGlyphVOrigin(hb_font_t *font, void *font_data, const gfxHarfBuzzShaper::FontCallbackData *fcd = static_cast(font_data); fcd->mShaper->GetGlyphVOrigin(glyph, x, y); - // Negate the value we computed from font data (see comment re coordinate - // system orientation in HBGetGlyphVAdvance). - *y = -*y; return true; } @@ -431,7 +428,7 @@ void gfxHarfBuzzShaper::GetGlyphVOrigin(hb_codepoint_t aGlyph, hb_position_t *aX, hb_position_t *aY) const { - *aX = -0.5 * GetGlyphHAdvance(aGlyph); + *aX = 0.5 * GetGlyphHAdvance(aGlyph); if (mVORGTable) { // We checked in Initialize() that the VORG table is safely readable, @@ -452,11 +449,11 @@ gfxHarfBuzzShaper::GetGlyphVOrigin(hb_codepoint_t aGlyph, } if (lo < limit && uint16_t(lo->glyphIndex) == aGlyph) { - *aY = -FloatToFixed(GetFont()->FUnitsToDevUnitsFactor() * - int16_t(lo->vertOriginY)); + *aY = FloatToFixed(GetFont()->FUnitsToDevUnitsFactor() * + int16_t(lo->vertOriginY)); } else { - *aY = -FloatToFixed(GetFont()->FUnitsToDevUnitsFactor() * - int16_t(vorg->defaultVertOriginY)); + *aY = FloatToFixed(GetFont()->FUnitsToDevUnitsFactor() * + int16_t(vorg->defaultVertOriginY)); } return; } @@ -484,8 +481,8 @@ gfxHarfBuzzShaper::GetGlyphVOrigin(hb_codepoint_t aGlyph, (&metrics->metrics[mNumLongVMetrics]); lsb = int16_t(sidebearings[aGlyph - mNumLongVMetrics]); } - *aY = -FloatToFixed(mFont->FUnitsToDevUnitsFactor() * - (lsb + int16_t(glyf->yMax))); + *aY = FloatToFixed(mFont->FUnitsToDevUnitsFactor() * + (lsb + int16_t(glyf->yMax))); return; } else { // XXX TODO: not a truetype font; need to get glyph extents @@ -508,13 +505,13 @@ gfxHarfBuzzShaper::GetGlyphVOrigin(hb_codepoint_t aGlyph, // to ascender:descender from the hhea table int16_t a = int16_t(hhea->ascender); int16_t d = int16_t(hhea->descender); - *aY = -FloatToFixed(GetFont()->GetAdjustedSize() * a / (a - d)); + *aY = FloatToFixed(GetFont()->GetAdjustedSize() * a / (a - d)); return; } } NS_NOTREACHED("we shouldn't be here!"); - *aY = -FloatToFixed(GetFont()->GetAdjustedSize() / 2); + *aY = FloatToFixed(GetFont()->GetAdjustedSize() / 2); } static hb_bool_t @@ -1693,11 +1690,12 @@ gfxHarfBuzzShaper::SetGlyphsFromRun(gfxShapedText *aShapedText, hb_position_t i_offset, i_advance; // inline-direction offset/advance hb_position_t b_offset, b_advance; // block-direction offset/advance if (aVertical) { - // our inline coordinate direction is the opposite of harfbuzz's + // our coordinate directions are the opposite of harfbuzz's + // when doing top-to-bottom shaping i_offset = -posInfo[glyphStart].y_offset; i_advance = -posInfo[glyphStart].y_advance; - b_offset = posInfo[glyphStart].x_offset; - b_advance = posInfo[glyphStart].x_advance; + b_offset = -posInfo[glyphStart].x_offset; + b_advance = -posInfo[glyphStart].x_advance; } else { i_offset = posInfo[glyphStart].x_offset; i_advance = posInfo[glyphStart].x_advance; @@ -1733,14 +1731,6 @@ gfxHarfBuzzShaper::SetGlyphsFromRun(gfxShapedText *aShapedText, // there must be at least one in the clump, and we already measured // its advance, hence the placement of the loop-exit test and the // measurement of the next glyph. - // For vertical orientation, we add a "base offset" to compensate - // for the positioning within the cluster being based on horizontal - // glyph origin/offset. - hb_position_t baseIOffset, baseBOffset; - if (aVertical) { - baseIOffset = 2 * (i_offset - i_advance); - baseBOffset = GetGlyphHAdvance(ginfo[glyphStart].codepoint); - } while (1) { gfxTextRun::DetailedGlyph* details = detailedGlyphs.AppendElement(); @@ -1763,11 +1753,10 @@ gfxHarfBuzzShaper::SetGlyphsFromRun(gfxShapedText *aShapedText, } if (aVertical) { - // our inline coordinate direction is the opposite of HB's - i_offset = baseIOffset + posInfo[glyphStart].y_offset; + i_offset = -posInfo[glyphStart].y_offset; i_advance = -posInfo[glyphStart].y_advance; - b_offset = baseBOffset - posInfo[glyphStart].x_offset; - b_advance = posInfo[glyphStart].x_advance; + b_offset = -posInfo[glyphStart].x_offset; + b_advance = -posInfo[glyphStart].x_advance; } else { i_offset = posInfo[glyphStart].x_offset; i_advance = posInfo[glyphStart].x_advance; diff --git a/layout/reftests/writing-mode/reftest.list b/layout/reftests/writing-mode/reftest.list index abbab34dcd8a2..24429d01d88f5 100644 --- a/layout/reftests/writing-mode/reftest.list +++ b/layout/reftests/writing-mode/reftest.list @@ -179,7 +179,7 @@ fuzzy-if(gtkWidget,1,25) fuzzy-if(cocoaWidget,1,2) == 1302389-scrolled-rect-2b.h fuzzy-if(Android,54,852) == 1302389-scrolled-rect-2d.html 1302389-scrolled-rect-2-ref.html == 1361631-mongolian-upright-1.html 1361631-mongolian-upright-1-ref.html -fails == 1395926-vertical-upright-gpos-1.html 1395926-vertical-upright-gpos-1-ref.html +== 1395926-vertical-upright-gpos-1.html 1395926-vertical-upright-gpos-1-ref.html # Suite of tests from Gérard Talbot in bug 1079151 # Frequent Windows 7 load failed: timed out waiting for test to complete (waiting for onload scripts to complete), bug 1167155 and friends