[HarfBuzz] DirectWrite backend of HarfBuzz and again, some bits from Mozilla

[Ebrahim Byagowi]

Great. Thanks for the reviews, done
<https://github.com/behdad/harfbuzz/pull/273/commits/8179ff5d7ba4a140cf6743729a22072800e98a79>
.

On Mon, Jun 27, 2016 at 3:01 AM, Nikolay Sivov <bunglehead at gmail.com> wrote:

> On 27.06.2016 0:58, Ebrahim Byagowi wrote:
> > I had a chance to improve some minor thing around DirectWrite backend of
> > HarfBuzz on this pull request
> > <https://github.com/behdad/harfbuzz/pull/273> which basically is
> > suggestion of Nikolay reviews on this mail
> > <
> https://lists.freedesktop.org/archives/harfbuzz/2015-September/005066.html
> >.
> > Thank you Nikolay for the suggestions and sorry for the delay :)
>
> Hi, Ebrahim.
>
> You're very welcome, thanks for working on this.
>
> Another issue I see with current implementation (which is minor because
> it will allocate more than needed, not less, but still):
>
> > retry_getglyphs:
> >   uint16_t* clusterMap = (uint16_t*) malloc (maxGlyphCount * sizeof
> (uint16_t));
> >   uint16_t* glyphIndices = (uint16_t*) malloc (maxGlyphCount * sizeof
> (uint16_t));
> >   DWRITE_SHAPING_TEXT_PROPERTIES* textProperties =
> (DWRITE_SHAPING_TEXT_PROPERTIES*)
> >     malloc (maxGlyphCount * sizeof (DWRITE_SHAPING_TEXT_PROPERTIES));
> >   DWRITE_SHAPING_GLYPH_PROPERTIES* glyphProperties =
> (DWRITE_SHAPING_GLYPH_PROPERTIES*)
> >     malloc (maxGlyphCount * sizeof (DWRITE_SHAPING_GLYPH_PROPERTIES));
>
> Cluster map and text properties should use WCHAR text length, not glyph
> count.
>
> >
> >   hr = analyzer->GetGlyphs (textString, textLength, fontFace, FALSE,
> >     isRightToLeft, &runHead->mScript, localeName, NULL, &dwFeatures,
> >     featureRangeLengths, 1, maxGlyphCount, clusterMap, textProperties,
> glyphIndices,
> >     glyphProperties, &glyphCount);
> >
> >   if (unlikely (hr == HRESULT_FROM_WIN32 (ERROR_INSUFFICIENT_BUFFER)))
> >   {
> >     free (clusterMap);
> >     free (glyphIndices);
> >     free (textProperties);
> >     free (glyphProperties);
> >
> >     maxGlyphCount *= 2;
> >
> >     goto retry_getglyphs;
> >   }
>
> And this could do realloc() or free/malloc on glyphIndices and
> glyphProperties only, because text length never changes.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/harfbuzz/attachments/20160627/e9d7590b/attachment.html>