Allow small-caps synthesis for @font-face with multiple sources including SVG format

Allow small-caps synthesis for @font-face with multiple sources including SVG format

Since small-caps synthesis is not supported for SVG fonts, our font
matching algorithm ignored @font-faces that have svg sources when
small-caps variant is requested. This behavior had a problem that it
discards @font-face even if non-svg (ex. woff) source is available.

This patch moves that SVG-font check to CSSSegmentedFontFace::getFontData()
so that it skips SVG fonts only when SVG source is used.

BUG=298970
TEST=fast/css,svg/*

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=159429

[eseidel, 2013-10-10 14:41:57]

Lgtm
On Oct 10, 2013 2:11 AM, <ksakamoto@chromium.org> wrote:

> Reviewers: Dimitri Glazkov, eae, eseidel,
>
> Description:
> Allow small-caps synthesis for @font-face with multiple sources including
> SVG
> format
>
> Since small-caps synthesis is not supported for SVG fonts, our font
> matching algorithm ignored @font-faces that have svg sources when
> small-caps variant is requested. This behavior had a problem that it
> discards @font-face even if non-svg (ex. woff) source is available.
>
> This patch moves that SVG-font check to CSSSegmentedFontFace::**
> getFontData()
> so that it skips SVG fonts only when SVG source is used.
>
> BUG=298970
> TEST=fast/css,svg/*
>
>
> Please review this at
https://codereview.chromium.**org/26702008/<https://codereview.chromium.org/2...
>
> SVN Base:
svn://svn.chromium.org/blink/**trunk<http://svn.chromium.org/blink/trunk>
>
> Affected files (+28, -39 lines):
>   A + LayoutTests/fast/css/font-**face-small-caps-with-svg-**
> source-expected.html
>   A LayoutTests/fast/css/font-**face-small-caps-with-svg-**source.html
>   M Source/core/css/CSSFontFace.h
>   M Source/core/css/CSSFontFace.**cpp
>   M Source/core/css/**CSSSegmentedFontFace.h
>   M Source/core/css/**CSSSegmentedFontFace.cpp
>   M Source/core/css/**CSSSegmentedFontFaceCache.cpp
>
>
> Index: LayoutTests/fast/css/font-**face-small-caps-with-svg-**
> source-expected.html
> diff --git a/LayoutTests/fast/css/font-**face-variant-expected.html
> b/LayoutTests/fast/css/font-**face-small-caps-with-svg-**
> source-expected.html
> similarity index 52%
> copy from LayoutTests/fast/css/font-**face-variant-expected.html
> copy to LayoutTests/fast/css/font-**face-small-caps-with-svg-**
> source-expected.html
> index 6aba2816c32dabfb096f8463e33bfb**29aa69d432..**
> c954c7a75f8d700fc89ff6c05e827f**ce8774e62d 100644
> --- a/LayoutTests/fast/css/font-**face-variant-expected.html
> +++ b/LayoutTests/fast/css/font-**face-small-caps-with-svg-**
> source-expected.html
> @@ -1,8 +1,8 @@
>  <style>
>      @font-face {
>          font-family: family1;
> -        src: local(Ahem), url(../../resources/Ahem.ttf);
> -        font-variant: small-caps;
> +        src: local(Ahem),
> +             url(../../resources/Ahem.ttf) format(truetype);
>      }
>      p.test {
>          font-family: family1;
> @@ -10,7 +10,7 @@
>      }
>  </style>
>  <p>
> -Tests that if @font-face does not provide an explicit font-variant,
> normal is used.
> +Test for <a href="http://crbug.com/298970"**>bug 298970</a>. Verifies
> that small-caps synthesis works when @font-face has a svg source.
>  </p>
>  <p>
>  The text below should be a series of black boxes.
> Index: LayoutTests/fast/css/font-**face-small-caps-with-svg-**source.html
> diff --git
a/LayoutTests/fast/css/font-**face-small-caps-with-svg-**source.html
> b/LayoutTests/fast/css/font-**face-small-caps-with-svg-**source.html
> new file mode 100644
> index 000000000000000000000000000000**0000000000..**
> d0a5fbe35e6698fcd57aa6cdb9870d**b1e3cc352a
> --- /dev/null
> +++ b/LayoutTests/fast/css/font-**face-small-caps-with-svg-**source.html
> @@ -0,0 +1,19 @@
> +<style>
> +    @font-face {
> +        font-family: family1;
> +        src: local(Ahem),
> +             url(../../resources/Ahem.ttf) format(truetype),
> +             url(../../resources/**nonexistent.svg) format(svg);
> +    }
> +    p.test {
> +        font-family: family1;
> +        font-variant: small-caps;
> +    }
> +</style>
> +<p>
> +Test for <a href="http://crbug.com/298970"**>bug 298970</a>. Verifies
> that small-caps synthesis works when @font-face has a svg source.
> +</p>
> +<p>
> +The text below should be a series of black boxes.
> +</p>
> +<p class="test">failure</p>
> Index: Source/core/css/CSSFontFace.**cpp
> diff --git a/Source/core/css/CSSFontFace.**cpp
> b/Source/core/css/CSSFontFace.**cpp
> index 82aa441190eedc5534a0d1c6abcd59**3e6dbcb565..**
> 1c06a46ddd179d2705ebe7647aacb8**d3838c3b52 100644
> --- a/Source/core/css/CSSFontFace.**cpp
> +++ b/Source/core/css/CSSFontFace.**cpp
> @@ -177,18 +177,6 @@ void CSSFontFace::setLoadStatus(**FontFace::LoadStatus
> newStatus)
>      }
>  }
>
> -#if ENABLE(SVG_FONTS)
> -bool CSSFontFace::**hasSVGFontFaceSource() const
> -{
> -    size_t size = m_sources.size();
> -    for (size_t i = 0; i < size; i++) {
> -        if (m_sources[i]->**isSVGFontFaceSource())
> -            return true;
> -    }
> -    return false;
> -}
> -#endif
> -
>  bool CSSFontFace::UnicodeRangeSet::**intersectsWith(const String& text)
> const
>  {
>      if (text.isEmpty())
> Index: Source/core/css/CSSFontFace.h
> diff --git a/Source/core/css/CSSFontFace.**h
> b/Source/core/css/CSSFontFace.**h
> index 136064ae434d1dc8a64403a0c32939**720a25b980..**
> b547667e8a2558f9397d7e13b03904**f785b73b22 100644
> --- a/Source/core/css/CSSFontFace.**h
> +++ b/Source/core/css/CSSFontFace.**h
> @@ -90,10 +90,6 @@ public:
>          Vector<UnicodeRange> m_ranges;
>      };
>
> -#if ENABLE(SVG_FONTS)
> -    bool hasSVGFontFaceSource() const;
> -#endif
> -
>      FontFace::LoadStatus loadStatus() const { return m_fontFace ?
> m_fontFace->loadStatus() : FontFace::Loaded; }
>      void willUseFontData(const FontDescription&);
>
> Index: Source/core/css/**CSSSegmentedFontFace.cpp
> diff --git a/Source/core/css/**CSSSegmentedFontFace.cpp b/Source/core/css/
> **CSSSegmentedFontFace.cpp
> index 46b7ed8a10babd1abfed2d245d6bab**d79f0fdab1..**
> 96c553fdbd33fe0deb17e70128670c**671f0e15f2 100644
> --- a/Source/core/css/**CSSSegmentedFontFace.cpp
> +++ b/Source/core/css/**CSSSegmentedFontFace.cpp
> @@ -138,6 +138,12 @@ PassRefPtr<FontData>
CSSSegmentedFontFace::**getFontData(const
> FontDescription& fo
>              continue;
>          if (RefPtr<SimpleFontData> faceFontData =
> m_fontFaces[i]->getFontData(**fontDescription, syntheticBold,
> syntheticItalic)) {
>              ASSERT(!faceFontData->**isSegmented());
> +#if ENABLE(SVG_FONTS)
> +            // For SVG Fonts that specify that they only support the
> "normal" variant, we will assume they are incapable
> +            // of small-caps synthesis and just ignore the font face.
> +            if (faceFontData->isSVGFont() && (desiredTraitsMask &
> FontVariantSmallCapsMask) && !(m_traitsMask & FontVariantSmallCapsMask))
> +                continue;
> +#endif
>              appendFontData(fontData.get(), faceFontData.release(),
> m_fontFaces[i]->ranges());
>          }
>      }
> @@ -147,16 +153,6 @@ PassRefPtr<FontData>
CSSSegmentedFontFace::**getFontData(const
> FontDescription& fo
>      return 0;
>  }
>
> -bool CSSSegmentedFontFace::**hasSVGFontFaceSource() const
> -{
> -    unsigned size = m_fontFaces.size();
> -    for (unsigned i = 0; i < size; i++) {
> -        if (m_fontFaces[i]->**hasSVGFontFaceSource())
> -            return true;
> -    }
> -    return false;
> -}
> -
>  bool CSSSegmentedFontFace::**isLoading() const
>  {
>      unsigned size = m_fontFaces.size();
> Index: Source/core/css/**CSSSegmentedFontFace.h
> diff --git a/Source/core/css/**CSSSegmentedFontFace.h b/Source/core/css/**
> CSSSegmentedFontFace.h
> index a930fea7a17bf37bda8bda63fa79fa**6c1c3285f1..**
> 7cfa61d165f8d1c030703eb4db6ead**d5d1d86683 100644
> --- a/Source/core/css/**CSSSegmentedFontFace.h
> +++ b/Source/core/css/**CSSSegmentedFontFace.h
> @@ -57,10 +57,6 @@ public:
>
>      PassRefPtr<FontData> getFontData(const FontDescription&);
>
> -#if ENABLE(SVG_FONTS)
> -    bool hasSVGFontFaceSource() const;
> -#endif
> -
>      class LoadFontCallback : public RefCounted<LoadFontCallback> {
>      public:
>          virtual ~LoadFontCallback() { }
> Index: Source/core/css/**CSSSegmentedFontFaceCache.cpp
> diff --git a/Source/core/css/**CSSSegmentedFontFaceCache.cpp
> b/Source/core/css/**CSSSegmentedFontFaceCache.cpp
> index b1ca51611559880823c5a664e59ba0**a1b5071ffe..**
> 7109579d7ba4ad6c93ebe8038ab272**edeba14f68 100644
> --- a/Source/core/css/**CSSSegmentedFontFaceCache.cpp
> +++ b/Source/core/css/**CSSSegmentedFontFaceCache.cpp
> @@ -262,12 +262,6 @@ CSSSegmentedFontFace*
CSSSegmentedFontFaceCache::**getFontFace(const
> FontDescripti
>                  continue;
>              if ((traitsMask & FontVariantNormalMask) &&
> !(candidateTraitsMask & FontVariantNormalMask))
>                  continue;
> -#if ENABLE(SVG_FONTS)
> -            // For SVG Fonts that specify that they only support the
> "normal" variant, we will assume they are incapable
> -            // of small-caps synthesis and just ignore the font face as a
> candidate.
> -            if (candidate->**hasSVGFontFaceSource() && (traitsMask &
> FontVariantSmallCapsMask) && !(candidateTraitsMask &
> FontVariantSmallCapsMask))
> -                continue;
> -#endif
>              if (!face || compareFontFaces(candidate, face.get(),
> traitsMask))
>                  face = candidate;
>          }
>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2013-10-11 05:03:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ksakamoto@chromium.org/26702008/4001

[commit-bot: I haz the power, 2013-10-11 07:11:18]

Change committed as 159429