[Webkit-unassigned] [Bug 46973] Need to swap glyphs for vertical writing

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=46973


mitz at webkit.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #70585|review?                     |review-
               Flag|                            |




--- Comment #11 from mitz at webkit.org  2010-10-13 11:02:04 PST ---
(From update of attachment 70585)
View in context: https://bugs.webkit.org/attachment.cgi?id=70585&action=review

> WebCore/platform/graphics/FontCache.cpp:85
> +    bool m_isVerticalLayout;

m_isVerticalLayout is a bad name for this instance variable and for the predicates. Please add an enum { Horizontal, Vertical } FontOrientation and use it.

> WebCore/platform/graphics/TypesettingFeatures.h:33
> +        VerticalLayout = 1 << 2,

This is a property of the font, so it doesn’t need to be a typesetting feature.

> WebCore/platform/graphics/mac/GlyphPageTreeNodeMac.cpp:63
> +        static const int ligaturesNotAllowedValue = 0;
> +        static CFNumberRef ligaturesNotAllowed = CFNumberCreate(kCFAllocatorDefault, kCFNumberIntType, &ligaturesNotAllowedValue);
> +        static const void* keys[] = {   kCTFontAttributeName,               kCTLigatureAttributeName,   kCTVerticalFormsAttributeName };
> +        const void* values[] = {        fontData->platformData().ctFont(),  ligaturesNotAllowed,        kCFBooleanTrue };
> +        RetainPtr<CFDictionaryRef> attributeDictionary(AdoptCF, CFDictionaryCreate(kCFAllocatorDefault, keys, values, sizeof(keys) / sizeof(*keys),
> +                                                                &kCFCopyStringDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks));
> +
> +        RetainPtr<CFStringRef> fontName(AdoptCF, CTFontCopyPostScriptName(fontData->platformData().ctFont()));

This should just use SimpleFontData::getCFStringAttributes().

> WebCore/platform/graphics/mac/GlyphPageTreeNodeMac.cpp:85
> +            RetainPtr<CFStringRef> runFontName(AdoptCF, CTFontCopyPostScriptName(runFont));
> +            if (CFEqual(fontName.get(), runFontName.get())) {

This is a strange way to compare fonts.

> WebCore/platform/graphics/mac/SimpleFontDataCoreText.cpp:81
> -        static const void* keysWithKerningDisabled[] = { kCTFontAttributeName, kCTKernAttributeName, kCTLigatureAttributeName };
> -        const void* valuesWithKerningDisabled[] = { platformData().ctFont(), kerningAdjustment, allowLigatures
> -            ? ligaturesAllowed : ligaturesNotAllowed };
> +        static const void* keysWithKerningDisabled[] = {    kCTFontAttributeName,
> +                                                            kCTKernAttributeName,
> +                                                            kCTLigatureAttributeName,
> +                                                            kCTVerticalFormsAttributeName };
> +        const void* valuesWithKerningDisabled[] = {         platformData().ctFont(),
> +                                                            kerningAdjustment,
> +                                                            allowLigatures ? ligaturesAllowed : ligaturesNotAllowed,
> +                                                            (typesettingFeatures & VerticalLayout) ? kCFBooleanTrue : kCFBooleanFalse };
>          attributesDictionary.adoptCF(CFDictionaryCreate(0, keysWithKerningDisabled, valuesWithKerningDisabled,
>              sizeof(keysWithKerningDisabled) / sizeof(*keysWithKerningDisabled),
>              &kCFCopyStringDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks));
>      } else {
>          // By omitting the kCTKernAttributeName attribute, we get Core Text's standard kerning.
> -        static const void* keysWithKerningEnabled[] = { kCTFontAttributeName, kCTLigatureAttributeName };
> -        const void* valuesWithKerningEnabled[] = { platformData().ctFont(), allowLigatures ? ligaturesAllowed : ligaturesNotAllowed };
> +        static const void* keysWithKerningEnabled[] = {     kCTFontAttributeName,
> +                                                            kCTLigatureAttributeName,
> +                                                            kCTVerticalFormsAttributeName };
> +        const void* valuesWithKerningEnabled[] = {          platformData().ctFont(),
> +                                                            allowLigatures ? ligaturesAllowed : ligaturesNotAllowed,
> +                                                            (typesettingFeatures & VerticalLayout) ? kCFBooleanTrue : kCFBooleanFalse };

Instead of making this change, you should change FontPlatformData::ctFont() to return a font with kCTFontOrientationAttribute set to kCTFontVerticalOrientation.

> WebCore/platform/graphics/mac/SimpleFontDataMac.mm:440
>  {
> -    NSFont* font = platformData().font();
> -    float pointSize = platformData().m_size;
> -    CGAffineTransform m = CGAffineTransformMakeScale(pointSize, pointSize);
>      CGSize advance;
> -    if (!wkGetGlyphTransformedAdvances(platformData().cgFont(), font, &m, &glyph, &advance)) {
> -        LOG_ERROR("Unable to cache glyph widths for %@ %f", [font displayName], pointSize);
> -        advance.width = 0;
> -    }
> +    CTFontGetAdvancesForGlyphs(m_platformData.ctFont(),
> +                                m_platformData.isVerticalLayout() ? kCTFontVerticalOrientation : kCTFontHorizontalOrientation, &glyph, &advance, 1);

For horizontal orientation, this should keep using wkGetGlyphTransformedAdvances, which takes the font’s rendering mode into account.

> WebCore/rendering/style/RenderStyle.cpp:720
> +void RenderStyle::setWritingMode(WritingMode v)
> +{
> +    inherited_flags.m_writingMode = v;
> +
> +    FontSelector* currentFontSelector = font().fontSelector();
> +    FontDescription desc(fontDescription());
> +    desc.setIsVerticalLayout(!isHorizontalWritingMode());
> +    setFontDescription(desc);
> +    font().update(currentFontSelector);
> +}
> +

This should be done in CSSStyleSelector using the m_fontDirty bit rather than unconditionally updating the font here.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.