.Net 4.7+ depends on this behaviour and expects to be able to do pointer equality tests for FontFamily objects.
Signed-off-by: Dmitry Timoshkov dmitry@baikal.ru --- dlls/gdiplus/font.c | 62 ++++++++++++++-------------------- dlls/gdiplus/gdiplus_private.h | 1 + dlls/gdiplus/tests/font.c | 57 ++++++++++++++++++++++++++++++- 3 files changed, 83 insertions(+), 37 deletions(-)
diff --git a/dlls/gdiplus/font.c b/dlls/gdiplus/font.c index eee272082f..2266e474df 100644 --- a/dlls/gdiplus/font.c +++ b/dlls/gdiplus/font.c @@ -115,8 +115,6 @@ typedef struct #define MS_OS2_TAG MS_MAKE_TAG('O','S','/','2') #define MS_HHEA_TAG MS_MAKE_TAG('h','h','e','a')
-static GpStatus clone_font_family(const GpFontFamily *, GpFontFamily **); - static GpFontCollection installedFontCollection = {0};
/******************************************************************************* @@ -183,13 +181,8 @@ GpStatus WINGDIPAPI GdipCreateFont(GDIPCONST GpFontFamily *fontFamily, (*font)->unit = unit; (*font)->emSize = emSize; (*font)->otm = otm; - - stat = clone_font_family(fontFamily, &(*font)->family); - if (stat != Ok) - { - heap_free(*font); - return stat; - } + (*font)->family = (GpFontFamily *)fontFamily; + InterlockedIncrement(&(*font)->family->ref);
TRACE("<-- %p\n", *font);
@@ -322,7 +315,10 @@ GpStatus WINGDIPAPI GdipGetFamily(GpFont *font, GpFontFamily **family) if (!(font && family)) return InvalidParameter;
- return GdipCloneFontFamily(font->family, family); + InterlockedIncrement(&font->family->ref); + *family = font->family; + + return Ok; }
static REAL get_font_size(const GpFont *font) @@ -518,8 +514,6 @@ GpStatus WINGDIPAPI GdipGetLogFontW(GpFont *font, GpGraphics *graphics, LOGFONTW */ GpStatus WINGDIPAPI GdipCloneFont(GpFont *font, GpFont **cloneFont) { - GpStatus stat; - TRACE("(%p, %p)\n", font, cloneFont);
if(!font || !cloneFont) @@ -528,11 +522,10 @@ GpStatus WINGDIPAPI GdipCloneFont(GpFont *font, GpFont **cloneFont) *cloneFont = heap_alloc_zero(sizeof(GpFont)); if(!*cloneFont) return OutOfMemory;
+ InterlockedIncrement(&font->family->ref); **cloneFont = *font; - stat = GdipCloneFontFamily(font->family, &(*cloneFont)->family); - if (stat != Ok) heap_free(*cloneFont);
- return stat; + return Ok; }
/******************************************************************************* @@ -769,6 +762,7 @@ GpStatus WINGDIPAPI GdipCreateFontFamilyFromName(GDIPCONST WCHAR *name, ffamily->descent = fm.descent; ffamily->line_spacing = fm.line_spacing; ffamily->dpi = fm.dpi; + ffamily->ref = 1;
*FontFamily = ffamily;
@@ -777,16 +771,6 @@ GpStatus WINGDIPAPI GdipCreateFontFamilyFromName(GDIPCONST WCHAR *name, return Ok; }
-static GpStatus clone_font_family(const GpFontFamily *family, GpFontFamily **clone) -{ - *clone = heap_alloc_zero(sizeof(GpFontFamily)); - if (!*clone) return OutOfMemory; - - **clone = *family; - - return Ok; -} - /******************************************************************************* * GdipCloneFontFamily [GDIPLUS.@] * @@ -799,19 +783,19 @@ static GpStatus clone_font_family(const GpFontFamily *family, GpFontFamily **clo * RETURNS * SUCCESS: Ok */ -GpStatus WINGDIPAPI GdipCloneFontFamily(GpFontFamily* FontFamily, GpFontFamily** clonedFontFamily) +GpStatus WINGDIPAPI GdipCloneFontFamily(GpFontFamily *family, GpFontFamily **clone) { - GpStatus status; + if (!family || !clone) return InvalidParameter;
- if (!(FontFamily && clonedFontFamily)) return InvalidParameter; + TRACE("%p (%s), %p\n", family, debugstr_w(family->FamilyName), clone);
- TRACE("%p (%s), %p\n", FontFamily, - debugstr_w(FontFamily->FamilyName), clonedFontFamily); + *clone = heap_alloc_zero(sizeof(**clone)); + if (!*clone) return OutOfMemory;
- status = clone_font_family(FontFamily, clonedFontFamily); - if (status != Ok) return status; + **clone = *family; + (*clone)->ref = 1;
- TRACE("<-- %p\n", *clonedFontFamily); + TRACE("<-- %p\n", *clone);
return Ok; } @@ -867,11 +851,17 @@ GpStatus WINGDIPAPI GdipGetFamilyName (GDIPCONST GpFontFamily *family, */ GpStatus WINGDIPAPI GdipDeleteFontFamily(GpFontFamily *FontFamily) { - if (!FontFamily) + LONG ref; + + if (!FontFamily || !FontFamily->ref) return InvalidParameter; - TRACE("Deleting %p (%s)\n", FontFamily, debugstr_w(FontFamily->FamilyName));
- heap_free (FontFamily); + ref = InterlockedDecrement(&FontFamily->ref); + if (!ref) + { + TRACE("Deleting %p (%s)\n", FontFamily, debugstr_w(FontFamily->FamilyName)); + heap_free(FontFamily); + }
return Ok; } diff --git a/dlls/gdiplus/gdiplus_private.h b/dlls/gdiplus/gdiplus_private.h index 8c4fcceded..6b48c360e6 100644 --- a/dlls/gdiplus/gdiplus_private.h +++ b/dlls/gdiplus/gdiplus_private.h @@ -518,6 +518,7 @@ struct GpFontCollection{ };
struct GpFontFamily{ + LONG ref; WCHAR FamilyName[LF_FACESIZE]; UINT16 em_height, ascent, descent, line_spacing; /* in font units */ int dpi; diff --git a/dlls/gdiplus/tests/font.c b/dlls/gdiplus/tests/font.c index 33b75c5bc5..3cada58e39 100644 --- a/dlls/gdiplus/tests/font.c +++ b/dlls/gdiplus/tests/font.c @@ -141,7 +141,6 @@ static void test_createfont(void) expect(Ok, stat); stat = GdipGetFamilyName(fontfamily2, familyname, 0); expect(Ok, stat); -todo_wine ok (fontfamily == fontfamily2, "Unexpected family instance.\n"); ok (lstrcmpiW(Tahoma, familyname) == 0, "Expected Tahoma, got %s\n", wine_dbgstr_w(familyname)); @@ -1282,6 +1281,61 @@ static void test_GdipGetFontCollectionFamilyCount(void) ok(status == InvalidParameter, "Unexpected status %d.\n", status); }
+static void test_CloneFont(void) +{ + GpStatus status; + GpFont *font, *font2; + GpFontFamily *family, *family2; + REAL height; + Unit unit; + int style; + + status = GdipCreateFontFamilyFromName(Tahoma, NULL, &family); + expect(Ok, status); + + status = GdipCreateFont(family, 30.0f, FontStyleRegular, UnitPixel, &font); + expect(Ok, status); + + status = GdipGetFontUnit(font, &unit); + expect(Ok, status); + ok(unit == UnitPixel, "got %u\n", unit); + + status = GdipGetFontSize(font, &height); + expect(Ok, status); + ok(height == 30.0f, "got %f\n", height); + + status = GdipGetFontStyle(font, &style); + expect(Ok, status); + ok(style == FontStyleRegular, "got %d\n", style); + + status = GdipGetFamily(font, &family2); + expect(Ok, status); + ok(family == family2, "got %p\n", family2); + + status = GdipCloneFont(font, &font2); + expect(Ok, status); + + status = GdipGetFontUnit(font2, &unit); + expect(Ok, status); + ok(unit == UnitPixel, "got %u\n", unit); + + status = GdipGetFontSize(font2, &height); + expect(Ok, status); + ok(height == 30.0f, "got %f\n", height); + + status = GdipGetFontStyle(font2, &style); + expect(Ok, status); + ok(style == FontStyleRegular, "got %d\n", style); + + status = GdipGetFamily(font2, &family2); + expect(Ok, status); + ok(family == family2, "got %p\n", family2); + + GdipDeleteFont(font2); + GdipDeleteFont(font); + GdipDeleteFontFamily(family); +} + START_TEST(font) { struct GdiplusStartupInput gdiplusStartupInput; @@ -1301,6 +1355,7 @@ START_TEST(font)
GdiplusStartup(&gdiplusToken, &gdiplusStartupInput, NULL);
+ test_CloneFont(); test_long_name(); test_font_transform(); test_font_substitution();
I'm not convinced this requires reference counting. Maybe FontFamily objects only exist as part of a FontCollection, and their lifetime is based on that.
"Vincent Povirk (they/them)" vincent@codeweavers.com wrote:
I'm not convinced this requires reference counting. Maybe FontFamily objects only exist as part of a FontCollection, and their lifetime is based on that.
I thought about that, however I think that GdipCloneFontFamily needs to allocate a copy and in order to avoid a memory leak GdipDeleteFontFamily should be able to free it, and reference counting solves this problem.
The only test we currently have for GdipCloneFontFamily says that the returned pointer is equal.
-
Dmitry Timoshkov -
Vincent Povirk (they/them)