From 461250f0aad5a3736b1908a3f7941ced014dada1 Mon Sep 17 00:00:00 2001 From: t-bltg Date: Thu, 31 Oct 2024 09:43:44 +0100 Subject: [PATCH] make FTE thread-safe --- ext/FreeTypeExt.jl | 67 +++++++++++++++++++++++++++----------------- test/tst_freetype.jl | 22 ++++++++++++++- 2 files changed, 63 insertions(+), 26 deletions(-) diff --git a/ext/FreeTypeExt.jl b/ext/FreeTypeExt.jl index bffa3343..ca3cb37a 100644 --- a/ext/FreeTypeExt.jl +++ b/ext/FreeTypeExt.jl @@ -12,6 +12,7 @@ using FreeType const REGULAR_STYLES = "regular", "normal", "medium", "standard", "roman", "book" const FT_LIB = FT_Library[C_NULL] +const LIB_LOCK = ReentrantLock() const VALID_FONTPATHS = String[] struct FontExtent{T} @@ -23,16 +24,21 @@ end mutable struct FTFont ft_ptr::FT_Face + lock::ReentrantLock # lock this for the duration of any FT operation on ft_ptr function FTFont(ft_ptr::FT_Face) - face = new(ft_ptr) - finalizer( - face -> (face.ft_ptr != C_NULL && FT_LIB[1] != C_NULL) && FT_Done_Face(face), - face, - ) + face = new(ft_ptr, ReentrantLock()) + finalizer(safe_free, face) face end end -FTFont(path::String) = FTFont(newface(path)) + +function safe_free(face::FTFont) + @lock face.lock begin + (face.ft_ptr != C_NULL && FT_LIB[1] != C_NULL) && FT_Done_Face(face) + end +end + +FTFont(path::String) = FTFont(new_face(path)) FTFont(::Nothing) = nothing family_name(font::FTFont) = lowercase(ft_property(font, :family_name)) @@ -43,9 +49,9 @@ Base.propertynames(font::FTFont) = fieldnames(FT_FaceRec) Base.cconvert(::Type{FT_Face}, font::FTFont) = font Base.unsafe_convert(::Type{FT_Face}, font::FTFont) = font.ft_ptr -function ft_property(font::FTFont, fieldname::Symbol) - fontrect = unsafe_load(font.ft_ptr) - if (field = getfield(fontrect, fieldname)) isa Ptr{FT_String} +function ft_property(face::FTFont, fieldname::Symbol) + font_rect = @lock face.lock unsafe_load(face.ft_ptr) + if (field = getfield(font_rect, fieldname)) isa Ptr{FT_String} field == C_NULL && return "" unsafe_string(field) else @@ -60,10 +66,10 @@ Base.show(io::IO, font::FTFont) = print( check_error(err, error_msg) = err == 0 || error("$error_msg with error: $err") -function newface(facename, faceindex::Real = 0, ftlib = FT_LIB) +function new_face(name, index::Real = 0, ftlib = FT_LIB) face = Ref{FT_Face}() - err = FT_New_Face(ftlib[1], facename, Int32(faceindex), face) - check_error(err, "Couldn't load font $facename") + err = @lock LIB_LOCK FT_New_Face(ftlib[1], name, Int32(index), face) + check_error(err, "Couldn't load font $name") face[] end @@ -201,18 +207,23 @@ FontExtent(func::Function, ext::FontExtent) = FontExtent( ) function set_pixelsize(face::FTFont, size::Integer) - check_error(FT_Set_Pixel_Sizes(face, size, size), "Couldn't set pixelsize") + @lock face.lock check_error( + FT_Set_Pixel_Sizes(face, size, size), + "Couldn't set pixelsize", + ) size end -glyph_index(face::FTFont, glyphname::String)::UInt64 = FT_Get_Name_Index(face, glyphname) -glyph_index(face::FTFont, char::Char)::UInt64 = FT_Get_Char_Index(face, char) +glyph_index(face::FTFont, glyphname::String)::UInt64 = + @lock face.lock FT_Get_Name_Index(face, glyphname) +glyph_index(face::FTFont, char::Char)::UInt64 = + @lock face.lock FT_Get_Char_Index(face, char) glyph_index(face::FTFont, idx::Integer) = UInt64(idx) function kerning(face::FTFont, glyphspecs...) i1, i2 = glyph_index.(Ref(face), glyphspecs) kerning2d = Ref{FT_Vector}() - err = FT_Get_Kerning(face, i1, i2, FT_KERNING_DEFAULT, kerning2d) + err = @lock face.lock FT_Get_Kerning(face, i1, i2, FT_KERNING_DEFAULT, kerning2d) # can error if font has no kerning! Since that's somewhat expected, we just return 0 err == 0 || return SVector(0.0, 0.0) divisor = 64 # 64 since metrics are in 1/64 units (units to 26.6 fractional pixels) @@ -221,14 +232,14 @@ end function load_glyph(face::FTFont, glyph) gi = glyph_index(face, glyph) - err = FT_Load_Glyph(face, gi, FT_LOAD_RENDER) + err = @lock face.lock FT_Load_Glyph(face, gi, FT_LOAD_RENDER) check_error(err, "Could not load glyph $(repr(glyph)) from $face to render.") end function load_glyph(face::FTFont, glyph, pixelsize::Integer; set_pix = true) set_pix && set_pixelsize(face, pixelsize) load_glyph(face, glyph) - gl = unsafe_load(ft_property(face, :glyph)) + gl = @lock face.lock unsafe_load(ft_property(face, :glyph)) @assert gl.format == FT_GLYPH_FORMAT_BITMAP gl end @@ -414,16 +425,22 @@ function UnicodePlots.render_string!( end function ft_init() - FT_LIB[1] != C_NULL && error("Freetype already initialized. init() called two times ?") - FT_Init_FreeType(FT_LIB) == 0 + @lock LIB_LOCK begin + FT_LIB[1] != C_NULL && + error("Freetype already initialized. init() called two times ?") + FT_Init_FreeType(FT_LIB) == 0 + end end function ft_done() - FT_LIB[1] == C_NULL && - error("Library == CNULL. done() called before init(), or done called two times ?") - err = FT_Done_FreeType(FT_LIB[1]) - FT_LIB[1] = C_NULL - err == 0 + @lock LIB_LOCK begin + FT_LIB[1] == C_NULL && error( + "Library == CNULL. done() called before init(), or done called two times ?", + ) + err = FT_Done_FreeType(FT_LIB[1]) + FT_LIB[1] = C_NULL + err == 0 + end end add_recursive(result, path) = diff --git a/test/tst_freetype.jl b/test/tst_freetype.jl index 99c52c9c..4c7f02c6 100644 --- a/test/tst_freetype.jl +++ b/test/tst_freetype.jl @@ -3,7 +3,8 @@ const FTE = if isdefined(Base, :get_extension) else UnicodePlots.FreeTypeExt end -push!(FTE.VALID_FONTPATHS, joinpath(@__DIR__, "fonts")) +const FT_DIR = joinpath(@__DIR__, "fonts") +push!(FTE.VALID_FONTPATHS, FT_DIR) @testset "init and done" begin @test_throws ErrorException FTE.ft_init() @@ -247,4 +248,23 @@ end @test FTE.fallback_fonts() isa Tuple end +@testset "thread safety" begin + mktempdir() do dir + n = 100 + fontfiles = map(1:n) do i + p = joinpath(dir, "hack_regular_$i.ttf") + cp(joinpath(FT_DIR, "hack_regular.ttf"), p) + p + end + Threads.@threads for f in fontfiles + fo = FTE.FTFont(f) + Threads.@threads for i = 1:n + FTE.load_glyph(fo, i) + FTE.load_glyph(fo, i, 64) + FTE.render_face(fo, i, 16) + end + end + end +end + pop!(FTE.VALID_FONTPATHS)