dwrite: Fix wrong run data reported to a renderer.

dlls/dwrite/layout.c

@@ -128,6 +128,8 @@ struct regular_layout_run {
     UINT16 *clustermap;
     FLOAT  *advances;
     DWRITE_GLYPH_OFFSET *offsets;
+    /* this is actual glyph count after shaping, it's not necessary the same as reported to Draw() */
+    UINT32 glyphcount;
 };
 
 struct layout_run {
@@ -436,12 +438,17 @@ static inline void init_cluster_metrics(const struct dwrite_textlayout *layout,
     UINT16 j;
 
     metrics->width = 0.0;
-    for (j = start_glyph; j < stop_glyph; j++)
-        metrics->width += run->run.glyphAdvances[j];
+
+    /* For clusters on control chars we report zero glyphs, and we need zero cluster
+       width as well; advances are already computed at this point and are not necessary zero. */
+    if (run->run.glyphCount) {
+        for (j = start_glyph; j < stop_glyph; j++)
+            metrics->width += run->run.glyphAdvances[j];
+    }
     metrics->length = 0;
 
     position = stop_position;
-    if (stop_glyph == run->run.glyphCount)
+    if (stop_glyph == run->glyphcount)
         breakcondition = get_effective_breakpoint(layout, stop_position).breakConditionAfter;
     else {
         breakcondition = get_effective_breakpoint(layout, stop_position).breakConditionBefore;
@@ -498,7 +505,7 @@ static void layout_set_cluster_metrics(struct dwrite_textlayout *layout, const s
         }
 
         if (end) {
-            init_cluster_metrics(layout, run, run->descr.clusterMap[start], run->run.glyphCount, i, metrics);
+            init_cluster_metrics(layout, run, run->descr.clusterMap[start], run->glyphcount, i, metrics);
             metrics->length = i - start + 1;
             c->position = start;
             c->run = r;
@@ -649,12 +656,12 @@ static HRESULT layout_compute_runs(struct dwrite_textlayout *layout)
             hr = IDWriteTextAnalyzer_GetGlyphs(analyzer, run->descr.string, run->descr.stringLength,
                 run->run.fontFace, run->run.isSideways, run->run.bidiLevel & 1, &run->sa, run->descr.localeName,
                 NULL /* FIXME */, NULL, NULL, 0, max_count, run->clustermap, text_props, run->glyphs, glyph_props,
-                &run->run.glyphCount);
+                &run->glyphcount);
             if (hr == E_NOT_SUFFICIENT_BUFFER) {
                 heap_free(run->glyphs);
                 heap_free(glyph_props);
 
-                max_count = run->run.glyphCount;
+                max_count = run->glyphcount;
 
                 run->glyphs = heap_alloc(max_count*sizeof(UINT16));
                 glyph_props = heap_alloc(max_count*sizeof(DWRITE_SHAPING_GLYPH_PROPERTIES));
@@ -677,21 +684,21 @@ static HRESULT layout_compute_runs(struct dwrite_textlayout *layout)
         run->run.glyphIndices = run->glyphs;
         run->descr.clusterMap = run->clustermap;
 
-        run->advances = heap_alloc(run->run.glyphCount*sizeof(FLOAT));
-        run->offsets = heap_alloc(run->run.glyphCount*sizeof(DWRITE_GLYPH_OFFSET));
+        run->advances = heap_alloc(run->glyphcount*sizeof(FLOAT));
+        run->offsets = heap_alloc(run->glyphcount*sizeof(DWRITE_GLYPH_OFFSET));
         if (!run->advances || !run->offsets)
             goto memerr;
 
         /* now set advances and offsets */
         if (layout->gdicompatible)
             hr = IDWriteTextAnalyzer_GetGdiCompatibleGlyphPlacements(analyzer, run->descr.string, run->descr.clusterMap,
-                text_props, run->descr.stringLength, run->run.glyphIndices, glyph_props, run->run.glyphCount,
+                text_props, run->descr.stringLength, run->run.glyphIndices, glyph_props, run->glyphcount,
                 run->run.fontFace, run->run.fontEmSize, layout->pixels_per_dip, &layout->transform, layout->use_gdi_natural,
                 run->run.isSideways, run->run.bidiLevel & 1, &run->sa, run->descr.localeName, NULL, NULL, 0,
                 run->advances, run->offsets);
         else
             hr = IDWriteTextAnalyzer_GetGlyphPlacements(analyzer, run->descr.string, run->descr.clusterMap, text_props,
-                run->descr.stringLength, run->run.glyphIndices, glyph_props, run->run.glyphCount, run->run.fontFace,
+                run->descr.stringLength, run->run.glyphIndices, glyph_props, run->glyphcount, run->run.fontFace,
                 run->run.fontEmSize, run->run.isSideways, run->run.bidiLevel & 1, &run->sa, run->descr.localeName,
                 NULL, NULL, 0, run->advances, run->offsets);
 
@@ -703,6 +710,13 @@ static HRESULT layout_compute_runs(struct dwrite_textlayout *layout)
         run->run.glyphAdvances = run->advances;
         run->run.glyphOffsets = run->offsets;
 
+        /* Special treatment of control script, shaping code adds normal glyphs for it,
+           with non-zero advances, and layout code exposes those as zero width clusters,
+           so we have to do it manually. */
+        if (run->sa.script == Script_Common)
+            run->run.glyphCount = 0;
+        else
+            run->run.glyphCount = run->glyphcount;
         layout_set_cluster_metrics(layout, r, &cluster);
 
         continue;
@@ -775,11 +789,13 @@ static HRESULT layout_compute(struct dwrite_textlayout *layout)
     return hr;
 }
 
-static HRESULT layout_add_effective_run(struct dwrite_textlayout *layout, const struct layout_run *r, UINT32 start,
-    UINT32 length, FLOAT origin_x)
+/* Effective run is built from consecutive clusters of a single nominal run, 'first_cluster' is 0 based cluster index,
+   'cluster_count' indicates how many clusters to add, including first one. */
+static HRESULT layout_add_effective_run(struct dwrite_textlayout *layout, const struct layout_run *r, UINT32 first_cluster,
+    UINT32 cluster_count, FLOAT origin_x)
 {
+    UINT32 i, start, length, last_cluster;
     struct layout_effective_run *run;
-    UINT32 i;
 
     if (r->kind == LAYOUT_RUN_INLINE) {
         struct layout_effective_inline *inlineobject;
@@ -804,6 +820,10 @@ static HRESULT layout_add_effective_run(struct dwrite_textlayout *layout, const
     if (!run)
         return E_OUTOFMEMORY;
 
+    /* no need to iterate for that */
+    last_cluster = first_cluster + cluster_count - 1;
+    length = layout->clusters[last_cluster].position + layout->clustermetrics[last_cluster].length;
+
     run->clustermap = heap_alloc(sizeof(UINT16)*length);
     if (!run->clustermap) {
         heap_free(run);
@@ -811,16 +831,20 @@ static HRESULT layout_add_effective_run(struct dwrite_textlayout *layout, const
     }
 
     run->run = r;
-    run->start = start;
+    run->start = start = layout->clusters[first_cluster].position;
     run->length = length;
     run->origin_x = origin_x;
     run->origin_y = 0.0; /* FIXME: set after line is built */
 
-    /* trim from the left */
-    run->glyphcount = r->u.regular.run.glyphCount - r->u.regular.clustermap[start];
-    /* trim from the right */
-    if (length < r->u.regular.descr.stringLength)
-        run->glyphcount -= r->u.regular.clustermap[start + length];
+    if (r->u.regular.run.glyphCount) {
+        /* trim from the left */
+        run->glyphcount = r->u.regular.run.glyphCount - r->u.regular.clustermap[start];
+        /* trim from the right */
+        if (length < r->u.regular.descr.stringLength)
+            run->glyphcount -= r->u.regular.clustermap[start + length];
+    }
+    else
+        run->glyphcount = 0;
 
     /* cluster map needs to be shifted */
     for (i = 0; i < length; i++)
@@ -901,8 +925,8 @@ static HRESULT layout_compute_effective_runs(struct dwrite_textlayout *layout)
 
             UINT32 strlength = metrics.length, index = i;
 
-            if (i > start) {
-                hr = layout_add_effective_run(layout, run, start, i - start, origin_x);
+            if (i >= start) {
+                hr = layout_add_effective_run(layout, run, start, i - start + 1, origin_x);
                 if (FAILED(hr))
                     return hr;
                 /* we don't need to update origin for next run as we're going to wrap */

dlls/dwrite/tests/analyzer.c

@@ -258,7 +258,6 @@ static HRESULT WINAPI analysissink_SetScriptAnalysis(IDWriteTextAnalysisSink *if
     UINT32 position, UINT32 length, DWRITE_SCRIPT_ANALYSIS const* sa)
 {
     struct call_entry entry;
-
     entry.kind = ScriptAnalysis;
     entry.sa.pos = position;
     entry.sa.len = length;
@@ -886,6 +885,13 @@ static struct sa_test sa_tests[] = {
             { 2, 3, DWRITE_SCRIPT_SHAPES_DEFAULT   },
             { 5, 1, DWRITE_SCRIPT_SHAPES_NO_VISUAL } }
     },
+    {
+      /* LRE/PDF and other visual and non-visual codes from Common script range */
+      {0x202a,0x202c,'r','!',0x200b,'\r',0}, 3,
+          { { 0, 2, DWRITE_SCRIPT_SHAPES_NO_VISUAL },
+            { 2, 2, DWRITE_SCRIPT_SHAPES_DEFAULT   },
+            { 4, 2, DWRITE_SCRIPT_SHAPES_NO_VISUAL } }
+    },
     /* keep this as end marker */
     { {0} }
 };

dlls/dwrite/tests/layout.c

@@ -31,14 +31,153 @@
 static const WCHAR tahomaW[] = {'T','a','h','o','m','a',0};
 static const WCHAR enusW[] = {'e','n','-','u','s',0};
 
-#define EXPECT_REF(obj,ref) _expect_ref((IUnknown*)obj, ref, __LINE__)
-static void _expect_ref(IUnknown* obj, ULONG ref, int line)
+static DWRITE_SCRIPT_ANALYSIS g_sa;
+static DWRITE_SCRIPT_ANALYSIS g_control_sa;
+
+/* test IDWriteTextAnalysisSink */
+static HRESULT WINAPI analysissink_QueryInterface(IDWriteTextAnalysisSink *iface, REFIID riid, void **obj)
 {
-    ULONG rc = IUnknown_AddRef(obj);
-    IUnknown_Release(obj);
-    ok_(__FILE__,line)(rc-1 == ref, "expected refcount %d, got %d\n", ref, rc-1);
+    if (IsEqualIID(riid, &IID_IDWriteTextAnalysisSink) || IsEqualIID(riid, &IID_IUnknown))
+    {
+        *obj = iface;
+        return S_OK;
+    }
+
+    *obj = NULL;
+    return E_NOINTERFACE;
+}
+
+static ULONG WINAPI analysissink_AddRef(IDWriteTextAnalysisSink *iface)
+{
+    return 2;
+}
+
+static ULONG WINAPI analysissink_Release(IDWriteTextAnalysisSink *iface)
+{
+    return 1;
+}
+
+static HRESULT WINAPI analysissink_SetScriptAnalysis(IDWriteTextAnalysisSink *iface,
+    UINT32 position, UINT32 length, DWRITE_SCRIPT_ANALYSIS const* sa)
+{
+    g_sa = *sa;
+    return S_OK;
 }
 
+static HRESULT WINAPI analysissink_SetLineBreakpoints(IDWriteTextAnalysisSink *iface,
+    UINT32 position, UINT32 length, DWRITE_LINE_BREAKPOINT const* breakpoints)
+{
+    ok(0, "unexpected call\n");
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI analysissink_SetBidiLevel(IDWriteTextAnalysisSink *iface,
+    UINT32 position, UINT32 length, UINT8 explicitLevel, UINT8 resolvedLevel)
+{
+    ok(0, "unexpected\n");
+    return E_NOTIMPL;
+}
+
+static HRESULT WINAPI analysissink_SetNumberSubstitution(IDWriteTextAnalysisSink *iface,
+    UINT32 position, UINT32 length, IDWriteNumberSubstitution* substitution)
+{
+    ok(0, "unexpected\n");
+    return E_NOTIMPL;
+}
+
+static IDWriteTextAnalysisSinkVtbl analysissinkvtbl = {
+    analysissink_QueryInterface,
+    analysissink_AddRef,
+    analysissink_Release,
+    analysissink_SetScriptAnalysis,
+    analysissink_SetLineBreakpoints,
+    analysissink_SetBidiLevel,
+    analysissink_SetNumberSubstitution
+};
+
+static IDWriteTextAnalysisSink analysissink = { &analysissinkvtbl };
+
+/* test IDWriteTextAnalysisSource */
+static HRESULT WINAPI analysissource_QueryInterface(IDWriteTextAnalysisSource *iface,
+    REFIID riid, void **obj)
+{
+    ok(0, "QueryInterface not expected\n");
+    return E_NOTIMPL;
+}
+
+static ULONG WINAPI analysissource_AddRef(IDWriteTextAnalysisSource *iface)
+{
+    ok(0, "AddRef not expected\n");
+    return 2;
+}
+
+static ULONG WINAPI analysissource_Release(IDWriteTextAnalysisSource *iface)
+{
+    ok(0, "Release not expected\n");
+    return 1;
+}
+
+static const WCHAR *g_source;
+
+static HRESULT WINAPI analysissource_GetTextAtPosition(IDWriteTextAnalysisSource *iface,
+    UINT32 position, WCHAR const** text, UINT32* text_len)
+{
+    if (position >= lstrlenW(g_source))
+    {
+        *text = NULL;
+        *text_len = 0;
+    }
+    else
+    {
+        *text = &g_source[position];
+        *text_len = lstrlenW(g_source) - position;
+    }
+
+    return S_OK;
+}
+
+static HRESULT WINAPI analysissource_GetTextBeforePosition(IDWriteTextAnalysisSource *iface,
+    UINT32 position, WCHAR const** text, UINT32* text_len)
+{
+    ok(0, "unexpected\n");
+    return E_NOTIMPL;
+}
+
+static DWRITE_READING_DIRECTION WINAPI analysissource_GetParagraphReadingDirection(
+    IDWriteTextAnalysisSource *iface)
+{
+    ok(0, "unexpected\n");
+    return DWRITE_READING_DIRECTION_RIGHT_TO_LEFT;
+}
+
+static HRESULT WINAPI analysissource_GetLocaleName(IDWriteTextAnalysisSource *iface,
+    UINT32 position, UINT32* text_len, WCHAR const** locale)
+{
+    *locale = NULL;
+    *text_len = 0;
+    return S_OK;
+}
+
+static HRESULT WINAPI analysissource_GetNumberSubstitution(IDWriteTextAnalysisSource *iface,
+    UINT32 position, UINT32* text_len, IDWriteNumberSubstitution **substitution)
+{
+    ok(0, "unexpected\n");
+    return E_NOTIMPL;
+}
+
+static IDWriteTextAnalysisSourceVtbl analysissourcevtbl = {
+    analysissource_QueryInterface,
+    analysissource_AddRef,
+    analysissource_Release,
+    analysissource_GetTextAtPosition,
+    analysissource_GetTextBeforePosition,
+    analysissource_GetParagraphReadingDirection,
+    analysissource_GetLocaleName,
+    analysissource_GetNumberSubstitution
+};
+
+static IDWriteTextAnalysisSource analysissource = { &analysissourcevtbl };
+
 static IDWriteFactory *create_factory(void)
 {
     IDWriteFactory *factory;
@@ -47,6 +186,35 @@ static IDWriteFactory *create_factory(void)
     return factory;
 }
 
+/* obvious limitation is that only last script data is returned, so this
+   helper is suitable for single script strings only */
+static void get_script_analysis(const WCHAR *str, UINT32 len, DWRITE_SCRIPT_ANALYSIS *sa)
+{
+    IDWriteTextAnalyzer *analyzer;
+    IDWriteFactory *factory;
+    HRESULT hr;
+
+    g_source = str;
+
+    factory = create_factory();
+    hr = IDWriteFactory_CreateTextAnalyzer(factory, &analyzer);
+    ok(hr == S_OK, "got 0x%08x\n", hr);
+
+    hr = IDWriteTextAnalyzer_AnalyzeScript(analyzer, &analysissource, 0, len, &analysissink);
+    ok(hr == S_OK, "got 0x%08x\n", hr);
+
+    *sa = g_sa;
+    IDWriteFactory_Release(factory);
+}
+
+#define EXPECT_REF(obj,ref) _expect_ref((IUnknown*)obj, ref, __LINE__)
+static void _expect_ref(IUnknown* obj, ULONG ref, int line)
+{
+    ULONG rc = IUnknown_AddRef(obj);
+    IUnknown_Release(obj);
+    ok_(__FILE__,line)(rc-1 == ref, "expected refcount %d, got %d\n", ref, rc-1);
+}
+
 enum drawcall_kind {
     DRAW_GLYPHRUN = 0,
     DRAW_UNDERLINE,
@@ -246,11 +414,27 @@ static HRESULT WINAPI testrenderer_DrawGlyphRun(IDWriteTextRenderer *iface,
     FLOAT baselineOriginX,
     FLOAT baselineOriginY,
     DWRITE_MEASURING_MODE mode,
-    DWRITE_GLYPH_RUN const *glyph_run,
-    DWRITE_GLYPH_RUN_DESCRIPTION const *run_descr,
+    DWRITE_GLYPH_RUN const *run,
+    DWRITE_GLYPH_RUN_DESCRIPTION const *descr,
     IUnknown *drawing_effect)
 {
     struct drawcall_entry entry;
+    DWRITE_SCRIPT_ANALYSIS sa;
+
+    /* see what's reported for control codes runs */
+    get_script_analysis(descr->string, descr->stringLength, &sa);
+    if (sa.script == g_control_sa.script) {
+        /* glyphs are not reported at all for control code runs */
+        ok(run->glyphCount == 0, "got %u\n", run->glyphCount);
+        ok(run->glyphAdvances != NULL, "advances array %p\n", run->glyphAdvances);
+        ok(run->glyphOffsets != NULL, "offsets array %p\n", run->glyphOffsets);
+        ok(run->fontFace != NULL, "got %p\n", run->fontFace);
+        /* text positions are still valid */
+        ok(descr->string != NULL, "got string %p\n", descr->string);
+        ok(descr->stringLength > 0, "got string length %u\n", descr->stringLength);
+        ok(descr->clusterMap != NULL, "clustermap %p\n", descr->clusterMap);
+    }
+
     entry.kind = DRAW_GLYPHRUN;
     add_call(sequences, RENDERER_ID, &entry);
     return S_OK;
@@ -875,9 +1059,16 @@ static const struct drawcall_entry draw_seq2[] = {
     { DRAW_LAST_KIND }
 };
 
+static const struct drawcall_entry draw_seq3[] = {
+    { DRAW_GLYPHRUN },
+    { DRAW_GLYPHRUN },
+    { DRAW_LAST_KIND }
+};
+
 static void test_Draw(void)
 {
     static const WCHAR strW[] = {'s','t','r','i','n','g',0};
+    static const WCHAR str2W[] = {0x202a,0x202c,'a','b',0};
     static const WCHAR ruW[] = {'r','u',0};
 
     IDWriteInlineObject *inlineobj;
@@ -932,8 +1123,17 @@ static void test_Draw(void)
     hr = IDWriteTextLayout_Draw(layout, NULL, &testrenderer, 0.0, 0.0);
     ok(hr == S_OK, "got 0x%08x\n", hr);
     ok_sequence(sequences, RENDERER_ID, draw_seq2, "draw test 2", TRUE);
+    IDWriteTextLayout_Release(layout);
 
+    /* string with control characters */
+    hr = IDWriteFactory_CreateTextLayout(factory, str2W, 4, format, 500.0, 100.0, &layout);
+    ok(hr == S_OK, "got 0x%08x\n", hr);
+    flush_sequence(sequences, RENDERER_ID);
+    hr = IDWriteTextLayout_Draw(layout, NULL, &testrenderer, 0.0, 0.0);
+    ok(hr == S_OK, "got 0x%08x\n", hr);
+    ok_sequence(sequences, RENDERER_ID, draw_seq3, "draw test 3", TRUE);
     IDWriteTextLayout_Release(layout);
+
     IDWriteTextFormat_Release(format);
     IDWriteFactory_Release(factory);
 }
@@ -1109,7 +1309,6 @@ todo_wine
     ok(hr == S_OK, "got 0x%08x\n", hr);
     ok(count == 3, "got %u\n", count);
 
-todo_wine
     ok(metrics[0].width == 0.0, "got %.2f\n", metrics[0].width);
     ok(metrics[0].length == 1, "got %d\n", metrics[0].length);
     ok(metrics[0].canWrapLineAfter == 0, "got %d\n", metrics[0].canWrapLineAfter);
@@ -1118,7 +1317,6 @@ todo_wine
     ok(metrics[0].isSoftHyphen == 0, "got %d\n", metrics[0].isSoftHyphen);
     ok(metrics[0].isRightToLeft == 0, "got %d\n", metrics[0].isRightToLeft);
 
-todo_wine
     ok(metrics[1].width == 0.0, "got %.2f\n", metrics[1].width);
     ok(metrics[1].length == 1, "got %d\n", metrics[1].length);
     ok(metrics[1].canWrapLineAfter == 0, "got %d\n", metrics[1].canWrapLineAfter);
@@ -1421,6 +1619,7 @@ static void test_DetermineMinWidth(void)
 
 START_TEST(layout)
 {
+    static const WCHAR ctrlstrW[] = {0x202a,0};
     IDWriteFactory *factory;
 
     if (!(factory = create_factory())) {
@@ -1428,6 +1627,9 @@ START_TEST(layout)
         return;
     }
 
+    /* actual script ids are not fixed */
+    get_script_analysis(ctrlstrW, 1, &g_control_sa);
+
     init_call_sequences(sequences, NUM_CALL_SEQUENCES);
     init_call_sequences(expected_seq, 1);