Repository: kyamagu/skia-python (language: python)

Issue:
Skparagraph does not properly handle newlines on windows
**Describe the bug**

When given text that has newlines in it, Skparagraph fails to render the newlines and instead renders them as unknown characters.
![test](https://github.com/user-attachments/assets/3612d143-278c-438d-adde-c9e286515831)

But it works on linux:
![test](https://github.com/user-attachments/assets/0f69f683-9978-4493-aa68-c61ce687c08d)

I‘m not sure if it's an upstream problem, but when I tried it with rust-skia, it worked on both windows and linux.


**To Reproduce**

Code:
```python
import math

import skia
from skia import textlayout

paint = skia.Paint()
paint.setColor(skia.ColorBLACK)
paint.setAntiAlias(True)

style = textlayout.TextStyle()
style.setFontSize(50)
style.setForegroundPaint(paint)

font_collection = textlayout.FontCollection()
font_collection.setDefaultFontManager(skia.FontMgr())

para_style = textlayout.ParagraphStyle()

builder = textlayout.ParagraphBuilder.make(
    para_style, font_collection, skia.Unicodes.ICU.Make()
)
builder.pushStyle(style)

builder.addText("test\ntest")
paragraph = builder.Build()
paragraph.layout(300)

image_width = math.ceil(paragraph.LongestLine)
image_height = math.ceil(paragraph.Height)
surface = skia.Surfaces.MakeRasterN32Premul(image_width, image_height)
canvas = surface.getCanvas()
canvas.clear(skia.ColorWHITE)
paragraph.paint(canvas, 0, 0)
surface.flushAndSubmit()
image = surface.makeImageSnapshot()
image.save("test.png", skia.kPNG)
```

**Expected behavior**

The text with newlines is rendered properly on windows.

**Desktop (please complete the following information):**
 - OS: windows 11, ubuntu 22.04
 - Python: 3.10
 - skia-python version: v130.0b10



Discussion / hints:
Probably the same as https://github.com/kyamagu/skia-python/issues/268 - did you copy a icudtl.dat ?
The icudtl.dat issue is also where we differ from rust-skia . See #270 
This does seem to be the problem. I did copy icudtl.dat before, otherwise there will be an error. But it seems to be something wrong with the icudtl.dat. When I replaced it with icudtl.dat from rust-skia and the problem disappeared!

https://github.com/rust-skia/skia-binaries/releases/download/0.78.2/skia-binaries-ec00cf219c4901d785ed-x86_64-pc-windows-msvc-textlayout.tar.gz
Maybe it's the icu version? I tried [ICU 74.2](https://github.com/unicode-org/icu/releases/tag/release-74-2) and it's working.
I don't quite understand where rust-skia's icudtl.dat comes from, maybe there is a icudtl.dat in skia's build results?
That's very interesting - so you are saying 75.1 doesn't work correctly, but 74.2 from unicode-org upstream and also whatever version from rust-skia does? Can you give the skipped tests in #270 (on windows) a try?
Okay, thanks. I checked, and basically you need v69 before m126, and current skia since m126 uses v74. Current v75 is too new. See https://github.com/kyamagu/skia-python/pull/270/commits/2b6ab54ea4c109583f0b28c67dfb246d00a0d01c
There seems to be an `icudtl.dat` under the `skia/out/Release` directory. Maybe it's better to use that file rather then directly downloading from unicode-org upstream?

And, should `icudtl.dat` be packaged into the python package in `setup.py`?
If you can figure out how to package that, that would be good. AFAIK, next to `skia-python.so` isn't it. Patching skia for our own location,  plus packaging it, is basically rust-skia's answer, I think. They carry a semi-permanent sizeable patch to skia just for that.

Fix patch:
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index f104578c..b25b71de 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -73,11 +73,11 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        os: [ubuntu-22.04, windows-2022, macos-12]
+        os: [ubuntu-22.04, windows-2022, macos-13]
         arch: [auto64]
         cp: ["cp3{8,9,10,11,12}"]
         include:
-          - os: macos-12
+          - os: macos-13
             arch: arm64
             cp: "cp3{8,9,10,11,12}"
           # aarch64 is emulated and takes longer, build one wheel per job
diff --git a/relnotes/README.m130.md b/relnotes/README.m130.md
index fa8bc7ba..4b61d791 100644
--- a/relnotes/README.m130.md
+++ b/relnotes/README.m130.md
@@ -16,10 +16,10 @@ Since m128 (last beta release):
     overloaded as an alias to this, too.
 
   * The `SkUnicode` class is now available under python as `skia.Unicode`.
-    The constructor is known to fail on windows - It is likely that downloading
-    a `icudtl.dat` file, renaming from the versioned data-bin-{l,b}.zip in
-    https://github.com/unicode-org/icu/releases, is needed. Windows users please report
-    success/failure on this.
+    The constructor is known to fail on windows on m130, without a bundled
+    `icudtl.dat` file.
+
+    EDIT: Therefore, we bundle a `icudtl.dat` file for windows from m131 onward.
 
   * There are two examples `shape_text.py` (a python port of upstream's example), and
     `skparagraph-example.py` hosted [elsewhere](https://github.com/HinTak/skia-python-examples/).
diff --git a/setup.py b/setup.py
index e4494234..a9d2bdbc 100644
--- a/setup.py
+++ b/setup.py
@@ -19,6 +19,7 @@
     'SKIA_OUT_PATH', os.path.join(SKIA_PATH, 'out', 'Release')
 )
 
+data_files = []
 if sys.platform == 'win32':
     DEFINE_MACROS = []  # doesn't work for cl.exe
     LIBRARIES = [
@@ -54,6 +55,7 @@
         '/OPT:ICF',
         '/OPT:REF',
     ]
+    data_files = [('Lib/site-packages', [os.path.join(SKIA_OUT_PATH, 'icudtl.dat')])]
 elif sys.platform == 'darwin':
     DEFINE_MACROS = [
         ('VERSION_INFO', __version__),
@@ -173,6 +175,7 @@ def build_extensions(self):
     long_description=open('README.md', 'r').read(),
     long_description_content_type='text/markdown',
     ext_modules=[extension],
+    data_files=data_files,
     install_requires=[
         'numpy',
         'pybind11>=2.6'


Test patch:
diff --git a/tests/test_font.py b/tests/test_font.py
index 2a220f98..b895633f 100644
--- a/tests/test_font.py
+++ b/tests/test_font.py
@@ -1,3 +1,5 @@
+import os
+import sys
 import skia
 import pytest
 
@@ -643,3 +645,47 @@ def test_FontMetrics_hasStrikeoutThickness(fontmetrics):
 def test_FontMetrics_hasStrikeoutPosition(fontmetrics):
     position = 0.
     assert isinstance(fontmetrics.hasStrikeoutPosition(position), bool)
+
+
+@pytest.fixture
+def color_emoji_run():
+    if sys.platform.startswith("linux"):
+        if (os.getenv("GITHUB_ACTION") == True):
+            # Ubuntu is weird - the font is on disk but not accessible to fontconfig
+            # - Possibly https://bugs.launchpad.net/bugs/2054924
+            typeface = skia.Typeface.MakeFromFile("/usr/share/fonts/truetype/noto/NotoColorEmoji.ttf")
+        else:
+            pytest.skip("Not in Ubuntu CI")
+    if sys.platform.startswith("darwin"):
+        typeface = skia.Typeface("Apple Color Emoji")
+    if sys.platform.startswith("win"):
+        typeface = skia.Typeface("Segoe UI Emoji") # COLRv0
+    text = "✌✌🏻"
+    font = skia.Font(typeface,109)
+    blob = skia.TextBlob.MakeFromShapedText(text, font)
+    run = [x for x in blob]
+    return run[0]
+
+def test_emoji_count(color_emoji_run):
+    assert (color_emoji_run.fGlyphCount == 2)
+
+def test_emoji_typeface(color_emoji_run):
+    assert ((color_emoji_run.fTypeface.getFamilyName() == "Noto Color Emoji")
+            or (color_emoji_run.fTypeface.getFamilyName() == "Apple Color Emoji")
+            or (color_emoji_run.fTypeface.getFamilyName() == "Segoe UI Emoji"))
+
+# The numbers are hardcoded for three specific version of fonts,
+# and will change if the hosts are upgraded.
+def test_emoji_glyph1(color_emoji_run):
+    if (os.getenv("GITHUB_ACTION") == True):
+        assert ((color_emoji_run.fGlyphIndices[0] == 148)
+                or (color_emoji_run.fGlyphIndices[0] == 247)
+                or (color_emoji_run.fGlyphIndices[0] == 1567))
+
+# The numbers are hardcoded for three specific version of fonts,
+# and will change if the hosts are upgraded.
+def test_emoji_glyph2(color_emoji_run):
+    if (os.getenv("GITHUB_ACTION") == True):
+        assert ((color_emoji_run.fGlyphIndices[1] == 1512)
+                or (color_emoji_run.fGlyphIndices[1] == 248)
+                or (color_emoji_run.fGlyphIndices[1] == 1571))
diff --git a/tests/test_paragraph.py b/tests/test_paragraph.py
index fb759af9..5f463358 100644
--- a/tests/test_paragraph.py
+++ b/tests/test_paragraph.py
@@ -1,15 +1,29 @@
 import skia
 import pytest
-import sys
 
-def test_FontCollection_init0():
-    assert isinstance(skia.textlayout.FontCollection(), skia.textlayout_FontCollection)
+@pytest.fixture(scope='session')
+def textlayout_font_collection():
+    return skia.textlayout.FontCollection()
+
+def test_FontCollection_init0(textlayout_font_collection):
+    assert isinstance(textlayout_font_collection, skia.textlayout_FontCollection)
+
+
+@pytest.fixture(scope='session')
+def paragraph_style():
+    return skia.textlayout.ParagraphStyle()
 
-def test_ParagraphStyle_init0():
-    assert isinstance(skia.textlayout.ParagraphStyle(),skia.textlayout_ParagraphStyle)
+def test_ParagraphStyle_init0(paragraph_style):
+    assert isinstance(paragraph_style, skia.textlayout_ParagraphStyle)
+
+
+@pytest.fixture(scope='session')
+def textlayout_text_style():
+    return skia.textlayout.TextStyle()
+
+def test_TextStyle_init0(textlayout_text_style):
+    assert isinstance(textlayout_text_style, skia.textlayout_TextStyle)
 
-def test_TextStyle_init0():
-    assert isinstance(skia.textlayout.TextStyle(), skia.textlayout_TextStyle)
 
 @pytest.fixture(scope='session')
 def paragraph_builder():
@@ -21,8 +35,29 @@ def test_ParagraphBuilder_init0(paragraph_builder):
     assert isinstance(paragraph_builder, skia.textlayout_ParagraphBuilder)
 
 def test_Paragraph_init0(paragraph_builder):
-    if sys.platform.startswith("win"):
-        pytest.skip("Known not to work; To be investigated.")
     paragraph_builder.addText("")
     paragraph = paragraph_builder.Build()
     assert isinstance(paragraph, skia.textlayout_Paragraph)
+
+
+# Adapted from #278, to make sure that "\n" results in a line break  (rather that .notdef).
+# Height is larger than twice width, if a break happens.
+def test_Paragraph_linebreak(paragraph_builder, textlayout_text_style, textlayout_font_collection, paragraph_style):
+    paint = skia.Paint()
+    paint.setColor(skia.ColorBLACK)
+    paint.setAntiAlias(True)
+
+    textlayout_text_style.setFontSize(50)
+    textlayout_text_style.setForegroundPaint(paint)
+
+    textlayout_font_collection.setDefaultFontManager(skia.FontMgr())
+
+    builder = skia.textlayout.ParagraphBuilder.make(
+        paragraph_style, textlayout_font_collection, skia.Unicodes.ICU.Make()
+    )
+    builder.pushStyle(textlayout_text_style)
+
+    builder.addText("o\no")
+    paragraph = builder.Build()
+    paragraph.layout(300)
+    assert (paragraph.Height > 0) and (paragraph.Height > paragraph.LongestLine * 2)
diff --git a/tests/test_unicode.py b/tests/test_unicode.py
index 9a1684e4..c7b8a364 100644
--- a/tests/test_unicode.py
+++ b/tests/test_unicode.py
@@ -1,20 +1,13 @@
 import skia
 import pytest
-import sys
 
 def test_Unicodes_init0():
-    if sys.platform.startswith("win"):
-        pytest.skip("Known not to work; To be investigated.")
     assert isinstance(skia.Unicode(), skia.Unicode)
 
 # recommended alias (upstream):
 def test_Unicodes_init1():
-    if sys.platform.startswith("win"):
-        pytest.skip("Known not to work; To be investigated.")
     assert isinstance(skia.Unicodes.ICU.Make(), skia.Unicode)
 
 # Canonical
 def test_Unicodes_init2():
-    if sys.platform.startswith("win"):
-        pytest.skip("Known not to work; To be investigated.")
     assert isinstance(skia.Unicode.ICU_Make(), skia.Unicode)