Re: [PATCH 5/6] dwrite: Use MapCharacters for non-visual characters

[Nikolay Sivov]

On 05/23/2018 09:07 PM, Lucian Poston wrote:
> Signed-off-by: Lucian Poston
>
> dlls/dwrite/layout.c       | 22 +++++++++++++++-------
>   dlls/dwrite/tests/layout.c | 24 ------------------------
>   2 files changed, 15 insertions(+), 31 deletions(-)
>
> diff --git a/dlls/dwrite/layout.c b/dlls/dwrite/layout.c
> index df3f3beabb..5d06bf9348 100644
> --- a/dlls/dwrite/layout.c
> +++ b/dlls/dwrite/layout.c
> @@ -822,7 +822,8 @@ static HRESULT layout_resolve_fonts(struct dwrite_textlayout *layout)
>       LIST_FOR_EACH_ENTRY(r, &layout->runs, struct layout_run, entry) {
>           struct regular_layout_run *run = &r->u.regular;
>           IDWriteFont *font;
> -
>    UINT32 length;
> -
>    UINT32 length, mapped_length;
> -
>    FLOAT scale;
>
>     if (r->kind == LAYOUT_RUN_INLINE)
>         continue;
>
> @@ -830,12 +831,19 @@ static HRESULT layout_resolve_fonts(struct dwrite_textlayout *layout)
>           range = get_layout_range_by_pos(layout, run->descr.textPosition);
>       if (run->sa.shapes == DWRITE_SCRIPT_SHAPES_NO_VISUAL) {
> -
>        IDWriteFontCollection *collection;
> -
> -
>        collection = range->collection ? range->collection : sys_collection;
> -
> -
>        if (FAILED(hr = create_matching_font(collection, range->fontfamily, range->weight, range->style,
> -
>                range->stretch, &font))) {
> -
>        hr = IDWriteFontFallback_MapCharacters(fallback,
> -
>            (IDWriteTextAnalysisSource *)&layout->IDWriteTextAnalysisSource1_iface,
> -
>            run->descr.textPosition,
> -
>            run->descr.stringLength,
> -
>            range->collection,
> -
>            range->fontfamily,
> -
>            range->weight,
> -
>            range->style,
> -
>            range->stretch,
> -
>            &mapped_length,
> -
>            &font,
> -
>            &scale);
> -
>        if (FAILED(hr)) {
>             WARN("%s: failed to create matching font for non visual run, family %s, collection %p\n",
>                     debugstr_rundescr(&run->descr), debugstr_w(range->fontfamily), range->collection);
>             break;

This only works together with patch 4/6, right? What we need to figure 
out here is which font is picked in NO_VISUAL case, does it change with 
range locale,
or custom collection. Then we can either hardcode it here, or add 
corresponding char ranges.
> diff --git a/dlls/dwrite/tests/layout.c b/dlls/dwrite/tests/layout.c
> index 85967e7ee8..975b6ac7b3 100644
> --- a/dlls/dwrite/tests/layout.c
> +++ b/dlls/dwrite/tests/layout.c
> @@ -5715,34 +5715,22 @@ static void test_GetMetrics_with_custom_fontcollection(void)
>       ok(hr == S_OK, "got 0x%08x\n", hr);
>       count = 9999;
>       hr = IDWriteTextLayout_GetClusterMetrics(layout, clusters, 4, &count);
> - todo_wine
>  ok(hr == S_OK, "got 0x%08x\n", hr);
> - todo_wine
>  ok(count == 4, "got %u\n", count);
>  for (i = 0, width = 0.0; i < count; i++)
>      width += clusters[i].width;
>  memset(&metrics, 0xcc, sizeof(metrics));
>  hr = IDWriteTextLayout_GetMetrics(layout, &metrics);
> - todo_wine
>  ok(hr == S_OK, "got 0x%08x\n", hr);
> - todo_wine
>  ok(metrics.left == 0.0, "got %.2f\n", metrics.left);
> - todo_wine
>  ok(metrics.top == 0.0, "got %.2f\n", metrics.top);
> - todo_wine
>  ok(metrics.width == width, "got %.2f, expected %.2f\n", metrics.width, width);
> - todo_wine
>  ok(metrics.widthIncludingTrailingWhitespace == width, "got %.2f, expected %.2f\n",
>      metrics.widthIncludingTrailingWhitespace, width);
> - todo_wine
>  ok(metrics.height > 0.0, "got %.2f\n", metrics.height);
> - todo_wine
>  ok(metrics.layoutWidth == 1000.0, "got %.2f\n", metrics.layoutWidth);
> - todo_wine
>  ok(metrics.layoutHeight == 1000.0, "got %.2f\n", metrics.layoutHeight);
> - todo_wine
>  ok(metrics.maxBidiReorderingDepth == 1, "got %u\n", metrics.maxBidiReorderingDepth);
> - todo_wine
>  ok(metrics.lineCount == 1, "got %u\n", metrics.lineCount);
>  IDWriteTextLayout_Release(layout);
>
> @@ -5751,34 +5739,22 @@ static void test_GetMetrics_with_custom_fontcollection(void)
>       ok(hr == S_OK, "got 0x%08x\n", hr);
>       count = 9999;
>       hr = IDWriteTextLayout_GetClusterMetrics(layout, clusters, 4, &count);
> - todo_wine
>  ok(hr == S_OK, "got 0x%08x\n", hr);
> - todo_wine
>  ok(count == 4, "got %u\n", count);
>  for (i = 0, width = 0.0; i < count; i++)
>      width += clusters[i].width;
>  memset(&metrics, 0xcc, sizeof(metrics));
>  hr = IDWriteTextLayout_GetMetrics(layout, &metrics);
> - todo_wine
>  ok(hr == S_OK, "got 0x%08x\n", hr);
> - todo_wine
>  ok(metrics.left == 0.0, "got %.2f\n", metrics.left);
> - todo_wine
>  ok(metrics.top == 0.0, "got %.2f\n", metrics.top);
> - todo_wine
>  ok(metrics.width == width, "got %.2f, expected %.2f\n", metrics.width, width);
> - todo_wine
>  ok(metrics.widthIncludingTrailingWhitespace == width, "got %.2f, expected %.2f\n",
>      metrics.widthIncludingTrailingWhitespace, width);
> - todo_wine
>  ok(metrics.height > 0.0, "got %.2f\n", metrics.height);
> - todo_wine
>  ok(metrics.layoutWidth == 1000.0, "got %.2f\n", metrics.layoutWidth);
> - todo_wine
>  ok(metrics.layoutHeight == 1000.0, "got %.2f\n", metrics.layoutHeight);
> - todo_wine
>  ok(metrics.maxBidiReorderingDepth == 1, "got %u\n", metrics.maxBidiReorderingDepth);
> - todo_wine
>  ok(metrics.lineCount == 1, "got %u\n", metrics.lineCount);
>  IDWriteTextLayout_Release(layout);