pharo-graphics / Alexandrie

FFI bindings and a 2D canvas for Pharo based on Cairo, Freetype and Harfbuzz
MIT License
5 stars 2 forks source link

Crash on Harbuzz example #46

Closed rvillemeur closed 2 days ago

rvillemeur commented 11 months ago

@tinchodias , I can still experience the problem of #42 (as of 15 of dec 2023). My script is

| aFTLibrary aFace aeCanvas cairoScaledFont  fontHeight string glyphs|
    aFTLibrary := AeFTLibrary newInitialized.
    aFace := AeCascadiaCodeDownloadedFont new 
        downloadDirectory: AeFilesystemResources downloadedFontsDirectory;
        ensureDownloaded;
        firstFaceUsing: aFTLibrary.
fontHeight := 20.
string := 'a := A->B->>C <= c|=>d~~>e.'.

    aeCanvas := AeCanvas extent: 1000 @ (fontHeight * 4).
    aeCanvas clear: Color white.

    cairoScaledFont := aeCanvas scaledFontForFace: aFace size: fontHeight.

    "Margin"
    aeCanvas pathTranslate: (fontHeight/2) @ 0.

    "Draw text with Harfbuzz:"
    aeCanvas pathTranslate: 0 @ (fontHeight*1.1).
    aeCanvas setSourceColor: Color green muchDarker.
    glyphs := AeHbBuffer new
        direction: AeHbDirection leftToRight;
        script: AeHbScript latin;
        language: AeHbLanguage en;
        clusterLevel: AeHbBufferClusterLevel recommended;
        flags: AeHbBufferFlags beginningOrEndingOfText;
        addString: string;
        cairoGlyphArrayForFace: aFace size: fontHeight.

    aeCanvas
        drawText: glyphs
        font: cairoScaledFont.

    ^ aeCanvas asForm

If I execute it several time very quickly in a playground, I'll experience a crash. I have the same behaviour on Windows 10 and Fedora Linux 38, latest Pharo 11 fresh image and latest VM update. It's frustrating because this script is a direct write of AeHarfbuzzRenderExample class >> example5CascadiaCode which seems to work OK (no crash experienced), so it's look like a timing issue, but as I don't have any crash message, I can't figure out what goes wrong. I added crash.dmp as watched on my linux machine.

crash.zip

rvillemeur commented 11 months ago

Or with a repetition loop (always crash)

10 timesRepeat: [ |  aFTLibrary aFace aeCanvas cairoScaledFont  fontHeight string glyphs|
    aFTLibrary := AeFTLibrary newInitialized.
    aFace := AeCascadiaCodeDownloadedFont new 
        downloadDirectory: AeFilesystemResources downloadedFontsDirectory;
        ensureDownloaded;
        firstFaceUsing: aFTLibrary.
fontHeight := 20.
string := 'a := A->B->>C <= c|=>d~~>e.'.

    aeCanvas := AeCanvas extent: 1000 @ (fontHeight * 4).
    aeCanvas clear: Color white.

    cairoScaledFont := aeCanvas scaledFontForFace: aFace size: fontHeight.

    "Margin"
    aeCanvas pathTranslate: (fontHeight/2) @ 0.

    "Draw text withOUT Harfbuzz:"
    aeCanvas pathTranslate: 0 @ (fontHeight*1.1).
    aeCanvas setSourceColor: Color red muchDarker.
    glyphs := cairoScaledFont glyphArrayForString: string.
    aeCanvas
        drawText: glyphs
        font: cairoScaledFont.

    "Draw text with Harfbuzz:"
    aeCanvas pathTranslate: 0 @ (fontHeight*1.1).
    aeCanvas setSourceColor: Color green muchDarker.
    glyphs := AeHbBuffer new
        direction: AeHbDirection leftToRight;
        script: AeHbScript latin;
        language: AeHbLanguage en;
        clusterLevel: AeHbBufferClusterLevel recommended;
        flags: AeHbBufferFlags beginningOrEndingOfText;
        addString: string;
        cairoGlyphArrayForFace: aFace size: fontHeight.

    aeCanvas
        drawText: glyphs
        font: cairoScaledFont.

    ^ aeCanvas asForm]
tinchodias commented 11 months ago

I reproduced the crash but removing the return from last line.

tinchodias commented 11 months ago

The script seems to be fixed if you extract and share aFTLibrary := AeFTLibrary newInitialized.. The reason of the crash would that the FreeType library external object (with all it's faces) is released by the Garbage Collector while other external objects that point to font faces. I think this can be part of the documentation: explaining that the library user is responsible of keeping the library alive until the end.

In other side, if we move to the most recent Harfbuzz (I have added bindings to new API in last weeks), we can avoid using AeFTLibrary (the font face is loaded in another way), so your script would be cleaner and without the bug. But still, maybe we don't want to remove the bindings to FreeType even if this Harfbuzz options looks better alternative. In that case, my explanation of previous paragraph should be stated somewhere.

rvillemeur commented 11 months ago

Instead of using AeFTLibrary, should I use AeFontManager globalInstance. as I do in my notes on using harfbuzz ? What would be the benefit ? Do we still have this object management problem ? AeFTLibrary seems to be pretty low level.

tinchodias commented 11 months ago

Yes, i think it's a good alternative.

But note that AeFontManager is in another package than AeFTLibrary. So, the snippet won't be "pure" FreeType anymore.

The font manager is an abstraction that covers the scenario where that you want an object that knows how to scan fonts in the file system, and remembers which fonts were scanned (scanning the file system is slow), and provide the fonts to the users via some query. The classes AeFontManager and AeFontScanner are based in code from current's Pharo FreeTypeFontProvider and friends. That's mentioned in a class comment.

The query API of AeFontManager is poor, too exact... it lacks some flexibility in the answer. It should support something like described in https://github.com/pharo-graphics/Bloc/issues/185 .

Also, scanning could be delegated to the FontConfig library, which is specialized on that.

rvillemeur commented 11 months ago

My goal was to use Alexandrie drawing API as close to Cairo, which can differ from Bloc canvas API (lower level) . I should probably do some other example around Font management, and document the lifecycle of FreeType objects.

tinchodias commented 11 months ago

+1 @rvillemeur

BTW, in this script you can see that with the new harfbuzz-cairo API:

string := 'Off'.
utf8Encoded := string utf8Encoded.
fontSize := 60.

cairoSurface := AeCairoImageSurface
    extent: fontSize * 20 asPoint
    format: AeCairoSurfaceFormat argb32.
cairoSurface status ensureIsSuccess.
cairoContext := cairoSurface newContext.

hbBlob := AeHbBlob newFrom: AeSourceSansPro_Regular fontContentsData.
hbFace := hbBlob newHbFaceAtIndex: 0.
hbFont := hbFace newHbFont.
cairoFontFace := hbFont newCairoFontFace.
cairoContext fontFace: cairoFontFace.
cairoContext fontSize: fontSize.
glyphArray := cairoContext scaledFont glyphArrayForString: string.

cairoContext translateByX: 0 y: fontSize.
cairoContext showGlyphs: glyphArray.
Screenshot 2023-12-26 at 20 21 43

But it is still not loaded by the BaselineOfAlexandrie since it requires versions of the graphic dependencies. It avoids creating/releasing the Freetype objects (Harfbuzz objectd intead, ok) and also discovered it's not necessary to create the "cairo scaled font". It's more compact and in my small benchmarks it wasn't bad.

tinchodias commented 11 months ago

We can close this issue with an improvement in class and/or method comments.

tinchodias commented 2 days ago

@rvillemeur Do not hesitate to re-open if you consider.