[PATCH 0/1] MR3744: dwrite: Treat null character as a C0 control character in script analyzer.

[Jeffrey Smith (＠whydoubt)]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55529

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3744

[PATCH 1/1] dwrite: Treat null character as a C0 control character in script analyzer.

[Jeff Smith]

From: Jeff Smith <whydoubt(a)gmail.com>

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=55529
---
 dlls/dwrite/analyzer.c       |  2 +-
 dlls/dwrite/tests/analyzer.c | 62 ++++++++++++++++++++++++++++++++----
 2 files changed, 56 insertions(+), 8 deletions(-)

diff --git a/dlls/dwrite/analyzer.c b/dlls/dwrite/analyzer.c
index 4e503eb8b5c..3c8e25d8efe 100644
--- a/dlls/dwrite/analyzer.c
+++ b/dlls/dwrite/analyzer.c
@@ -658,7 +658,7 @@ static DWRITE_SCRIPT_ANALYSIS get_char_sa(UINT32 c)
 
     sa.script = get_char_script(c);
     sa.shapes = DWRITE_SCRIPT_SHAPES_DEFAULT;
-    if ((c >= 0x0001 && c <= 0x001f)           /* C0 controls */
+    if ((c <= 0x001f)                          /* C0 controls */
             || (c >= 0x007f && c <= 0x009f)    /* DELETE, C1 controls */
             || (c == 0x00ad)                   /* SOFT HYPHEN */
             || (c >= 0x200b && c <= 0x200f)    /* ZWSP, ZWNJ, ZWJ, LRM, RLM */
diff --git a/dlls/dwrite/tests/analyzer.c b/dlls/dwrite/tests/analyzer.c
index fbe7044e402..5ddb8933e50 100644
--- a/dlls/dwrite/tests/analyzer.c
+++ b/dlls/dwrite/tests/analyzer.c
@@ -420,10 +420,10 @@ struct testanalysissource
 };
 
 static void init_textsource(struct testanalysissource *source, const WCHAR *text,
-        DWRITE_READING_DIRECTION direction)
+        INT text_length, DWRITE_READING_DIRECTION direction)
 {
     source->text = text;
-    source->text_length = lstrlenW(text);
+    source->text_length = text_length == -1 ? lstrlenW(text) : text_length;
     source->direction = direction;
 };
 
@@ -1069,7 +1069,7 @@ static void get_script_analysis(const WCHAR *str, DWRITE_SCRIPT_ANALYSIS *sa)
     IDWriteTextAnalyzer *analyzer;
     HRESULT hr;
 
-    init_textsource(&analysissource, str, DWRITE_READING_DIRECTION_LEFT_TO_RIGHT);
+    init_textsource(&analysissource, str, -1, DWRITE_READING_DIRECTION_LEFT_TO_RIGHT);
 
     analyzer = create_text_analyzer(&IID_IDWriteTextAnalyzer);
     ok(!!analyzer, "Failed to create analyzer instance.\n");
@@ -1094,7 +1094,7 @@ static void test_AnalyzeScript(void)
 
     while (*ptr->string)
     {
-        init_textsource(&analysissource, ptr->string, DWRITE_READING_DIRECTION_LEFT_TO_RIGHT);
+        init_textsource(&analysissource, ptr->string, -1, DWRITE_READING_DIRECTION_LEFT_TO_RIGHT);
 
         winetest_push_context("Test %s", debugstr_w(ptr->string));
 
@@ -1111,6 +1111,53 @@ static void test_AnalyzeScript(void)
     IDWriteTextAnalyzer_Release(analyzer);
 }
 
