[PATCH 1/2] dwrite/tests: Add some tests for Segoe UI font family. (Resend)
Signed-off-by: Dmitry Timoshkov <dmitry(a)baikal.ru> --- dlls/dwrite/tests/layout.c | 139 +++++++++++++++++++++++++++++++++++++ 1 file changed, 139 insertions(+) diff --git a/dlls/dwrite/tests/layout.c b/dlls/dwrite/tests/layout.c index 3fe149927ca..c242155cce3 100644 --- a/dlls/dwrite/tests/layout.c +++ b/dlls/dwrite/tests/layout.c @@ -6489,6 +6489,144 @@ if (SUCCEEDED(hr)) IDWriteFactory_Release(factory); } +#define get_font_name(a, b, c) get_font_name_(a, b, c, __LINE__) +static void get_font_name_(IDWriteFont *font, WCHAR *name, UINT32 size, int line) +{ + HRESULT hr; + IDWriteFontFamily *family; + IDWriteLocalizedStrings *names; + UINT32 index; + BOOL exists; + + hr = IDWriteFont_GetFontFamily(font, &family); + ok_(__FILE__, line)(hr == S_OK, "got %#x\n", hr); + + hr = IDWriteFontFamily_GetFamilyNames(family, &names); + ok_(__FILE__, line)(hr == S_OK, "got %#x\n", hr); + + hr = IDWriteLocalizedStrings_FindLocaleName(names, L"en-us", &index, &exists); + ok_(__FILE__, line)(hr == S_OK, "got %#x\n", hr); + ok_(__FILE__, line)(index != UINT_MAX && exists, "name was not found\n"); + hr = IDWriteLocalizedStrings_GetString(names, index, name, size); + ok_(__FILE__, line)(hr == S_OK, "got %#x\n", hr); + + IDWriteLocalizedStrings_Release(names); + IDWriteFontFamily_Release(family); +} + +static void test_SegoeUI(void) +{ + static const WCHAR *families[] = { L"Tahoma", L"Segoe UI", NULL }; + static const WCHAR text[] = { 0x25d4, 0 }; + HRESULT hr; + IDWriteFactory2 *factory; + IDWriteFontCollection *collection; + IDWriteFontFamily *family; + IDWriteFont *font; + IDWriteFontFallback *fallback; + UINT32 index, count, i, mappedlength; + WCHAR name[256]; + BOOL exists, found; + FLOAT scale; + + hr = DWriteCreateFactory(DWRITE_FACTORY_TYPE_SHARED, &IID_IDWriteFactory2, (IUnknown **)&factory); + if (hr != S_OK) + { + win_skip("IDWriteFactory2 is not supported\n"); + return; + } + + hr = IDWriteFactory2_GetSystemFontCollection(factory, &collection, FALSE); + ok(hr == S_OK, "got %#x\n", hr); + + hr = IDWriteFontCollection_FindFamilyName(collection, L"Segoe UI", &index, &exists); + ok(hr == S_OK, "got %#x\n", hr); + if (!exists) + { + skip("Segoe UI is not installed\n"); + IDWriteFontCollection_Release(collection); + IDWriteFactory2_Release(factory); + return; + } + ok(index != UINT_MAX && exists, "Segoe UI was not found\n"); + + hr = IDWriteFontCollection_GetFontFamily(collection, index, &family); + ok(hr == S_OK, "got %#x\n", hr); + + count = IDWriteFontFamily_GetFontCount(family); + trace("family Segoe UI has %u fonts\n", count); + + found = FALSE; + + for (i = 0; i < count; i++) + { + hr = IDWriteFontFamily_GetFont(family, i, &font); + ok(hr == S_OK, "got %#x\n", hr); + + get_font_name(font, name, ARRAY_SIZE(name)); + if (!wcscmp(name, L"Segoe UI Symbol")) + found = TRUE; + + hr = IDWriteFont_HasCharacter(font, 0x25d4, &exists); + ok(hr == S_OK, "got %#x\n", hr); + ok(!exists, "%u: %s has character 0x25d4\n", i, wine_dbgstr_w(name)); + + IDWriteFont_Release(font); + } + + ok(!found, "Segoe UI Symbol should not be part of Segoe UI family\n"); + + IDWriteFontFamily_Release(family); + + hr = IDWriteFactory2_GetSystemFontFallback(factory, &fallback); + ok(hr == S_OK, "got %#x\n", hr); + + g_source = text; + + hr = IDWriteFontFallback_MapCharacters(fallback, &analysissource, 0, 1, collection, L"Segoe UI", DWRITE_FONT_WEIGHT_NORMAL, + DWRITE_FONT_STYLE_NORMAL, DWRITE_FONT_STRETCH_NORMAL, &mappedlength, &font, &scale); + ok(hr == S_OK, "got %#x\n", hr); + ok(mappedlength == 1, "got %u\n", mappedlength); + ok(scale == 1.0f, "got %f\n", scale); + + get_font_name(font, name, ARRAY_SIZE(name)); +todo_wine + ok(!wcscmp(name, L"Segoe UI Symbol"), "got %s\n", wine_dbgstr_w(name)); + + hr = IDWriteFont_HasCharacter(font, 0x25d4, &exists); + ok(hr == S_OK, "got %#x\n", hr); +todo_wine + ok(exists, "%s should have character 0x25d4\n", wine_dbgstr_w(name)); + + IDWriteFont_Release(font); + + for (i = 0; i < ARRAY_SIZE(families); i++) + { + hr = IDWriteFontFallback_MapCharacters(fallback, &analysissource, 0, 1, collection, families[i], DWRITE_FONT_WEIGHT_NORMAL, + DWRITE_FONT_STYLE_NORMAL, DWRITE_FONT_STRETCH_NORMAL, &mappedlength, &font, &scale); +todo_wine_if(families[i] == NULL) + ok(hr == S_OK, "%u: %s - got %#x\n", i, wine_dbgstr_w(families[i]), hr); + if (hr != S_OK) continue; + ok(mappedlength == 1, "got %u\n", mappedlength); + ok(scale == 1.0f, "got %f\n", scale); + + get_font_name(font, name, ARRAY_SIZE(name)); +todo_wine + ok(!wcscmp(name, L"Segoe UI Symbol"), "got %s\n", wine_dbgstr_w(name)); + + hr = IDWriteFont_HasCharacter(font, 0x25d4, &exists); + ok(hr == S_OK, "got %#x\n", hr); +todo_wine + ok(exists, "%s should have character 0x25d4\n", wine_dbgstr_w(name)); + + IDWriteFont_Release(font); + } + + IDWriteFontFallback_Release(fallback); + IDWriteFontCollection_Release(collection); + IDWriteFactory2_Release(factory); +} + START_TEST(layout) { IDWriteFactory *factory; @@ -6544,6 +6682,7 @@ START_TEST(layout) test_text_format_axes(); test_layout_range_length(); test_HitTestTextRange(); + test_SegoeUI(); IDWriteFactory_Release(factory); } -- 2.34.1
Hello, It looks like assigning a reviewer for a patch doesn't work very well. Can the situation be improved somehow? Is there any responsibility of a reviewer for ignoring the assigment? -- Dmitry.
On 2/1/22 12:11, Dmitry Timoshkov wrote:
Hello,
It looks like assigning a reviewer for a patch doesn't work very well. Can the situation be improved somehow? Is there any responsibility of a reviewer for ignoring the assigment?
Since this is exactly the same as the original patches, same concerns expressed initially apply for it as well. I'm planning to work on this, unfortunately can't give exact time frame when it will be done.
Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
It looks like assigning a reviewer for a patch doesn't work very well. Can the situation be improved somehow? Is there any responsibility of a reviewer for ignoring the assigment?
Since this is exactly the same as the original patches, same concerns expressed initially apply for it as well.
I didn't see any real concern. Could you please try to be more specific?
I'm planning to work on this, unfortunately can't give exact time frame when it will be done.
I already suggested a solution for a bunch of real world WPF applications that rely on this behaviour: the test shows exactly what's the problem, and the patch provides the fix. What's wrong with this approach? What would you suggest to do instead? -- Dmitry.
On 2/1/22 22:16, Dmitry Timoshkov wrote:
Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
It looks like assigning a reviewer for a patch doesn't work very well. Can the situation be improved somehow? Is there any responsibility of a reviewer for ignoring the assigment?
Since this is exactly the same as the original patches, same concerns expressed initially apply for it as well. I didn't see any real concern. Could you please try to be more specific?
Everything that I mentioned before - using windows-specific font name both for tests and code changes for no good reason, especially since user won't have it installed; char ranges picked rather arbitrarily with no explanation.
I'm planning to work on this, unfortunately can't give exact time frame when it will be done. I already suggested a solution for a bunch of real world WPF applications that rely on this behaviour: the test shows exactly what's the problem, and the patch provides the fix. What's wrong with this approach? What would you suggest to do instead?
I don't know yet, it's one of those things that you have to spend more time on. For now I'd suggest to use your fix locally, if it works for you.
Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
On 2/1/22 22:16, Dmitry Timoshkov wrote:
Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
It looks like assigning a reviewer for a patch doesn't work very well. Can the situation be improved somehow? Is there any responsibility of a reviewer for ignoring the assigment?
Since this is exactly the same as the original patches, same concerns expressed initially apply for it as well. I didn't see any real concern. Could you please try to be more specific?
Everything that I mentioned before - using windows-specific font name both for tests and code changes for no good reason, especially since user won't have it installed;
That's not a problem at all: font with name Segoe UI Symbol could be added to Wine just like Tahoma, Wingdings, Webdings and others. I've already created such a font with just one glyph - 0x25d4.
char ranges picked rather arbitrarily with no explanation.
I have provided an explanation together with an application that generates complete dwrite fallback ranges. If that helps I'm fine with just adding U+25A0..U+25FF (Geometric Shapes) fallback range that fully fits into Segoe UI Symbol, and is a very small subset covered by that font: https://en.wikipedia.org/wiki/Geometric_Shapes_(Unicode) http://www.unicode-symbol.com/block/Geometric_Shapes.html -- Dmitry.
Dmitry Timoshkov <dmitry(a)baikal.ru> wrote:
Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
On 2/1/22 22:16, Dmitry Timoshkov wrote:
Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
It looks like assigning a reviewer for a patch doesn't work very well. Can the situation be improved somehow? Is there any responsibility of a reviewer for ignoring the assigment?
Since this is exactly the same as the original patches, same concerns expressed initially apply for it as well. I didn't see any real concern. Could you please try to be more specific?
Everything that I mentioned before - using windows-specific font name both for tests and code changes for no good reason, especially since user won't have it installed;
That's not a problem at all: font with name Segoe UI Symbol could be added to Wine just like Tahoma, Wingdings, Webdings and others. I've already created such a font with just one glyph - 0x25d4.
char ranges picked rather arbitrarily with no explanation.
I have provided an explanation together with an application that generates complete dwrite fallback ranges.
If that helps I'm fine with just adding U+25A0..U+25FF (Geometric Shapes) fallback range that fully fits into Segoe UI Symbol, and is a very small subset covered by that font: https://en.wikipedia.org/wiki/Geometric_Shapes_(Unicode) http://www.unicode-symbol.com/block/Geometric_Shapes.html
Aborting the conversation and not answering the questions to you, as a maintainer, is plain rude, isn't it, Nikolay? -- Dmitry.
Dmitry Timoshkov <dmitry(a)baikal.ru> wrote:
Dmitry Timoshkov <dmitry(a)baikal.ru> wrote:
Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
On 2/1/22 22:16, Dmitry Timoshkov wrote:
Nikolay Sivov <nsivov(a)codeweavers.com> wrote:
It looks like assigning a reviewer for a patch doesn't work very well. Can the situation be improved somehow? Is there any responsibility of a reviewer for ignoring the assigment?
Since this is exactly the same as the original patches, same concerns expressed initially apply for it as well. I didn't see any real concern. Could you please try to be more specific?
Everything that I mentioned before - using windows-specific font name both for tests and code changes for no good reason, especially since user won't have it installed;
That's not a problem at all: font with name Segoe UI Symbol could be added to Wine just like Tahoma, Wingdings, Webdings and others. I've already created such a font with just one glyph - 0x25d4.
char ranges picked rather arbitrarily with no explanation.
I have provided an explanation together with an application that generates complete dwrite fallback ranges.
If that helps I'm fine with just adding U+25A0..U+25FF (Geometric Shapes) fallback range that fully fits into Segoe UI Symbol, and is a very small subset covered by that font: https://en.wikipedia.org/wiki/Geometric_Shapes_(Unicode) http://www.unicode-symbol.com/block/Geometric_Shapes.html
Aborting the conversation and not answering the questions to you, as a maintainer, is plain rude, isn't it, Nikolay?
Please find another reviewer for these patches. -- Dmitry.
participants (2)
-
Dmitry Timoshkov -
Nikolay Sivov