[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

--- Comment #16 from Carlos Garcia Campos <cgarcia at igalia.com> ---
(In reply to Myles C. Maxfield from comment #14)
> Comment on attachment 322296 [details]
> 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!

Indeed, and hopefully less buggy too :-)

> > 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.

Ok, let me know if I can help.

> > 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?

I don't know I copied what we had, if there's anything missing it can be added in a separate patch, I don't even know what font-variant-* settings are :-P

> > 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?

Good question :-)

-- 
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/20171004/9805f4cd/attachment.html>