+static void test_AnalyzeScript_null(void)
+{
+    struct {
+        int str_len;
+        struct sa_test sa;
+    } td[] =
+    {
+        { 5, { L"\0test", 2, {
+            { 0, 1, DWRITE_SCRIPT_SHAPES_NO_VISUAL },
+            { 1, 4, DWRITE_SCRIPT_SHAPES_DEFAULT },
+        } } },
+        { 5, { L"te\0st", 3, {
+            { 0, 2, DWRITE_SCRIPT_SHAPES_DEFAULT },
+            { 2, 1, DWRITE_SCRIPT_SHAPES_NO_VISUAL },
+            { 3, 2, DWRITE_SCRIPT_SHAPES_DEFAULT },
+        } } },
+        { 5, { L"test\0", 2, {
+            { 0, 4, DWRITE_SCRIPT_SHAPES_DEFAULT },
+            { 4, 1, DWRITE_SCRIPT_SHAPES_NO_VISUAL },
+        } } },
+    };
+    IDWriteTextAnalyzer *analyzer;
+    HRESULT hr;
+    UINT i;
+
+    analyzer = create_text_analyzer(&IID_IDWriteTextAnalyzer);
+    ok(!!analyzer, "Failed to create analyzer instance.\n");
+
+    for (i = 0; i < ARRAY_SIZE(td); i++)
+    {
+        init_textsource(&analysissource, td[i].sa.string, td[i].str_len, DWRITE_READING_DIRECTION_LEFT_TO_RIGHT);
+
+        winetest_push_context("Test %s", wine_dbgstr_wn(td[i].sa.string, td[i].str_len));
+
+        init_expected_sa(expected_seq, &td[i].sa);
+        hr = IDWriteTextAnalyzer_AnalyzeScript(analyzer, &analysissource.IDWriteTextAnalysisSource_iface, 0,
+            td[i].str_len, &analysissink);
+        ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
+        ok_sequence(sequences, ANALYZER_ID, expected_seq[0]->sequence,
+            wine_dbgstr_wn(td[i].sa.string, td[i].str_len), FALSE);
+
+        winetest_pop_context();
+    }
+
+    IDWriteTextAnalyzer_Release(analyzer);
+}
+
 struct linebreaks_test {
     const WCHAR text[BREAKPOINT_COUNT+1];
     DWRITE_LINE_BREAKPOINT bp[BREAKPOINT_COUNT];
@@ -1208,7 +1255,7 @@ static void test_AnalyzeLineBreakpoints(void)
     analyzer = create_text_analyzer(&IID_IDWriteTextAnalyzer);
     ok(!!analyzer, "Failed to create analyzer instance.\n");
 
-    init_textsource(&analysissource, L"", DWRITE_READING_DIRECTION_LEFT_TO_RIGHT);
+    init_textsource(&analysissource, L"", 0, DWRITE_READING_DIRECTION_LEFT_TO_RIGHT);
     hr = IDWriteTextAnalyzer_AnalyzeLineBreakpoints(analyzer, &analysissource.IDWriteTextAnalysisSource_iface, 0, 0,
         &analysissink);
     ok(hr == S_OK, "Unexpected hr %#lx.\n", hr);
@@ -1217,7 +1264,7 @@ static void test_AnalyzeLineBreakpoints(void)
     {
         UINT32 len;
 
-        init_textsource(&analysissource, ptr->text, DWRITE_READING_DIRECTION_LEFT_TO_RIGHT);
+        init_textsource(&analysissource, ptr->text, -1, DWRITE_READING_DIRECTION_LEFT_TO_RIGHT);
 
         len = lstrlenW(ptr->text);
         if (len > BREAKPOINT_COUNT) {
@@ -2806,7 +2853,7 @@ static void test_AnalyzeBidi(void)
     {
         UINT32 len;
 
-        init_textsource(&analysissource, ptr->text, ptr->direction);
+        init_textsource(&analysissource, ptr->text, -1, ptr->direction);
 
         len = lstrlenW(ptr->text);
         if (len > BIDI_LEVELS_COUNT) {
@@ -2955,6 +3002,7 @@ START_TEST(analyzer)
     init_call_sequences(expected_seq, 1);
 
     test_AnalyzeScript();
+    test_AnalyzeScript_null();
     test_AnalyzeLineBreakpoints();
     test_AnalyzeBidi();
     test_GetScriptProperties();
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/3744

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/dwrite/tests/analyzer.c:
> init_call_sequences(expected_seq, 1);
>
> test_AnalyzeScript();
> +    test_AnalyzeScript_null();
>      test_AnalyzeLineBreakpoints();

Just add it to the existing function.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3744#note_44439

[Nikolay Sivov (＠nsivov)]

Nikolay Sivov (@nsivov) commented about dlls/dwrite/analyzer.c:
> sa.script = get_char_script(c);
>      sa.shapes = DWRITE_SCRIPT_SHAPES_DEFAULT;
> -    if ((c >= 0x0001 && c <= 0x001f)           /* C0 controls */
> +    if ((c <= 0x001f)                          /* C0 controls */

Please remove parentheses.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3744#note_44440

[Nikolay Sivov (＠nsivov)]

It's better to update the commit message, this change is not about C0 range, but more about setting NON_VISUAL attribute correctly.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3744#note_44441

[Jeffrey Smith (＠whydoubt)]

On Wed Sep  6 16:46:36 2023 +0000, Nikolay Sivov wrote:
> Please remove parentheses.
> Are you sure? It appears to be inconsistent with the styling for the rest of the if condition. All other or-separated terms are using parenthesis, regardless of whether they contain an &&.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3744#note_44447