[Webkit-unassigned] [Bug 192151] [FreeType] Add initial implementation of variation fonts

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Dec 6 12:30:00 PST 2018


Adrian Perez <aperez at igalia.com> changed:

           What    |Removed                     |Added
                 CC|                            |aperez at igalia.com

--- Comment #15 from Adrian Perez <aperez at igalia.com> ---
Comment on attachment 356597
  --> https://bugs.webkit.org/attachment.cgi?id=356597

View in context: https://bugs.webkit.org/attachment.cgi?id=356597&action=review

Without being an expert myself when it comes to FreeType, this
is looking very good!

>> Source/WebCore/platform/graphics/freetype/FontCacheFreeType.cpp:46
> Is this a header file? I'm confused.

Yes, this is correct. FreeType is one of the few projects which makes
use of include expansions from macros. Look:

  % grep -r 'define FT_MULTIPLE' /usr/include/freetype2/
  /usr/include/freetype2/freetype/config/ftheader.h:#define FT_MULTIPLE_MASTERS_H  <freetype/ftmm.h>


> Source/WebCore/platform/graphics/freetype/FontCustomPlatformDataFreeType.cpp:60
>  }

Given that the destructor is empty, this one here could be removed to
let the C++ compiler generate a default destructor automatically. If you
want to be explicit, you can write this in the struct/class definition:

  struct FontPlatformData {
     // ...
     ~FontPlatformData() = default;

> Source/WebCore/platform/graphics/freetype/FontPlatformDataFreeType.cpp:212
> +    if (m_pattern != other.m_pattern && !FcPatternEqual(m_pattern.get(), other.m_pattern.get()))

This can be a bit puzzling: The comment above says that FcPatternEqual() does NOT
accept NULL pointers, but now that the checks before calling it are removed, there
is no explanation telling why is is known that the m_pattern.get() calls would
always return a non-NULL pointer.

Maybe it would be a good thing to add a note in the comment telling about this,
or even a couple of ASSERTs as you have done in other parts of the code.

You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20181206/13b350a4/attachment.html>

More information about the webkit-unassigned mailing list