Dear Michal,

I'm really sorry for the reply I made: I reviewed again your patch it
and applied to my tree and it appears to be totally fine and correct
to me and and I recommend Dominik/zyx to apply it.
Yes, the destructor of PdfFont was destructing the global copy of
PdfFontMetricsBase14.

I really misunderstood and confused your patch, what I was referring
is a completely different topic: FontSize is part of the state
PdfFont, which IMO is harmful since FontSize should be part of the
state of who is using PdfFont.

Sorry again for the confusion.

Francesco



On Fri, 16 Nov 2018 at 18:18, Michal Sudolsky <sudols...@gmail.com> wrote:
>
> One thing I forgot. PODOFO_Base14FontDef_FindBuiltinData is used on places 
> where return value is not deallocated. This would cause memory leaks. I am 
> sending better patch.
>
> Metrics object is allocated and copy constructed from global only on places 
> where it will be eventually deallocated.
>
> Still is good idea to have PODOFO_BUILTIN_FONTS const and 
> PODOFO_Base14FontDef_FindBuiltinData to return const pointer.
>
>
> On Fri, Nov 16, 2018 at 5:56 PM Michal Sudolsky <sudols...@gmail.com> wrote:
>>
>> Fonts created for different documents should be independent. In other case 
>> you cannot do anything usable with them when using multiple threads.
>>
>> Code:
>> ``` c++
>> PdfMemDocument doc0;
>> PdfFont *font0 = doc0.CreateFont("Helvetica");
>> font0->SetFontSize(10);
>>
>> PdfMemDocument doc1;
>> PdfFont *font1 = doc1.CreateFont("Helvetica");
>> font1->SetFontSize(20);
>>
>> printf("font0 size %g\n", font0->GetFontSize());
>> printf("font1 size %g\n", font1->GetFontSize());
>> ```
>>
>> Output:
>> ```
>> font0 size 20
>> font1 size 20
>> ```
>>
>> What I would expect (or what happens when is font for example "Arial"):
>> ```
>> font0 size 10
>> font1 size 20
>> ```
>>
>> Fonts font0 and font1 are different objects but they share same m_pMetrics 
>> (in case of base 14 fonts) which is used for font size and other font 
>> settings. This is because PODOFO_Base14FontDef_FindBuiltinData is returning 
>> mutable pointer to static global array PODOFO_BUILTIN_FONTS.
>>
>> This is here probably from beginning.
>>
>> I am sending patch. Hopefully I did not forget something. 
>> PODOFO_Base14FontDef_FindBuiltinData is now returning newly allocated 
>> metrics object copy constructed from PODOFO_BUILTIN_FONTS[i]. I deleted 
>> destructor of PdfFontTypeBase14 so metrics object is properly deleted with 
>> font. I also changed PODOFO_BUILTIN_FONTS to const for sure that there is 
>> nothing else that changes it.
>>
> _______________________________________________
> Podofo-users mailing list
> Podofo-users@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/podofo-users


_______________________________________________
Podofo-users mailing list
Podofo-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/podofo-users

Reply via email to