Re: [PATCH 2/9] dwrite/tests: Test GetMetrics with custom fontcollection

[Nikolay Sivov]

On 3/4/21 1:23 PM, Giovanni Mascellani wrote:
> From: Lucian Poston
> Signed-off-by: Giovanni Mascellani
>
> dlls/dwrite/tests/layout.c | 263 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 263 insertions(+)
>
> diff --git a/dlls/dwrite/tests/layout.c b/dlls/dwrite/tests/layout.c
> index 35d54ae41f0..df2cdb36f1c 100644
> --- a/dlls/dwrite/tests/layout.c
> +++ b/dlls/dwrite/tests/layout.c
> @@ -4538,6 +4538,7 @@ static void test_SetWordWrapping(void)
>  /* Collection dedicated to fallback testing */
>
> static const WCHAR g_blahfontW[] = {'B','l','a','h',0};
> +static const WCHAR g_fontNotInCollectionW[] = {'n','o','t','B','l','a','h',0};
>  static HRESULT WINAPI fontcollection_QI(IDWriteFontCollection *iface, REFIID riid, void **obj)
>  {
>      if (IsEqualIID(riid, &IID_IDWriteFontCollection) || IsEqualIID(riid, &IID_IUnknown)) {
> @@ -4599,6 +4600,9 @@ static HRESULT WINAPI fontcollection_FindFamilyName(IDWriteFontCollection *iface
>          *index = 123456;
>          *exists = TRUE;
>          return S_OK;
> - } else if (!lstrcmpW(name, g_fontNotInCollectionW)) {
> -
>    *exists = FALSE;
> -
>    return S_OK;
> }
>    ok(0, "unexpected call, name %s\n", wine_dbgstr_w(name));
>    return E_NOTIMPL;
>
> @@ -6424,6 +6428,264 @@ static void test_layout_range_length(void)
>      IDWriteTextLayout_Release(layout);
>  IDWriteTextFormat_Release(format);
> -
> - IDWriteFactory_Release(factory);
>
> +}
> -
>
> +static void test_GetMetrics_with_custom_fontcollection(void)

That's not how existing functions are structured.
> +{
> - static const WCHAR emptystringW[] = {0};
> - static const WCHAR mappedW[] = {'a','b','c','d',0};
> - static const WCHAR notmappedW[] = {'a',0xffff,'b',0}; // u+ffff = not a unicode character

I don't see what's special about 0xffff.
> - DWRITE_CLUSTER_METRICS clusters[4];
> - DWRITE_TEXT_METRICS metrics;
> - IDWriteTextFormat *format;
> - IDWriteTextLayout *layout;
> - IDWriteFactory *factory;
> - UINT32 count, i;
> - FLOAT width;
> - HRESULT hr;
> -
> - factory = create_factory();
> -
> - /* font is in font collection */
> - hr = IDWriteFactory_CreateTextFormat(factory, g_blahfontW, &fallbackcollection,
> -
>        DWRITE_FONT_WEIGHT_NORMAL, DWRITE_FONT_STYLE_NORMAL,
> -
>        DWRITE_FONT_STRETCH_NORMAL, 10.0, L"en-us", &format);
> - ok(hr == S_OK, "got 0x%08x\n", hr);
> -
> - /* text is mapped by fontfallback */
> - hr = IDWriteFactory_CreateTextLayout(factory, mappedW, 4, format, 1000.0, 1000.0, &layout);
> - ok(hr == S_OK, "got 0x%08x\n", hr);
> - count = 9999;
> - hr = IDWriteTextLayout_GetClusterMetrics(layout, clusters, 4, &count);
> - ok(hr == S_OK, "got 0x%08x\n", hr);
> - ok(count == 4, "got %u\n", count);
> - width = 0.0;
> - if (hr == S_OK)
> -
>    for (i = 0; i < count; i++)
> -
>        width += clusters[i].width;
> - memset(&metrics, 0xcc, sizeof(metrics));
> - hr = IDWriteTextLayout_GetMetrics(layout, &metrics);
> - ok(hr == S_OK, "got 0x%08x\n", hr);
> - ok(metrics.left == 0.0, "got %.2f\n", metrics.left);
> - ok(metrics.top == 0.0, "got %.2f\n", metrics.top);
> - ok(metrics.width == width, "got %.2f, expected %.2f\n", metrics.width, width);
> - ok(metrics.widthIncludingTrailingWhitespace == width, "got %.2f, expected %.2f\n",
> -
>    metrics.widthIncludingTrailingWhitespace, width);
> - ok(metrics.height > 0.0, "got %.2f\n", metrics.height);
> - ok(metrics.layoutWidth == 1000.0, "got %.2f\n", metrics.layoutWidth);
> - ok(metrics.layoutHeight == 1000.0, "got %.2f\n", metrics.layoutHeight);
> - ok(metrics.maxBidiReorderingDepth == 1, "got %u\n", metrics.maxBidiReorderingDepth);
> - ok(metrics.lineCount == 1, "got %u\n", metrics.lineCount);
> - IDWriteTextLayout_Release(layout);
> -
> - /* text is not mapped by fontfallback */
> - hr = IDWriteFactory_CreateTextLayout(factory, notmappedW, 4, format, 1000.0, 1000.0, &layout);
> - ok(hr == S_OK, "got 0x%08x\n", hr);
> - count = 9999;
> - hr = IDWriteTextLayout_GetClusterMetrics(layout, clusters, 4, &count);
> - ok(hr == S_OK, "got 0x%08x\n", hr);
> - ok(count == 4, "got %u\n", count);
> - width = 0.0;
> - if (hr == S_OK)
> -
>    for (i = 0; i < count; i++)
> -
>        width += clusters[i].width;
> - memset(&metrics, 0xcc, sizeof(metrics));
> - hr = IDWriteTextLayout_GetMetrics(layout, &metrics);
> - ok(hr == S_OK, "got 0x%08x\n", hr);
> - ok(metrics.left == 0.0, "got %.2f\n", metrics.left);
> - ok(metrics.top == 0.0, "got %.2f\n", metrics.top);
> - ok(metrics.width == width, "got %.2f, expected %.2f\n", metrics.width, width);
> - ok(metrics.widthIncludingTrailingWhitespace == width, "got %.2f, expected %.2f\n",
> -
>    metrics.widthIncludingTrailingWhitespace, width);
> - ok(metrics.height > 0.0, "got %.2f\n", metrics.height);
> - ok(metrics.layoutWidth == 1000.0, "got %.2f\n", metrics.layoutWidth);
> - ok(metrics.layoutHeight == 1000.0, "got %.2f\n", metrics.layoutHeight);
> - ok(metrics.maxBidiReorderingDepth == 1, "got %u\n", metrics.maxBidiReorderingDepth);
> - ok(metrics.lineCount == 1, "got %u\n", metrics.lineCount);
> - IDWriteTextLayout_Release(layout);
> -
> - /* empty string */
> - hr = IDWriteFactory_CreateTextLayout(factory, emptystringW, 4, format, 1000.0, 1000.0, &layout);
> - ok(hr == S_OK, "got 0x%08x\n", hr);
> - count = 9999;
> - hr = IDWriteTextLayout_GetClusterMetrics(layout, clusters, 4, &count);
> - ok(hr == S_OK, "got 0x%08x\n", hr);
> - ok(count == 4, "got %u\n", count);
> - width = 0.0;
> - if (hr == S_OK)
> -
>    for (i = 0; i < count; i++)
> -
>        width += clusters[i].width;
> - memset(&metrics, 0xcc, sizeof(metrics));
> - hr = IDWriteTextLayout_GetMetrics(layout, &metrics);
> - ok(hr == S_OK, "got 0x%08x\n", hr);
> - ok(metrics.left == 0.0, "got %.2f\n", metrics.left);
> - ok(metrics.top == 0.0, "got %.2f\n", metrics.top);
> - ok(metrics.width == width, "got %.2f, expected %.2f\n", metrics.width, width);
> - ok(metrics.widthIncludingTrailingWhitespace == width, "got %.2f, expected %.2f\n",
> -
>    metrics.widthIncludingTrailingWhitespace, width);
> - ok(metrics.height > 0.0, "got %.2f\n", metrics.height);
> - ok(metrics.layoutWidth == 1000.0, "got %.2f\n", metrics.layoutWidth);
> - ok(metrics.layoutHeight == 1000.0, "got %.2f\n", metrics.layoutHeight);
> - ok(metrics.maxBidiReorderingDepth == 1, "got %u\n", metrics.maxBidiReorderingDepth);
> - ok(metrics.lineCount == 1, "got %u\n", metrics.lineCount);
> - IDWriteTextLayout_Release(layout);
> -
> - /* zero-length empty string */
> - hr = IDWriteFactory_CreateTextLayout(factory, emptystringW, 0, format, 1000.0, 1000.0, &layout);
> - ok(hr == S_OK, "got 0x%08x\n", hr);
> - count = 9999;
> - hr = IDWriteTextLayout_GetClusterMetrics(layout, clusters, 4, &count);
> - ok(hr == S_OK, "got 0x%08x\n", hr);
> - ok(count == 0, "got %u\n", count);
> - width = 0.0;
> - if (hr == S_OK)
> -
>    for (i = 0; i < count; i++)
> -
>        width += clusters[i].width;
> - memset(&metrics, 0xcc, sizeof(metrics));
> - hr = IDWriteTextLayout_GetMetrics(layout, &metrics);
> - ok(hr == S_OK, "got 0x%08x\n", hr);
> - ok(metrics.left == 0.0, "got %.2f\n", metrics.left);
> - ok(metrics.top == 0.0, "got %.2f\n", metrics.top);
> - ok(metrics.width == width, "got %.2f, expected %.2f\n", metrics.width, width);
> - ok(metrics.widthIncludingTrailingWhitespace == width, "got %.2f, expected %.2f\n",
> -
>    metrics.widthIncludingTrailingWhitespace, width);
> - ok(metrics.height > 0.0, "got %.2f\n", metrics.height);
> - ok(metrics.layoutWidth == 1000.0, "got %.2f\n", metrics.layoutWidth);
> - ok(metrics.layoutHeight == 1000.0, "got %.2f\n", metrics.layoutHeight);
> - ok(metrics.maxBidiReorderingDepth == 1, "got %u\n", metrics.maxBidiReorderingDepth);
> - ok(metrics.lineCount == 1, "got %u\n", metrics.lineCount);
> - IDWriteTextLayout_Release(layout);
> -
> - IDWriteTextFormat_Release(format);
> -
> - /* font not in font collection */
> - hr = IDWriteFactory_CreateTextFormat(factory, g_fontNotInCollectionW, &fallbackcollection,
> -
>        DWRITE_FONT_WEIGHT_NORMAL, DWRITE_FONT_STYLE_NORMAL,
> -
>        DWRITE_FONT_STRETCH_NORMAL, 10.0, L"en-us", &format);
> - ok(hr == S_OK, "got 0x%08x\n", hr);
> -
> - /* text is mapped by fontfallback */
> - hr = IDWriteFactory_CreateTextLayout(factory, mappedW, 4, format, 1000.0, 1000.0, &layout);
> - 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);
> - ok(count == 4, "got %u\n", count);
>
> +}
> - width = 0.0;
> - if (hr == S_OK)
> -
>    for (i = 0; i < count; i++)
> -
>        width += clusters[i].width;
> - memset(&metrics, 0xcc, sizeof(metrics));
> - hr = IDWriteTextLayout_GetMetrics(layout, &metrics);
> - ok(hr == S_OK, "got 0x%08x\n", hr);
> - ok(metrics.left == 0.0, "got %.2f\n", metrics.left);
> - ok(metrics.top == 0.0, "got %.2f\n", metrics.top);
> - ok(metrics.width == width, "got %.2f, expected %.2f\n", metrics.width, width);
> - ok(metrics.widthIncludingTrailingWhitespace == width, "got %.2f, expected %.2f\n",
> -
>    metrics.widthIncludingTrailingWhitespace, width);
>
> +todo_wine
> - ok(metrics.height > 0.0, "got %.2f\n", metrics.height);
> - ok(metrics.layoutWidth == 1000.0, "got %.2f\n", metrics.layoutWidth);
> - ok(metrics.layoutHeight == 1000.0, "got %.2f\n", metrics.layoutHeight);
> - ok(metrics.maxBidiReorderingDepth == 1, "got %u\n", metrics.maxBidiReorderingDepth);
>
> +todo_wine
> - ok(metrics.lineCount == 1, "got %u\n", metrics.lineCount);
> - IDWriteTextLayout_Release(layout);
> -
> - /* text is not mapped by fontfallback */
> - hr = IDWriteFactory_CreateTextLayout(factory, notmappedW, 4, format, 1000.0, 1000.0, &layout);
> - 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);
> - ok(count == 4, "got %u\n", count);
>
> +}
> - width = 0.0;
> - if (hr == S_OK)
> -
>    for (i = 0; i < count; i++)
> -
>        width += clusters[i].width;
> - memset(&metrics, 0xcc, sizeof(metrics));
> - hr = IDWriteTextLayout_GetMetrics(layout, &metrics);
> - ok(hr == S_OK, "got 0x%08x\n", hr);
> - ok(metrics.left == 0.0, "got %.2f\n", metrics.left);
> - ok(metrics.top == 0.0, "got %.2f\n", metrics.top);
> - ok(metrics.width == width, "got %.2f, expected %.2f\n", metrics.width, width);
> - ok(metrics.widthIncludingTrailingWhitespace == width, "got %.2f, expected %.2f\n",
> -
>    metrics.widthIncludingTrailingWhitespace, width);
>
> +todo_wine
> - ok(metrics.height > 0.0, "got %.2f\n", metrics.height);
> - ok(metrics.layoutWidth == 1000.0, "got %.2f\n", metrics.layoutWidth);
> - ok(metrics.layoutHeight == 1000.0, "got %.2f\n", metrics.layoutHeight);
> - ok(metrics.maxBidiReorderingDepth == 1, "got %u\n", metrics.maxBidiReorderingDepth);
>
> +todo_wine
> - ok(metrics.lineCount == 1, "got %u\n", metrics.lineCount);
> - IDWriteTextLayout_Release(layout);
> -
> - /* empty string */
> - hr = IDWriteFactory_CreateTextLayout(factory, emptystringW, 4, format, 1000.0, 1000.0, &layout);
> - 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);
> - ok(count == 4, "got %u\n", count);
>
> +}
> - width = 0.0;
> - if (hr == S_OK)
> -
>    for (i = 0; i < count; i++)
> -
>        width += clusters[i].width;
> - memset(&metrics, 0xcc, sizeof(metrics));
> - hr = IDWriteTextLayout_GetMetrics(layout, &metrics);
> - ok(hr == S_OK, "got 0x%08x\n", hr);
> - ok(metrics.left == 0.0, "got %.2f\n", metrics.left);
> - ok(metrics.top == 0.0, "got %.2f\n", metrics.top);
> - ok(metrics.width == width, "got %.2f, expected %.2f\n", metrics.width, width);
> - ok(metrics.widthIncludingTrailingWhitespace == width, "got %.2f, expected %.2f\n",
> -
>    metrics.widthIncludingTrailingWhitespace, width);
>
> +todo_wine
> - ok(metrics.height > 0.0, "got %.2f\n", metrics.height);
> - ok(metrics.layoutWidth == 1000.0, "got %.2f\n", metrics.layoutWidth);
> - ok(metrics.layoutHeight == 1000.0, "got %.2f\n", metrics.layoutHeight);
> - ok(metrics.maxBidiReorderingDepth == 1, "got %u\n", metrics.maxBidiReorderingDepth);
>
> +todo_wine
> - ok(metrics.lineCount == 1, "got %u\n", metrics.lineCount);
> - IDWriteTextLayout_Release(layout);
> -
> - /* zero-length empty string */
> - hr = IDWriteFactory_CreateTextLayout(factory, emptystringW, 0, format, 1000.0, 1000.0, &layout);
> - ok(hr == S_OK, "got 0x%08x\n", hr);
> - count = 9999;
> - hr = IDWriteTextLayout_GetClusterMetrics(layout, clusters, 4, &count);
> - ok(hr == S_OK, "got 0x%08x\n", hr);
> - ok(count == 0, "got %u\n", count);
> - width = 0.0;
> - if (hr == S_OK)
> -
>    for (i = 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);
> - ok(metrics.left == 0.0, "got %.2f\n", metrics.left);
> - ok(metrics.top == 0.0, "got %.2f\n", metrics.top);
> - ok(metrics.width == width, "got %.2f, expected %.2f\n", metrics.width, width);
> - ok(metrics.widthIncludingTrailingWhitespace == width, "got %.2f, expected %.2f\n",
> -
>    metrics.widthIncludingTrailingWhitespace, width);
> - ok(metrics.height > 0.0, "got %.2f\n", metrics.height);
> - ok(metrics.layoutWidth == 1000.0, "got %.2f\n", metrics.layoutWidth);
> - ok(metrics.layoutHeight == 1000.0, "got %.2f\n", metrics.layoutHeight);
> - ok(metrics.maxBidiReorderingDepth == 1, "got %u\n", metrics.maxBidiReorderingDepth);
> - ok(metrics.lineCount == 1, "got %u\n", metrics.lineCount);
>
> +}
> - IDWriteTextLayout_Release(layout);
> -
> - IDWriteTextFormat_Release(format);
> - IDWriteFactory_Release(factory);
>
> }
>
> @@ -6460,6 +6722,7 @@ START_TEST(layout)
>      test_SetFontStretch();
>      test_SetStrikethrough();
>      test_GetMetrics();
> - test_GetMetrics_with_custom_fontcollection();
> test_SetFlowDirection();
> test_SetDrawingEffect();
> test_GetLineMetrics();

All of that does not look closely related to fallback. We already use
format collection if specified. And again, CreateTextLayout() arguments
are occasionally incorrect.