From 6b861dbd8b3662d0fa0e51fad1736d72192da868 Mon Sep 17 00:00:00 2001 From: Ebrahim Byagowi Date: Tue, 21 Jun 2016 13:57:26 +0430 Subject: [PATCH 1/5] [dwrite] Use stream font loader instead GDI interop With help of https://dxr.mozilla.org/mozilla-central/source/gfx/2d/NativeFontResourceDWrite.cpp --- src/hb-directwrite.cc | 378 +++++++++++++++++------------------------- src/hb-directwrite.h | 2 +- 2 files changed, 149 insertions(+), 231 deletions(-) diff --git a/src/hb-directwrite.cc b/src/hb-directwrite.cc index 96d1870a0..f273f51b8 100644 --- a/src/hb-directwrite.cc +++ b/src/hb-directwrite.cc @@ -45,176 +45,172 @@ HB_SHAPER_DATA_ENSURE_DECLARE(directwrite, face) HB_SHAPER_DATA_ENSURE_DECLARE(directwrite, font) + +/* + * DirectWrite font stream helpers + */ + +// This is a font loader which provides only one font (unlike its original design). +// For a better implementation which was also source of this +// and DWriteFontFileStream, have a look at to NativeFontResourceDWrite.cpp in Mozilla +class DWriteFontFileLoader : public IDWriteFontFileLoader +{ +private: + IDWriteFontFileStream *mFontFileStream; +public: + DWriteFontFileLoader (IDWriteFontFileStream *fontFileStream) { + mFontFileStream = fontFileStream; + } + + // IUnknown interface + IFACEMETHOD(QueryInterface)(IID const& iid, OUT void** ppObject) { return S_OK; } + IFACEMETHOD_(ULONG, AddRef)() { return 1; } + IFACEMETHOD_(ULONG, Release)() { return 1; } + + // IDWriteFontFileLoader methods + virtual HRESULT STDMETHODCALLTYPE CreateStreamFromKey(void const* fontFileReferenceKey, + UINT32 fontFileReferenceKeySize, + OUT IDWriteFontFileStream** fontFileStream) + { + *fontFileStream = mFontFileStream; + return S_OK; + } +}; + +class DWriteFontFileStream : public IDWriteFontFileStream +{ +private: + uint8_t *mData; + uint32_t mSize; +public: + DWriteFontFileStream(uint8_t *aData, uint32_t aSize) + { + mData = aData; + mSize = aSize; + } + + // IUnknown interface + IFACEMETHOD(QueryInterface)(IID const& iid, OUT void** ppObject) { return S_OK; } + IFACEMETHOD_(ULONG, AddRef)() { return 1; } + IFACEMETHOD_(ULONG, Release)() { return 1; } + + // IDWriteFontFileStream methods + virtual HRESULT STDMETHODCALLTYPE ReadFileFragment(void const** fragmentStart, + UINT64 fileOffset, + UINT64 fragmentSize, + OUT void** fragmentContext) + { + // We are required to do bounds checking. + if (fileOffset + fragmentSize > mSize) { + return E_FAIL; + } + + // truncate the 64 bit fileOffset to size_t sized index into mData + size_t index = static_cast (fileOffset); + + // We should be alive for the duration of this. + *fragmentStart = &mData[index]; + *fragmentContext = nullptr; + return S_OK; + } + + virtual void STDMETHODCALLTYPE ReleaseFileFragment(void* fragmentContext) { } + + virtual HRESULT STDMETHODCALLTYPE GetFileSize(OUT UINT64* fileSize) + { + *fileSize = mSize; + return S_OK; + } + + virtual HRESULT STDMETHODCALLTYPE GetLastWriteTime(OUT UINT64* lastWriteTime) + { + return E_NOTIMPL; + } +}; + + /* * shaper face data */ struct hb_directwrite_shaper_face_data_t { - HANDLE fh; - wchar_t face_name[LF_FACESIZE]; + IDWriteFactory* dwriteFactory; + IDWriteFontFile* fontFile; + IDWriteFontFileLoader* fontFileLoader; + IDWriteFontFace* fontFace; }; -/* face_name should point to a wchar_t[LF_FACESIZE] object. */ -static void -_hb_generate_unique_face_name(wchar_t *face_name, unsigned int *plen) -{ - /* We'll create a private name for the font from a UUID using a simple, - * somewhat base64-like encoding scheme */ - const char *enc = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+-"; - UUID id; - UuidCreate ((UUID*)&id); - ASSERT_STATIC (2 + 3 * (16 / 2) < LF_FACESIZE); - unsigned int name_str_len = 0; - face_name[name_str_len++] = 'F'; - face_name[name_str_len++] = '_'; - unsigned char *p = (unsigned char *)&id; - for (unsigned int i = 0; i < 16; i += 2) - { - /* Spread the 16 bits from two bytes of the UUID across three chars of face_name, - * using the bits in groups of 5,5,6 to select chars from enc. - * This will generate 24 characters; with the 'F_' prefix we already provided, - * the name will be 26 chars (plus the NUL terminator), so will always fit within - * face_name (LF_FACESIZE = 32). */ - face_name[name_str_len++] = enc[p[i] >> 3]; - face_name[name_str_len++] = enc[((p[i] << 2) | (p[i + 1] >> 6)) & 0x1f]; - face_name[name_str_len++] = enc[p[i + 1] & 0x3f]; - } - face_name[name_str_len] = 0; - if (plen) - *plen = name_str_len; -} - -/* Destroys blob. */ -static hb_blob_t * -_hb_rename_font(hb_blob_t *blob, wchar_t *new_name) -{ - /* Create a copy of the font data, with the 'name' table replaced by a - * table that names the font with our private F_* name created above. - * For simplicity, we just append a new 'name' table and update the - * sfnt directory; the original table is left in place, but unused. - * - * The new table will contain just 5 name IDs: family, style, unique, - * full, PS. All of them point to the same name data with our unique name. - */ - - blob = OT::Sanitizer::sanitize (blob); - - unsigned int length, new_length, name_str_len; - const char *orig_sfnt_data = hb_blob_get_data (blob, &length); - - _hb_generate_unique_face_name (new_name, &name_str_len); - - static const uint16_t name_IDs[] = { 1, 2, 3, 4, 6 }; - - unsigned int name_table_length = OT::name::min_size + - ARRAY_LENGTH(name_IDs) * OT::NameRecord::static_size + - name_str_len * 2; /* for name data in UTF16BE form */ - unsigned int name_table_offset = (length + 3) & ~3; - - new_length = name_table_offset + ((name_table_length + 3) & ~3); - void *new_sfnt_data = calloc(1, new_length); - if (!new_sfnt_data) - { - hb_blob_destroy (blob); - return NULL; - } - - memcpy(new_sfnt_data, orig_sfnt_data, length); - - OT::name &name = OT::StructAtOffset (new_sfnt_data, name_table_offset); - name.format.set (0); - name.count.set (ARRAY_LENGTH (name_IDs)); - name.stringOffset.set (name.get_size()); - for (unsigned int i = 0; i < ARRAY_LENGTH (name_IDs); i++) - { - OT::NameRecord &record = name.nameRecord[i]; - record.platformID.set(3); - record.encodingID.set(1); - record.languageID.set(0x0409u); /* English */ - record.nameID.set(name_IDs[i]); - record.length.set(name_str_len * 2); - record.offset.set(0); - } - - /* Copy string data from new_name, converting wchar_t to UTF16BE. */ - unsigned char *p = &OT::StructAfter(name); - for (unsigned int i = 0; i < name_str_len; i++) - { - *p++ = new_name[i] >> 8; - *p++ = new_name[i] & 0xff; - } - - /* Adjust name table entry to point to new name table */ - const OT::OpenTypeFontFile &file = *(OT::OpenTypeFontFile *) (new_sfnt_data); - unsigned int face_count = file.get_face_count (); - for (unsigned int face_index = 0; face_index < face_count; face_index++) - { - /* Note: doing multiple edits (ie. TTC) can be unsafe. There may be - * toe-stepping. But we don't really care. */ - const OT::OpenTypeFontFace &face = file.get_face (face_index); - unsigned int index; - if (face.find_table_index (HB_OT_TAG_name, &index)) - { - OT::TableRecord &record = const_cast (face.get_table (index)); - record.checkSum.set_for_data (&name, name_table_length); - record.offset.set (name_table_offset); - record.length.set (name_table_length); - } - else if (face_index == 0) /* Fail if first face doesn't have 'name' table. */ - { - free (new_sfnt_data); - hb_blob_destroy (blob); - return NULL; - } - } - - /* The checkSumAdjustment field in the 'head' table is now wrong, - * but that doesn't actually seem to cause any problems so we don't - * bother. */ - - hb_blob_destroy (blob); - return hb_blob_create ((const char *)new_sfnt_data, new_length, - HB_MEMORY_MODE_WRITABLE, NULL, free); -} - hb_directwrite_shaper_face_data_t * _hb_directwrite_shaper_face_data_create(hb_face_t *face) { hb_directwrite_shaper_face_data_t *data = - (hb_directwrite_shaper_face_data_t *) calloc (1, sizeof (hb_directwrite_shaper_face_data_t)); + (hb_directwrite_shaper_face_data_t *) malloc (sizeof (hb_directwrite_shaper_face_data_t)); if (unlikely (!data)) return NULL; - hb_blob_t *blob = hb_face_reference_blob (face); - if (unlikely (!hb_blob_get_length (blob))) - DEBUG_MSG(DIRECTWRITE, face, "Face has empty blob"); + // TODO: factory and fontFileLoader should be cached separately + IDWriteFactory* dwriteFactory; + DWriteCreateFactory ( + DWRITE_FACTORY_TYPE_SHARED, + __uuidof (IDWriteFactory), + (IUnknown**) &dwriteFactory + ); - blob = _hb_rename_font (blob, data->face_name); - if (unlikely (!blob)) - { - free(data); - return NULL; + + HRESULT hr; + hb_blob_t* blob = hb_face_reference_blob (face); + IDWriteFontFileStream *fontFileStream = new DWriteFontFileStream ( + (uint8_t*) hb_blob_get_data (blob, NULL), hb_blob_get_length (blob)); + + IDWriteFontFileLoader *fontFileLoader = new DWriteFontFileLoader (fontFileStream); + dwriteFactory->RegisterFontFileLoader (fontFileLoader); + + IDWriteFontFile *fontFile; + uint64_t fontFileKey = 0; + hr = dwriteFactory->CreateCustomFontFileReference (&fontFileKey, sizeof (fontFileKey), + fontFileLoader, &fontFile); + +#define FAIL(...) \ + HB_STMT_START { \ + DEBUG_MSG (DIRECTWRITE, NULL, __VA_ARGS__); \ + return false; \ + } HB_STMT_END; + + if (FAILED (hr)) { + FAIL ("Failed to load font file from data!"); + return false; } - DWORD num_fonts_installed; - data->fh = AddFontMemResourceEx ((void *)hb_blob_get_data(blob, NULL), - hb_blob_get_length (blob), - 0, &num_fonts_installed); - if (unlikely (!data->fh)) - { - DEBUG_MSG (DIRECTWRITE, face, "Face AddFontMemResourceEx() failed"); - free (data); - return NULL; + BOOL isSupported; + DWRITE_FONT_FILE_TYPE fileType; + DWRITE_FONT_FACE_TYPE faceType; + UINT32 numberOfFaces; + hr = fontFile->Analyze (&isSupported, &fileType, &faceType, &numberOfFaces); + if (FAILED (hr) || !isSupported) { + FAIL ("Font file is not supported."); + return false; } +#undef FAIL + + IDWriteFontFace *fontFace; + dwriteFactory->CreateFontFace (faceType, 1, &fontFile, 0, + DWRITE_FONT_SIMULATIONS_NONE, &fontFace); + + data->dwriteFactory = dwriteFactory; + data->fontFile = fontFile; + data->fontFileLoader = fontFileLoader; + data->fontFace = fontFace; + return data; } void _hb_directwrite_shaper_face_data_destroy(hb_directwrite_shaper_face_data_t *data) { - RemoveFontMemResourceEx(data->fh); - free(data); + data->dwriteFactory->UnregisterFontFileLoader (data->fontFileLoader); + delete data->fontFileLoader; + free (data); } @@ -223,90 +219,27 @@ _hb_directwrite_shaper_face_data_destroy(hb_directwrite_shaper_face_data_t *data */ struct hb_directwrite_shaper_font_data_t { - HDC hdc; - LOGFONTW log_font; - HFONT hfont; }; -static bool -populate_log_font (LOGFONTW *lf, - hb_font_t *font) -{ - memset (lf, 0, sizeof (*lf)); - lf->lfHeight = -font->y_scale; - lf->lfCharSet = DEFAULT_CHARSET; - - hb_face_t *face = font->face; - hb_directwrite_shaper_face_data_t *face_data = HB_SHAPER_DATA_GET (face); - - memcpy (lf->lfFaceName, face_data->face_name, sizeof (lf->lfFaceName)); - - return true; -} - hb_directwrite_shaper_font_data_t * _hb_directwrite_shaper_font_data_create (hb_font_t *font) { if (unlikely (!hb_directwrite_shaper_face_data_ensure (font->face))) return NULL; hb_directwrite_shaper_font_data_t *data = - (hb_directwrite_shaper_font_data_t *) calloc (1, sizeof (hb_directwrite_shaper_font_data_t)); + (hb_directwrite_shaper_font_data_t *) malloc (sizeof (hb_directwrite_shaper_font_data_t)); if (unlikely (!data)) return NULL; - data->hdc = GetDC (NULL); - - if (unlikely (!populate_log_font (&data->log_font, font))) - { - DEBUG_MSG (DIRECTWRITE, font, "Font populate_log_font() failed"); - _hb_directwrite_shaper_font_data_destroy (data); - return NULL; - } - - data->hfont = CreateFontIndirectW (&data->log_font); - if (unlikely (!data->hfont)) - { - DEBUG_MSG (DIRECTWRITE, font, "Font CreateFontIndirectW() failed"); - _hb_directwrite_shaper_font_data_destroy (data); - return NULL; - } - - if (!SelectObject (data->hdc, data->hfont)) - { - DEBUG_MSG (DIRECTWRITE, font, "Font SelectObject() failed"); - _hb_directwrite_shaper_font_data_destroy (data); - return NULL; - } - return data; } void _hb_directwrite_shaper_font_data_destroy (hb_directwrite_shaper_font_data_t *data) { - if (data->hdc) - ReleaseDC (NULL, data->hdc); - if (data->hfont) - DeleteObject (data->hfont); free (data); } -LOGFONTW * -hb_directwrite_font_get_logfontw (hb_font_t *font) -{ - if (unlikely (!hb_directwrite_shaper_font_data_ensure (font))) return NULL; - hb_directwrite_shaper_font_data_t *font_data = HB_SHAPER_DATA_GET (font); - return &font_data->log_font; -} - -HFONT -hb_directwrite_font_get_hfont (hb_font_t *font) -{ - if (unlikely (!hb_directwrite_shaper_font_data_ensure (font))) return NULL; - hb_directwrite_shaper_font_data_t *font_data = HB_SHAPER_DATA_GET (font); - return font_data->hfont; -} - /* * shaper shape_plan data @@ -327,7 +260,7 @@ _hb_directwrite_shaper_shape_plan_data_destroy (hb_directwrite_shaper_shape_plan { } -// Most of here TextAnalysis is originally written by Bas Schouten for Mozilla project +// Most of TextAnalysis is originally written by Bas Schouten for Mozilla project // but now is relicensed to MIT for HarfBuzz use class TextAnalysis : public IDWriteTextAnalysisSource, public IDWriteTextAnalysisSink @@ -581,9 +514,9 @@ protected: Run mRunHead; }; -static inline uint16_t hb_uint16_swap (const uint16_t v) +static inline uint16_t hb_uint16_swap(const uint16_t v) { return (v >> 8) | (v << 8); } -static inline uint32_t hb_uint32_swap (const uint32_t v) +static inline uint32_t hb_uint32_swap(const uint32_t v) { return (hb_uint16_swap(v) << 16) | hb_uint16_swap(v >> 16); } /* @@ -600,23 +533,8 @@ _hb_directwrite_shape(hb_shape_plan_t *shape_plan, hb_face_t *face = font->face; hb_directwrite_shaper_face_data_t *face_data = HB_SHAPER_DATA_GET (face); hb_directwrite_shaper_font_data_t *font_data = HB_SHAPER_DATA_GET (font); - - // factory probably should be cached -#ifndef HB_DIRECTWRITE_EXPERIMENTAL_JUSTIFICATION - IDWriteFactory* dwriteFactory; -#else - IDWriteFactory1* dwriteFactory; -#endif - DWriteCreateFactory ( - DWRITE_FACTORY_TYPE_SHARED, - __uuidof (IDWriteFactory), - (IUnknown**) &dwriteFactory - ); - - IDWriteGdiInterop *gdiInterop; - dwriteFactory->GetGdiInterop (&gdiInterop); - IDWriteFontFace* fontFace; - gdiInterop->CreateFontFaceFromHdc (font_data->hdc, &fontFace); + IDWriteFactory *dwriteFactory = face_data->dwriteFactory; + IDWriteFontFace *fontFace = face_data->fontFace; #ifndef HB_DIRECTWRITE_EXPERIMENTAL_JUSTIFICATION IDWriteTextAnalyzer* analyzer; @@ -672,7 +590,6 @@ _hb_directwrite_shape(hb_shape_plan_t *shape_plan, } } - HRESULT hr; // TODO: Handle TEST_DISABLE_OPTIONAL_LIGATURES DWRITE_READING_DIRECTION readingDirection = buffer->props.direction ? @@ -688,6 +605,7 @@ _hb_directwrite_shape(hb_shape_plan_t *shape_plan, TextAnalysis analysis(textString, textLength, NULL, readingDirection); TextAnalysis::Run *runHead; + HRESULT hr; hr = analysis.GenerateResults(analyzer, &runHead); #define FAIL(...) \ diff --git a/src/hb-directwrite.h b/src/hb-directwrite.h index adf33dfe9..0e1b4799d 100644 --- a/src/hb-directwrite.h +++ b/src/hb-directwrite.h @@ -31,4 +31,4 @@ HB_BEGIN_DECLS HB_END_DECLS -#endif /* HB_UNISCRIBE_H */ +#endif /* HB_DIRECTWRITE_H */ From f3f0ea980a359343ac0e3d359a95855c2cf7be25 Mon Sep 17 00:00:00 2001 From: Ebrahim Byagowi Date: Thu, 23 Jun 2016 16:41:37 +0430 Subject: [PATCH 2/5] [dwrite] Remove ifdefs without breaking execution on old Windows versions --- src/hb-directwrite.cc | 209 +++++++++++++++++++++--------------------- 1 file changed, 105 insertions(+), 104 deletions(-) diff --git a/src/hb-directwrite.cc b/src/hb-directwrite.cc index f273f51b8..c6fac4d30 100644 --- a/src/hb-directwrite.cc +++ b/src/hb-directwrite.cc @@ -25,11 +25,7 @@ #define HB_SHAPER directwrite #include "hb-shaper-impl-private.hh" -#ifndef HB_DIRECTWRITE_EXPERIMENTAL_JUSTIFICATION - #include -#else - #include -#endif +#include #include "hb-directwrite.h" @@ -156,7 +152,6 @@ _hb_directwrite_shaper_face_data_create(hb_face_t *face) (IUnknown**) &dwriteFactory ); - HRESULT hr; hb_blob_t* blob = hb_face_reference_blob (face); IDWriteFontFileStream *fontFileStream = new DWriteFontFileStream ( @@ -377,7 +372,8 @@ public: IFACEMETHODIMP GetLocaleName(uint32_t textPosition, uint32_t* textLength, - wchar_t const** localeName) { + wchar_t const** localeName) + { return S_OK; } @@ -402,7 +398,8 @@ public: { SetCurrentRun(textPosition); SplitCurrentRun(textPosition); - while (textLength > 0) { + while (textLength > 0) + { Run *run = FetchNextRun(&textLength); run->mScript = *scriptAnalysis; } @@ -435,10 +432,12 @@ protected: Run *origRun = mCurrentRun; // Split the tail if needed (the length remaining is less than the // current run's size). - if (*textLength < mCurrentRun->mTextLength) { - SplitCurrentRun(mCurrentRun->mTextStart + *textLength); + if (*textLength < mCurrentRun->mTextLength) + { + SplitCurrentRun (mCurrentRun->mTextStart + *textLength); } - else { + else + { // Just advance the current run. mCurrentRun = mCurrentRun->nextRun; } @@ -455,12 +454,14 @@ protected: // this will usually just return early. If not, find the // corresponding run for the text position. - if (mCurrentRun && mCurrentRun->ContainsTextPosition(textPosition)) { + if (mCurrentRun && mCurrentRun->ContainsTextPosition (textPosition)) + { return; } for (Run *run = &mRunHead; run; run = run->nextRun) { - if (run->ContainsTextPosition(textPosition)) { + if (run->ContainsTextPosition (textPosition)) + { mCurrentRun = run; return; } @@ -471,13 +472,15 @@ protected: void SplitCurrentRun(uint32_t splitPosition) { - if (!mCurrentRun) { + if (!mCurrentRun) + { //NS_ASSERTION(false, "SplitCurrentRun called without current run."); // Shouldn't be calling this when no current run is set! return; } // Split the current run. - if (splitPosition <= mCurrentRun->mTextStart) { + if (splitPosition <= mCurrentRun->mTextStart) + { // No need to split, already the start of a run // or before it. Usually the first. return; @@ -514,9 +517,9 @@ protected: Run mRunHead; }; -static inline uint16_t hb_uint16_swap(const uint16_t v) +static inline uint16_t hb_uint16_swap (const uint16_t v) { return (v >> 8) | (v << 8); } -static inline uint32_t hb_uint32_swap(const uint32_t v) +static inline uint32_t hb_uint32_swap (const uint32_t v) { return (hb_uint16_swap(v) << 16) | hb_uint16_swap(v >> 16); } /* @@ -536,14 +539,8 @@ _hb_directwrite_shape(hb_shape_plan_t *shape_plan, IDWriteFactory *dwriteFactory = face_data->dwriteFactory; IDWriteFontFace *fontFace = face_data->fontFace; -#ifndef HB_DIRECTWRITE_EXPERIMENTAL_JUSTIFICATION IDWriteTextAnalyzer* analyzer; dwriteFactory->CreateTextAnalyzer(&analyzer); -#else - IDWriteTextAnalyzer* analyzer0; - dwriteFactory->CreateTextAnalyzer (&analyzer0); - IDWriteTextAnalyzer1* analyzer = (IDWriteTextAnalyzer1*) analyzer0; -#endif unsigned int scratch_size; hb_buffer_t::scratch_buffer_t *scratch = buffer->get_scratch_buffer (&scratch_size); @@ -717,106 +714,110 @@ retry_getglyphs: return false; } -#ifdef HB_DIRECTWRITE_EXPERIMENTAL_JUSTIFICATION - - DWRITE_JUSTIFICATION_OPPORTUNITY* justificationOpportunities = - (DWRITE_JUSTIFICATION_OPPORTUNITY*) - malloc (maxGlyphCount * sizeof (DWRITE_JUSTIFICATION_OPPORTUNITY)); - hr = analyzer->GetJustificationOpportunities (fontFace, fontEmSize, - runHead->mScript, textLength, glyphCount, textString, clusterMap, - glyphProperties, justificationOpportunities); - - if (FAILED (hr)) - { - FAIL ("Analyzer failed to get justification opportunities."); - return false; - } - // TODO: get lineWith from somewhere - float lineWidth = 60000; + float lineWidth = 0; - float* justifiedGlyphAdvances = - (float*) malloc (maxGlyphCount * sizeof (float)); - DWRITE_GLYPH_OFFSET* justifiedGlyphOffsets = (DWRITE_GLYPH_OFFSET*) - malloc (glyphCount * sizeof (DWRITE_GLYPH_OFFSET)); - hr = analyzer->JustifyGlyphAdvances (lineWidth, glyphCount, justificationOpportunities, - glyphAdvances, glyphOffsets, justifiedGlyphAdvances, justifiedGlyphOffsets); + IDWriteTextAnalyzer1* analyzer1; + analyzer->QueryInterface (&analyzer1); - if (FAILED (hr)) + if (analyzer1 && lineWidth) { - FAIL ("Analyzer failed to get justified glyph advances."); - return false; - } - DWRITE_SCRIPT_PROPERTIES scriptProperties; - hr = analyzer->GetScriptProperties (runHead->mScript, &scriptProperties); - if (FAILED (hr)) - { - FAIL ("Analyzer failed to get script properties."); - return false; - } - uint32_t justificationCharacter = scriptProperties.justificationCharacter; + DWRITE_JUSTIFICATION_OPPORTUNITY* justificationOpportunities = + (DWRITE_JUSTIFICATION_OPPORTUNITY*) + malloc (maxGlyphCount * sizeof (DWRITE_JUSTIFICATION_OPPORTUNITY)); + hr = analyzer1->GetJustificationOpportunities (fontFace, fontEmSize, + runHead->mScript, textLength, glyphCount, textString, clusterMap, + glyphProperties, justificationOpportunities); - // if a script justificationCharacter is not space, it can have GetJustifiedGlyphs - if (justificationCharacter != 32) - { -retry_getjustifiedglyphs: - uint16_t* modifiedClusterMap = (uint16_t*) malloc (maxGlyphCount * sizeof (uint16_t)); - uint16_t* modifiedGlyphIndices = (uint16_t*) malloc (maxGlyphCount * sizeof (uint16_t)); - float* modifiedGlyphAdvances = (float*) malloc (maxGlyphCount * sizeof (float)); - DWRITE_GLYPH_OFFSET* modifiedGlyphOffsets = (DWRITE_GLYPH_OFFSET*) - malloc (maxGlyphCount * sizeof (DWRITE_GLYPH_OFFSET)); - uint32_t actualGlyphsCount; - hr = analyzer->GetJustifiedGlyphs (fontFace, fontEmSize, runHead->mScript, + if (FAILED (hr)) + { + FAIL ("Analyzer failed to get justification opportunities."); + return false; + } + + float* justifiedGlyphAdvances = + (float*) malloc (maxGlyphCount * sizeof (float)); + DWRITE_GLYPH_OFFSET* justifiedGlyphOffsets = (DWRITE_GLYPH_OFFSET*) + malloc (glyphCount * sizeof (DWRITE_GLYPH_OFFSET)); + hr = analyzer1->JustifyGlyphAdvances (lineWidth, glyphCount, justificationOpportunities, + glyphAdvances, glyphOffsets, justifiedGlyphAdvances, justifiedGlyphOffsets); + + if (FAILED (hr)) + { + FAIL("Analyzer failed to get justified glyph advances."); + return false; + } + + DWRITE_SCRIPT_PROPERTIES scriptProperties; + hr = analyzer1->GetScriptProperties (runHead->mScript, &scriptProperties); + if (FAILED (hr)) + { + FAIL("Analyzer failed to get script properties."); + return false; + } + uint32_t justificationCharacter = scriptProperties.justificationCharacter; + + // if a script justificationCharacter is not space, it can have GetJustifiedGlyphs + if (justificationCharacter != 32) + { + retry_getjustifiedglyphs: + uint16_t* modifiedClusterMap = (uint16_t*) malloc (maxGlyphCount * sizeof(uint16_t)); + uint16_t* modifiedGlyphIndices = (uint16_t*) malloc (maxGlyphCount * sizeof(uint16_t)); + float* modifiedGlyphAdvances = (float*) malloc (maxGlyphCount * sizeof(float)); + DWRITE_GLYPH_OFFSET* modifiedGlyphOffsets = (DWRITE_GLYPH_OFFSET*) + malloc (maxGlyphCount * sizeof (DWRITE_GLYPH_OFFSET)); + uint32_t actualGlyphsCount; + hr = analyzer1->GetJustifiedGlyphs (fontFace, fontEmSize, runHead->mScript, textLength, glyphCount, maxGlyphCount, clusterMap, glyphIndices, glyphAdvances, justifiedGlyphAdvances, justifiedGlyphOffsets, glyphProperties, &actualGlyphsCount, modifiedClusterMap, modifiedGlyphIndices, modifiedGlyphAdvances, modifiedGlyphOffsets); - if (hr == HRESULT_FROM_WIN32 (ERROR_INSUFFICIENT_BUFFER)) - { - maxGlyphCount = actualGlyphsCount; - free (modifiedClusterMap); - free (modifiedGlyphIndices); - free (modifiedGlyphAdvances); - free (modifiedGlyphOffsets); + if (hr == HRESULT_FROM_WIN32 (ERROR_INSUFFICIENT_BUFFER)) + { + maxGlyphCount = actualGlyphsCount; + free (modifiedClusterMap); + free (modifiedGlyphIndices); + free (modifiedGlyphAdvances); + free (modifiedGlyphOffsets); - maxGlyphCount = actualGlyphsCount; + maxGlyphCount = actualGlyphsCount; - goto retry_getjustifiedglyphs; + goto retry_getjustifiedglyphs; + } + if (FAILED (hr)) + { + FAIL ("Analyzer failed to get justified glyphs."); + return false; + } + + free (clusterMap); + free (glyphIndices); + free (glyphAdvances); + free (glyphOffsets); + + glyphCount = actualGlyphsCount; + clusterMap = modifiedClusterMap; + glyphIndices = modifiedGlyphIndices; + glyphAdvances = modifiedGlyphAdvances; + glyphOffsets = modifiedGlyphOffsets; + + free (justifiedGlyphAdvances); + free (justifiedGlyphOffsets); } - if (FAILED (hr)) + else { - FAIL ("Analyzer failed to get justified glyphs."); - return false; + free (glyphAdvances); + free (glyphOffsets); + + glyphAdvances = justifiedGlyphAdvances; + glyphOffsets = justifiedGlyphOffsets; } - free (clusterMap); - free (glyphIndices); - free (glyphAdvances); - free (glyphOffsets); + free (justificationOpportunities); - glyphCount = actualGlyphsCount; - clusterMap = modifiedClusterMap; - glyphIndices = modifiedGlyphIndices; - glyphAdvances = modifiedGlyphAdvances; - glyphOffsets = modifiedGlyphOffsets; - - free(justifiedGlyphAdvances); - free(justifiedGlyphOffsets); } - else - { - free(glyphAdvances); - free(glyphOffsets); - - glyphAdvances = justifiedGlyphAdvances; - glyphOffsets = justifiedGlyphOffsets; - } - - free(justificationOpportunities); - -#endif /* Ok, we've got everything we need, now compose output buffer, * very, *very*, carefully! */ From be565d17141818e006aa1e4582f3ae14c726fa85 Mon Sep 17 00:00:00 2001 From: Ebrahim Byagowi Date: Fri, 24 Jun 2016 11:42:01 +0430 Subject: [PATCH 3/5] [dwrite] Release allocated blob on face destroy This reduces memory consumption of my iterated font create/destroy cycle test significantly and makes it much better than uniscribe backend even --- src/hb-directwrite.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/hb-directwrite.cc b/src/hb-directwrite.cc index c6fac4d30..a74e31833 100644 --- a/src/hb-directwrite.cc +++ b/src/hb-directwrite.cc @@ -134,6 +134,7 @@ struct hb_directwrite_shaper_face_data_t { IDWriteFontFile* fontFile; IDWriteFontFileLoader* fontFileLoader; IDWriteFontFace* fontFace; + hb_blob_t* faceBlob; }; hb_directwrite_shaper_face_data_t * @@ -153,7 +154,7 @@ _hb_directwrite_shaper_face_data_create(hb_face_t *face) ); HRESULT hr; - hb_blob_t* blob = hb_face_reference_blob (face); + hb_blob_t *blob = hb_face_reference_blob (face); IDWriteFontFileStream *fontFileStream = new DWriteFontFileStream ( (uint8_t*) hb_blob_get_data (blob, NULL), hb_blob_get_length (blob)); @@ -196,6 +197,7 @@ _hb_directwrite_shaper_face_data_create(hb_face_t *face) data->fontFile = fontFile; data->fontFileLoader = fontFileLoader; data->fontFace = fontFace; + data->faceBlob = blob; return data; } @@ -205,6 +207,7 @@ _hb_directwrite_shaper_face_data_destroy(hb_directwrite_shaper_face_data_t *data { data->dwriteFactory->UnregisterFontFileLoader (data->fontFileLoader); delete data->fontFileLoader; + hb_blob_destroy (data->faceBlob); free (data); } From 07b724f3419a28c479cd8a75ae0eecb841a6d2f3 Mon Sep 17 00:00:00 2001 From: Ebrahim Byagowi Date: Fri, 24 Jun 2016 12:23:25 +0430 Subject: [PATCH 4/5] [dwrite] Delete remained objects No longer noticeable memory increase on create/destroy iterations, highly better than current state of uniscribe backend --- src/hb-directwrite.cc | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/src/hb-directwrite.cc b/src/hb-directwrite.cc index a74e31833..36b4d5d98 100644 --- a/src/hb-directwrite.cc +++ b/src/hb-directwrite.cc @@ -130,11 +130,12 @@ public: */ struct hb_directwrite_shaper_face_data_t { - IDWriteFactory* dwriteFactory; - IDWriteFontFile* fontFile; - IDWriteFontFileLoader* fontFileLoader; - IDWriteFontFace* fontFace; - hb_blob_t* faceBlob; + IDWriteFactory *dwriteFactory; + IDWriteFontFile *fontFile; + IDWriteFontFileStream *fontFileStream; + IDWriteFontFileLoader *fontFileLoader; + IDWriteFontFace *fontFace; + hb_blob_t *faceBlob; }; hb_directwrite_shaper_face_data_t * @@ -195,6 +196,7 @@ _hb_directwrite_shaper_face_data_create(hb_face_t *face) data->dwriteFactory = dwriteFactory; data->fontFile = fontFile; + data->fontFileStream = fontFileStream; data->fontFileLoader = fontFileLoader; data->fontFace = fontFace; data->faceBlob = blob; @@ -205,10 +207,23 @@ _hb_directwrite_shaper_face_data_create(hb_face_t *face) void _hb_directwrite_shaper_face_data_destroy(hb_directwrite_shaper_face_data_t *data) { - data->dwriteFactory->UnregisterFontFileLoader (data->fontFileLoader); - delete data->fontFileLoader; - hb_blob_destroy (data->faceBlob); - free (data); + if (data->fontFace) + data->fontFace->Release (); + if (data->fontFile) + data->fontFile->Release (); + if (data->dwriteFactory) { + if (data->fontFileLoader) + data->dwriteFactory->UnregisterFontFileLoader(data->fontFileLoader); + data->dwriteFactory->Release(); + } + if (data->fontFileLoader) + delete data->fontFileLoader; + if (data->fontFileStream) + delete data->fontFileStream; + if (data->faceBlob) + hb_blob_destroy (data->faceBlob); + if (data) + free (data); } From 8179ff5d7ba4a140cf6743729a22072800e98a79 Mon Sep 17 00:00:00 2001 From: Ebrahim Byagowi Date: Mon, 27 Jun 2016 03:54:15 +0430 Subject: [PATCH 5/5] [dwrite] Don't allocate more than needed Addressing Nikolay Sivov reviews on harfbuzz mailing list --- src/hb-directwrite.cc | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/hb-directwrite.cc b/src/hb-directwrite.cc index 36b4d5d98..09889d04e 100644 --- a/src/hb-directwrite.cc +++ b/src/hb-directwrite.cc @@ -664,11 +664,11 @@ _hb_directwrite_shape(hb_shape_plan_t *shape_plan, (const DWRITE_TYPOGRAPHIC_FEATURES*) &singleFeatures; const uint32_t featureRangeLengths[] = { textLength }; -retry_getglyphs: - uint16_t* clusterMap = (uint16_t*) malloc (maxGlyphCount * sizeof (uint16_t)); - uint16_t* glyphIndices = (uint16_t*) malloc (maxGlyphCount * sizeof (uint16_t)); + uint16_t* clusterMap = (uint16_t*) malloc (textLength * sizeof (uint16_t)); DWRITE_SHAPING_TEXT_PROPERTIES* textProperties = (DWRITE_SHAPING_TEXT_PROPERTIES*) - malloc (maxGlyphCount * sizeof (DWRITE_SHAPING_TEXT_PROPERTIES)); + malloc (textLength * sizeof (DWRITE_SHAPING_TEXT_PROPERTIES)); +retry_getglyphs: + uint16_t* glyphIndices = (uint16_t*) malloc (maxGlyphCount * sizeof (uint16_t)); DWRITE_SHAPING_GLYPH_PROPERTIES* glyphProperties = (DWRITE_SHAPING_GLYPH_PROPERTIES*) malloc (maxGlyphCount * sizeof (DWRITE_SHAPING_GLYPH_PROPERTIES)); @@ -679,9 +679,7 @@ retry_getglyphs: if (unlikely (hr == HRESULT_FROM_WIN32 (ERROR_INSUFFICIENT_BUFFER))) { - free (clusterMap); free (glyphIndices); - free (textProperties); free (glyphProperties); maxGlyphCount *= 2; @@ -779,10 +777,10 @@ retry_getglyphs: // if a script justificationCharacter is not space, it can have GetJustifiedGlyphs if (justificationCharacter != 32) { + uint16_t* modifiedClusterMap = (uint16_t*) malloc (textLength * sizeof (uint16_t)); retry_getjustifiedglyphs: - uint16_t* modifiedClusterMap = (uint16_t*) malloc (maxGlyphCount * sizeof(uint16_t)); - uint16_t* modifiedGlyphIndices = (uint16_t*) malloc (maxGlyphCount * sizeof(uint16_t)); - float* modifiedGlyphAdvances = (float*) malloc (maxGlyphCount * sizeof(float)); + uint16_t* modifiedGlyphIndices = (uint16_t*) malloc (maxGlyphCount * sizeof (uint16_t)); + float* modifiedGlyphAdvances = (float*) malloc (maxGlyphCount * sizeof (float)); DWRITE_GLYPH_OFFSET* modifiedGlyphOffsets = (DWRITE_GLYPH_OFFSET*) malloc (maxGlyphCount * sizeof (DWRITE_GLYPH_OFFSET)); uint32_t actualGlyphsCount; @@ -795,7 +793,6 @@ retry_getglyphs: if (hr == HRESULT_FROM_WIN32 (ERROR_INSUFFICIENT_BUFFER)) { maxGlyphCount = actualGlyphsCount; - free (modifiedClusterMap); free (modifiedGlyphIndices); free (modifiedGlyphAdvances); free (modifiedGlyphOffsets);