[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [ft-devel] PCF: Issues with lazy copy in `ft_bitmap_glyph_init'
From: |
Werner LEMBERG |
Subject: |
Re: [ft-devel] PCF: Issues with lazy copy in `ft_bitmap_glyph_init' |
Date: |
Sat, 01 Sep 2018 11:24:01 +0200 (CEST) |
>> Here is all the confusing parts about FT_Get_Glyph:
>>
>> - It takes ownership on the bitmap without copying the data. Thus,
>> it should not be called twice on the same GlyphSlot and should
>> fail if FT_GLYPH_OWN_BITMAP is not set. The current behavior with
>> deep copy when FT_GLYPH_OWN_BITMAP is not set is a bug that need
>> to be fixed.
I've just tried that, see attached patch. However, this patch makes
BDF files fail with current ftview: Bitmap fonts are no longer
displayed. Interestingly, PCF files work...
Care to debug? I won't have time the next few days I fear...
>> - It copies the outline data without modifying original
>> ownership. So it is the opposite of bitmap glyphs. To add to the
>> confusion FT_OUTLINE_OWNER is in a different place.
Sigh.
>> I think we should stick to the lazy copy in both cases because it
>> is usually used once. The deep copy is a waste because
>> FT_GlyphSlot is usually discarded. I do not think the deep copy is
>> useful.
>
> What is the usual use case and why are there two (very) different
> glyph containers in first place (`FT_Glyph' and `FT_GlyphSlot')?
A glyph slot is always tied to a face (which normally contains a
single one). On the other hand `FT_Glyph' objects are containers for
`glyph objects'. The idea is to hide the type; either bitmaps or
outlines should be handled identically.
> I would expect that `FT_GlyphSlot' is a wrapper around `FT_Glyph' to
> add more information that is e.g. needed for rendering etc. But it
> turns out that they are two rather distinct things.
Yes. It's unfortunate that we have `face->glyph' and not
`face->glyph_slot'...
> I totally agree with whatever the two of you cook up; just trying to
> shed some light from a user's perspective who tries to work with
> FreeType only (!) reading the API docs (not trying to understand any
> of the black magic that goes on in the depths of FT):
This is very helpful.
> The docs surrounding `FT_Get_Glyph' really make it look like I get
> an independent `FT_Glyph' that can be used whatever.
Yeah, bad documentation...
> I would definitely add several warning signs to the docs. Also, can
> FreeType become the owner of that generated `FT_Glyph'?
No, the ownership of an `FT_Glyph' object can't be taken back.
However, you can use `FT_GlyphSlot_Own_Bitmap' to make a glyph slot
object own a bitmap again – as long as the object exists that took
away the ownership.
> That way the confusion with `FT_Done_Glyph' would vanish & it would
> be clearer that I will always get the same (maybe, but who cares?)
> object from calling `FT_Get_Glyph' -- changes that I make to that
> object might get propageted into `FT_GlyphSlot' (also something that
> should be put into the docs explicitly IMO).
Please provide a documentation patch.
> Reflecting on it, I do really believe that the call to
> `FT_Done_Glyph' is the source of much confusion (at least for me).
Again, please provide a patch that we can work on.
Werner
2018-09-01 Werner Lemberg <address@hidden>
* src/base/ftglyph.c (ft_bitmap_glyph_init): Disallow deep copying.
Right now, calling `FT_Get_Glyph' more than once would lead to
double frees if you follow the documentation, cf. the thread
starting with
http://lists.nongnu.org/archive/html/freetype-devel/2018-08/msg00069.html
This commit fixes the issue and documents it (which is not
necessarily the best solution).
diff --git a/include/freetype/ftglyph.h b/include/freetype/ftglyph.h
index d570b205c..7e68ec365 100644
--- a/include/freetype/ftglyph.h
+++ b/include/freetype/ftglyph.h
@@ -260,8 +260,11 @@ FT_BEGIN_HEADER
* FT_Get_Glyph
*
* @description:
- * A function used to extract a glyph image from a slot. Note that the
- * created @FT_Glyph object must be released with @FT_Done_Glyph.
+ * Extract a glyph image from a slot. Note that the created @FT_Glyph
+ * object must be released with @FT_Done_Glyph.
+ *
+ * If `slot` holds a bitmap, this function can be called only once since
+ * it takes ownership of the bitmap.
*
* @input:
* slot ::
@@ -278,6 +281,41 @@ FT_BEGIN_HEADER
* Because `*aglyph->advance.x` and `*aglyph->advance.y` are 16.16
* fixed-point numbers, `slot->advance.x` and `slot->advance.y` (which
* are in 26.6 fixed-point format) must be in the range ]-32768;32768[.
+ *
+ * If you need to create more than a single @FT_Glyph object from
+ * `slot`, use code similar to the following example (which works
+ * for both bitmap and outline glyph formats).
+ *
+ * {
+ * FT_Glyph reference, glyph1, glyph2;
+ *
+ *
+ * FT_Load_Glyph( face, 0, 0 );
+ * FT_Get_Glyph( face->glyph, &reference );
+ *
+ * FT_Glyph_Copy( reference, &glyph1 );
+ * FT_Glyph_Copy( reference, &glyph2 );
+ * ...
+ * FT_Done_Glyph( reference );
+ * FT_Done_Glyph( glyph1 );
+ * FT_Done_Glyph( glyph2 );
+ * }
+ *
+ * An alternative is to make `slot` own the bitmap again (i.e., copying
+ * back) after it has been taken by `FT_Get_Glyph`.
+ *
+ * {
+ * FT_Load_Glyph( face, 0, 0 );
+ * FT_Get_Glyph( face->glyph, &glyph1 );
+ *
+ * FT_GlyphSlot_Own_Bitmap( face->glyph );
+ * FT_Get_Glyph( face->glyph, &glyph2 );
+ *
+ * FT_Done_Glyph( glyph1 );
+ * FT_Done_Glyph( glyph2 );
+ * }
+ *
+ * Error handling is omitted in both examples for simplicity.
*/
FT_EXPORT( FT_Error )
FT_Get_Glyph( FT_GlyphSlot slot,
diff --git a/src/base/ftglyph.c b/src/base/ftglyph.c
index 27402ecf8..54bd35427 100644
--- a/src/base/ftglyph.c
+++ b/src/base/ftglyph.c
@@ -59,9 +59,8 @@
ft_bitmap_glyph_init( FT_Glyph bitmap_glyph,
FT_GlyphSlot slot )
{
- FT_BitmapGlyph glyph = (FT_BitmapGlyph)bitmap_glyph;
- FT_Error error = FT_Err_Ok;
- FT_Library library = FT_GLYPH( glyph )->library;
+ FT_BitmapGlyph glyph = (FT_BitmapGlyph)bitmap_glyph;
+ FT_Error error = FT_Err_Ok;
if ( slot->format != FT_GLYPH_FORMAT_BITMAP )
@@ -73,17 +72,14 @@
glyph->left = slot->bitmap_left;
glyph->top = slot->bitmap_top;
- /* do lazy copying whenever possible */
+ /* do lazy copying */
if ( slot->internal->flags & FT_GLYPH_OWN_BITMAP )
{
glyph->bitmap = slot->bitmap;
slot->internal->flags &= ~FT_GLYPH_OWN_BITMAP;
}
else
- {
- FT_Bitmap_Init( &glyph->bitmap );
- error = FT_Bitmap_Copy( library, &slot->bitmap, &glyph->bitmap );
- }
+ error = FT_THROW( Invalid_Argument );
Exit:
return error;
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [ft-devel] PCF: Issues with lazy copy in `ft_bitmap_glyph_init',
Werner LEMBERG <=