[cairo] OS/2 backend support files

[Doodle]

Carl Worth wrote:
> On Sun, 30 Jul 2006 13:49:17 +0200 (CEST), Doodle wrote:
> 
>>On Fri, 28 Jul 2006, Carl Worth wrote:
>>
>>Here on OS/2 we use Cairo for some screen saver modules (not counting
>>Mozilla), and those screen saver modules are loaded into memory when
>>needed, and unloaded when not needed, while the screen saver "master"
>>application/process is always running in the background. Imagine, if
>>neither Cairo nor Fontconfig would be cleaned up, we'd have a nice memory
>>leak after every screen saver module running.
> 
> 
> I didn't explain this very well. If you have a long-running process
> that occasionally loads a fontconfig-using module, and you don't call
> FcFini when unloading each module then you will not have any memory
> leaks, (excepting bugs of course).

Sure I will.

I guess we have a misunderstanding here. It's not that FontConfig and
Cairo is always loaded into that process and that we sometimes also
load and unload some other code which uses them. Cairo and FontConfig is
only loaded and initialized when needed, and unloaded/uninitialized when
not needed anymore. Some of the screen saver modules utilize Cairo, some
don't. (Similarly, some of the screen saver modules use Video Overlay,
some don't, and we don't have the Video Overlay support code loaded into
memory and initialized all the time, only when needed...)

So, I have a long running process that occasionally loads and unloads
Fontconfig itself (and Cairo itself) implicitly together with the saver
module which depends on it. If I don't call FcFini() and
cairo_debug_reset_static_data() when Cairo is uninitialized, then some
previously malloc()'d memory won't be free'd, and those structures will
be allocated again next time when those libraries are loaded into memory
and initialized in the same process.

I wouldn't like to argue on this, but I did detect memory leaks every
time those screen saver modules were unloaded, and the memory leak
detector showed that the leaked memory areas are from FontConfig (the
in-memory cache structure) and from Cairo (the area called "static 
data"). Calling FcFini() and cairo_debug_reset_static_data() did resolve
the memory leak.

To summarize it into one question:
Do you really think that if a process _repeatedly_ loads FontConfig.DLL,
calls FcInit(), then unloads FontConfig.DLL without calling FcFini(),
then it won't have memory leaks?

> 
> However, if you do call FcFini, then you will be removing a lot of
> cached data and will be inflicting horrific performance
> characteristics on your long-running process every time it starts
> using fontconfig again.

Yes I know, however, this is the only way to clean up everything after
the library.

> 
> So, again, the FcFini call is *not* designed to be called by running
> code---it's only designed to be called when debugging memory
> issues. (The naming convention we came up with for the analogous cairo
> function more clearly hints at this usage: cairo_debug_reset_static_data).
> The other naming bug is that the FcInit call is required, so the
> API does give the appearance that a call to FcFini is required to
> avoid leaks.
> 

I wouldn't say it's any bug, not even naming, imho. It was absolutely
clear for me, it's a well designed library: FcInit() initializes the
library, and this initialization allocates memories. This means that
a good library must have a function to do the opposite too, which is
clearly FcFini() here, which undo's everything that was done in
FcInit(), including the freeing of memory.

I think it should be the same with Cairo too, if there is a function
which allocates resources (memory or any other), then there should be
a function to deallocate it.

> 
> 
>>That's why I introduced cairo_os2_initialize() and
>>cairo_os2_uninitialize(), so now Cairo is a nice player on OS/2, not
>>leaving any dirt behind, but more about them later.
> 
> And I still prefer init/fini over initialize/uninitialize.

Just renamed them, thanks for reminding me. :)

> So the assumption we've been making in all other backends is that the
> initial state of the object being passed to surface_create is dirty
> everywhere. I suppose you could avoid the performance penalty by being
> lazy about the copy and avoiding it in the case of an initial SOURCE
> paint() operation, (which would clear the dirty state everywhere).

That sounds good! I'll do it this way too.

> OK. That makes more sense to me now. I think that both the naming and
> the documentation of these functions could be improved to better show
> the connection between blit_as_changes and repaint_window.
> 
> You might come up with something different that suits you and OS/2
> better than what I just wrote. But here are some of the important
> features of the above that should be preserved:
> 
> 1) A common verb, ("refresh" in my example), is used in both functions
>    to indicate their connection.
> 
> 2) The verb is obviously distinct from existing cairo functionality,
>    (I didn't like the similarity of "repaint" and "paint" in this
>    regard).

Okay, I've renamed them to
cairo_os2_surface_set_manual_window_refresh();
cairo_os2_surface_get_manual_window_refresh();
cairo_os2_surface_refresh_window();

and moved the less-used ones to the bottom of the header file.
Also, I've modified the documentation in the header file as
you've suggested to have more information about the connectivity
between these functions, and what they can be used to.


Bye,
   Doodle