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 <[email protected]> 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. >
_______________________________________________ HarfBuzz mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/harfbuzz
