Skparagraph does not properly handle newlines on windows

[MeetWq]

Describe the bug

When given text that has newlines in it, Skparagraph fails to render the newlines and instead renders them as unknown characters.

But it works on linux:

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:

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

[HinTak]

Probably the same as #268 - did you copy a icudtl.dat ?

[HinTak]

The icudtl.dat issue is also where we differ from rust-skia . See #270

[MeetWq]

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

[MeetWq]

Maybe it's the icu version? I tried ICU 74.2 and it's working.

[MeetWq]

I don't quite understand where rust-skia's icudtl.dat comes from, maybe there is a icudtl.dat in skia's build results?

[HinTak]

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?

[HinTak]

Okay, thanks. I checked, and basically you need v69 before m126, and current skia since m126 uses v74. Current v75 is too new. See 2b6ab54

[MeetWq]

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?

[HinTak]

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.

[MeetWq]

I tried adding the data_files option in setup.py and it did the job. https://github.com/MeetWq/skia-python/commits/icudtl

[HinTak]

Thanks. That looks neat. I'll need to withdraw #270 and do a "try 2". :-).

[HinTak]

One possible problem is that we might break another python package which also uses unicode-related functionality (since the location is outside skia-python's). Forcing the user to copy it from somewhere at least he/she is aware of it. Anyway, that's just hypothetical, we don't know any other python package doing that.