<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-family:tahoma,sans-serif">Oops, sorry, wrong thread. :)<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Jun 14, 2020 at 10:34 PM Tobias Fleischer (reduxFX) <<a href="mailto:tobias.fleischer@reduxfx.com">tobias.fleischer@reduxfx.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div style="font-family:tahoma,sans-serif">If the new smoothing parameter is set to "Color", it should apply roughly the same anti-aliasing technique as AE itself, at least it looked really similar when putting a text layer next to the plugins rendered output.<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Jun 13, 2020 at 11:03 PM Tobias Fleischer (reduxFX) <<a href="mailto:tobias.fleischer@reduxfx.com" target="_blank">tobias.fleischer@reduxfx.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div style="font-family:tahoma,sans-serif">Thanks!</div><div style="font-family:tahoma,sans-serif">I will try to test it soon.</div><div style="font-family:tahoma,sans-serif">As I needed a quick and working solution for a bigger project I am working on, for now I simply replaced in my personal Cairo branch all re-assignments of the font variation string with a direct copy of the pointer variable, as the pointer to the string will always exist in my program. But I will check if your patch solves the problem also for the dynamic assignment case.</div><div style="font-family:tahoma,sans-serif">Thanks again,</div><div style="font-family:tahoma,sans-serif">Toby</div><div style="font-family:tahoma,sans-serif"><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Jun 13, 2020 at 10:44 PM Behdad Esfahbod <<a href="mailto:behdad@behdad.org" target="_blank">behdad@behdad.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">You are absolutely right. It's a mess.<div><br></div><div>cairo_font_options_t used to be a plain old struct that didn't own anything. The ->get_font_options() unfortunately was designed to take a *options that it assumed didn't need cleanup and just plain wrote to it.</div><div><br></div><div>When we added variations, all those assumptions broke. I've given it a try to clean up:</div><div><br></div><div> <a href="https://github.com/behdad/cairo/commit/53cffff1578e4bbd1fa4d8f730c6a963617572bb" target="_blank">https://github.com/behdad/cairo/commit/53cffff1578e4bbd1fa4d8f730c6a963617572bb</a></div><div><br></div><div>I think I got it all right but am not completely sure. I think what I did is the best approach given the mess we are in.</div><div><br></div><div>Please review and test. It compiles but I haven't tested at all. I also wrote the patch on a very old checkout of mine then rebase on master. I missed any code in between that would need the same treatment but can't do another review right now.</div><div><br></div><div>Hope someone can take it from there.</div><div><br></div><div>Thanks,</div><div>behdad</div><div><br></div><div>PS. I also found this mirror is stalled and confusing:</div><div><br></div><div> <a href="https://github.com/freedesktop/cairo/commits/master" target="_blank">https://github.com/freedesktop/cairo/commits/master</a></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jun 10, 2020 at 10:39 PM Tobias Fleischer (reduxFX) <<a href="mailto:tobias.fleischer@reduxfx.com" target="_blank">tobias.fleischer@reduxfx.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div style="font-family:tahoma,sans-serif"> <div style="font-family:tahoma,sans-serif">Unfortunately, I just found out it is not the only location where this is happening :(<br></div><div style="font-family:tahoma,sans-serif">For example cairo_scaled_font_create() also calls _cairo_font_options_init_copy(), which then leads to a leak.</div><div style="font-family:tahoma,sans-serif">I tried to add these lines to _cairo_scaled_font_fini_internal():<br>_cairo_font_options_fini (&scaled_font->options);<br> scaled_font->options.variations = NULL;<br></div><div style="font-family:tahoma,sans-serif"><br></div><div style="font-family:tahoma,sans-serif">But that didn't help, I still get leaks because of unmatched malloc/free blocks. Maybe it has to do with the fontmap caching/release count here?<br></div><div style="font-family:tahoma,sans-serif"><br></div><div style="font-family:tahoma,sans-serif">Or maybe we shouldn't try to duplicate the variation option string at all, but just copy the pointer and say it is up to the calling application to guarantee its existence?</div><div style="font-family:tahoma,sans-serif">A fixed char array could also be an option but that does sound very hacky.</div> </div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jun 11, 2020 at 7:30 AM Behdad Esfahbod <<a href="mailto:behdad@behdad.org" target="_blank">behdad@behdad.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="auto"><div dir="auto">Thanks Tobias. This definitely makes sense. Thanks for debugging. I'll try to take a look on computer and fix. Unless, I hope, someone beats me to that.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jun 10, 2020, 10:25 PM Tobias Fleischer (reduxFX) <<a href="mailto:tobias.fleischer@reduxfx.com" rel="noreferrer" target="_blank">tobias.fleischer@reduxfx.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div style="font-family:tahoma,sans-serif">I think I found a bug concerning non-released memory when using font variations.<br>I tested against cairo-1.17.2. <br></div><div style="font-family:tahoma,sans-serif"><br></div><div style="font-family:tahoma,sans-serif">The internal function _cairo_gstate_init_copy() is supposed to make a deep copy of the fields from one instance/state to another, used for example by cairo_save(). It does however call _cairo_font_options_init_copy(), which has this line in it:<br>options->variations = other->variations ? strdup (other->variations) : NULL;<br><br>This means that if a font variation string has been set, instead of a copy, it will always allocate and use a copy of the string (via strdup), which will then never be freed.<br>This leads to memory leaks as for example just by calling cairo_save(), with each call an additional pointer is created that is never released.<br><br>Simple sample code to reproduce:</div><div style="font-family:tahoma,sans-serif"> cairo_surface_t* surface = cairo_image_surface_create(CAIRO_FORMAT_ARGB32, 1920, 1080);<br> cairo_t* cr = cairo_create(surface);<br> </div><div style="font-family:tahoma,sans-serif">cairo_font_options_t* t = cairo_font_options_create();<br> cairo_get_font_options(cr, t);<br>cairo_font_options_set_variations(t, "wght=400");<br> cairo_set_font_options(cr, t);<br> cairo_font_options_destroy(t);<br> cairo_save(cr);<br> cairo_restore(cr); <div style="font-family:tahoma,sans-serif">cairo_destroy(cr);<br> cairo_surface_destroy(surface);<br></div> </div><div style="font-family:tahoma,sans-serif"></div><div style="font-family:tahoma,sans-serif"><br></div><div style="font-family:tahoma,sans-serif">I think what is missing is a matching free-and-null call in _cairo_gstate_fini().</div><div style="font-family:tahoma,sans-serif"></div><div style="font-family:tahoma,sans-serif">If I add the following two lines at the beginning of_cairo_gstate_fini(), it seems to fix this issue, as every allocated copy gets freed again:<br>_cairo_font_options_fini (&gstate->font_options);<br> gstate->font_options.variations = NULL;</div><div style="font-family:tahoma,sans-serif"><br></div><div style="font-family:tahoma,sans-serif">Let me know if this makes sense.</div><div style="font-family:tahoma,sans-serif">Cheers,</div><div style="font-family:tahoma,sans-serif">Toby<br></div></div> -- <br> cairo mailing list<br> <a href="mailto:cairo@cairographics.org" rel="noreferrer noreferrer" target="_blank">cairo@cairographics.org</a><br> <a href="https://lists.cairographics.org/mailman/listinfo/cairo" rel="noreferrer noreferrer noreferrer" target="_blank">https://lists.cairographics.org/mailman/listinfo/cairo</a><br> </blockquote></div></div> </blockquote></div> -- <br> cairo mailing list<br> <a href="mailto:cairo@cairographics.org" target="_blank">cairo@cairographics.org</a><br> <a href="https://lists.cairographics.org/mailman/listinfo/cairo" rel="noreferrer" target="_blank">https://lists.cairographics.org/mailman/listinfo/cairo</a><br> </blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr">behdad<br><a href="http://behdad.org/" target="_blank">http://behdad.org/</a></div> </blockquote></div> </blockquote></div> </blockquote></div></div>