[Webkit-unassigned] [Bug 167956] [Harfbuzz] Implement ComplexTextController on top of HarfBuzz

[bugzilla-daemon at webkit.org]

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

Myles C. Maxfield <mmaxfield at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #322296|review?                     |review+
              Flags|                            |

--- Comment #14 from Myles C. Maxfield <mmaxfield at apple.com> ---
Comment on attachment 322296
  --> https://bugs.webkit.org/attachment.cgi?id=322296
Patch

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

This is really great, and is way less code than before!

> Source/WebCore/platform/graphics/ComplexTextController.h:82
> +        static Ref<ComplexTextRun> create(hb_buffer_t* buffer, const Font& font, const UChar* characters, unsigned stringLocation, unsigned stringLength, unsigned indexBegin, unsigned indexEnd, bool ltr)

This is fine for now, but at some point we should probably make the header file not know anything about CT* types or hb_* types. We could do this by migrating to the last create() below. I can probably do this task.

> Source/WebCore/platform/graphics/FontCascade.cpp:1333
> +#if PLATFORM(COCOA) || USE(HARFBUZZ)

Can we change these to !PLATFORM(WIN)? I think Windows is the only one who doesn't want ComplexTextController.

> Source/WebCore/platform/graphics/harfbuzz/ComplexTextControllerHarfBuzz.cpp:102
> +    for (auto& feature : font.fontDescription().featureSettings()) {

What about font-variant-* settings?

> Source/WebCore/platform/graphics/harfbuzz/HarfBuzzShaper.h:-47
> -class HarfBuzzShaper {

\o/

>> Source/WebCore/platform/graphics/harfbuzz/HbUniquePtr.h:37
>> +        RELEASE_ASSERT_NOT_REACHED();
> 
> I have no clue if it would work or not, but you could try static_assert(false) here?

Why don't you " = delete" the function?

-- 
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/20171003/a07c9e84/attachment.html>