[cairo] OS/2 backend support files

[Doodle]

On Fri, 28 Jul 2006, Carl Worth wrote:

> There are a few small comments on the patch itself which I will make
> below. After those are addressed all we will need is someone who is
> willing to maintain this code, (Doodle? Peter?)

I guess it will be Peter (as he has a fully working
GCC+Configure+git+etc.. setup), and I'll support him from the background.

>
> I don't really like seeing the os2 implementation spread out over four
> files like this. None of the other backends do this for example.

I tried to keep logical places separated, so if somebody doesn't want it
to be compiled into DLL then simply don't compile and link the
cairo-os2-dll.c file, and I felt that the cairo-os2-initialize() and
cairo-os2-uninitialize() APIs do not really fit into the current Cairo API
design, so that's why they're in a separate file.

Anyway, to cut it short, I guess it's absolutely no problem to simply join
all these files into a one-to-rule-them-all .c file. Maybe one #define
would be needed then to tell if we want the DLL
Initialization/Uninitialization code to be included or not (to tell if the
build target is static or dynamic library).

> > +  /* Uninitialize FontConfig */
> > +  FcFini();
>
> As it turns out, there's no good reason to call FcFini in "production"
> code. It's only there so that one can cause fontconfig to free up all
> of its static data, (compare to cairo_debug_reset_static_data). That's
> nice when doing testing with valgrind, but not actually helpful in
> general, (and could in fact slow things down).

It's called exactly for that reason: to free up everything once Cairo will
not be used anymore.

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.

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.

> > +  /*
> > +  pOS2Surface->bmi2BitmapInfo.ulCompression = BCA_UNCOMP;
> > +  pOS2Surface->bmi2BitmapInfo.cbImage = 0;
> > +  pOS2Surface->bmi2BitmapInfo.cxResolution = 70;
> > +  pOS2Surface->bmi2BitmapInfo.cyResolution = 70;
> > +  pOS2Surface->bmi2BitmapInfo.cclrUsed = 0;
> > +  pOS2Surface->bmi2BitmapInfo.cclrImportant = 0;
> > +  pOS2Surface->bmi2BitmapInfo.usUnits = BRU_METRIC;
> > +  pOS2Surface->bmi2BitmapInfo.usReserved = 0;
> > +  pOS2Surface->bmi2BitmapInfo.usRecording = BRA_BOTTOMUP;
> > +  pOS2Surface->bmi2BitmapInfo.usRendering = BRH_NOTHALFTONED;
> > +  pOS2Surface->bmi2BitmapInfo.cSize1 = 0;
> > +  pOS2Surface->bmi2BitmapInfo.cSize2 = 0;
> > +  pOS2Surface->bmi2BitmapInfo.ulColorEncoding = BCE_RGB;
> > +  pOS2Surface->bmi2BitmapInfo.ulIdentifier = 0;
> > +  */
>
> Why is all this dead code included? It should be removed, unless there
> is a very good reason for it to stay, (in which case that reason needs
> to be present as a comment).

You're right, it should be removed. Just for the log: that is the standard
way of initializing that structure, but it also works perfectly without
doing them, so I rem'd those lines out (but forgot to remove).

>
> > +  /* This is possibly not needed, malloc'd space is
> > +   * usually zero'd out!
> > +   */
> > +  /*
> > +   memset(pOS2Surface->pchPixels, 0x00, swpTemp.cx * swpTemp.cy * 4);
> > +   */
>
> That's not a good assumption to make in general. Use calloc instead of
> malloc if you want to ensure that.

I'm still not sure if we want that memset() or not.
What happens in other backends, when they are created?
Will they have a fully black surface by default?
If they do, then the OS/2 surface should also do, so the memset() should
be in there. However, if it's not defined what the newly created surface
will contain, then the memset() is not needed because the user will redraw
the whole surface for sure, so it's useless, only takes time.
Another idea was that as it's a surface which is connected to an OS/2
window, it should initialize its contents from the window itself.
I dropped this idea because most of the time it's used to draw an image
from scratch, and the whole window content will be redrawn, so copying
over the window contents into the surface only for later being overwritten
is stupid. If one really needs it, cairo's own dirty area API can be used
to force such a copy.

Any comments?

>
> > +    /* Invalid parameter (wrong surface)! */
> > +    return;
> ..
> > +    /* Could not get mutex! */
> > +    return;
>
> Shouldn't there be error values returned in these cases?

They are in void blabla() functions. I guess you meant to ask if those
functions should be changed to return bool or something similar?
Well, some of them are defined by Cairo itself (e.g.: release_source_image)
and some might have no use (e.g. cairo_surface_repaint_window()) in
returning success or failure, but I'm not against changing it.

> > +/* cairo_os2_initialize() :                                       */
> > +/*                                                                */
> > +/* Initializes the Cairo library. This function is automatically  */
> > +/* called if Cairo was compiled to be a DLL (however it's not a   */
> > +/* problem if it's called multiple times), but if you link to     */
> > +/* Cairo statically, you have to call it once to set up Cairo's   */
> > +/* internal structures and mutexes.                               */
> > +
> > +cairo_public void
> > +cairo_os2_initialize(void);
> > +
> > +/* cairo_os2_uninitialize() :                                     */
> > +/*                                                                */
> > +/* Uninitializes the Cairo library. This function is automatically*/
> > +/* called if Cairo was compiled to be a DLL (however it's not a   */
> > +/* problem if it's called multiple times), but if you link to     */
> > +/* Cairo statically, you have to call it once to shut down Cairo, */
> > +/* to let it free all the resources it has allocated.             */
> > +
> > +cairo_public void
> > +cairo_os2_uninitialize(void);
>
> Hmmm... I'm not sure about this. We currently have a similar
> static-linking problem with the win32 backend. If we do decide to use
> extra functions like this, then we should also add them to the win32
> backend and use consistent naming, (I'd vote for cairo_<foo>_init and
> cairo_<foo>_fini myself).

Yes I know, Win32 has very similar things. One of the problems is with the
mutexes, that they have to be initialized somewhere, without introducing
any race condition. The best solution for that (imho) is a global
init/fini function pair.

>
> But the problem is that this is an inherently fragile approach. The
> extra functions are not needed when dynamically linking, (nor are they
> needed for many backend), so we expect many users to not put them
> in. This means that programs that work fine with dynamic linking will
> fail with static linking.

I agree, however, these extra functions are not needed when dynamically
linking because I tried to be compatible with original Cairo as much as
possible, that's why I put an implicit cairo_os2_initialize() and
cairo_os2_uninitialize() call into the dll's init/term function.

But imagine platforms where there is no such possibility, where there is
no such thing as library load-time init/uninit function. There the only
possibility is to call explicitly the init/uninit functions.

So, I'd vote for removing the implicit stuffs, and make sure that the
users will get that if they want to have a cross-platform code, they have
to initialize the library before the first use.

>
> An alternate idea would be to identify the minimal necessary entry
> points and sprinkle them with the initialization calls internally. I
> think those entry points are surface_create, (which is an easy one,
> since it's already backend-specific), and font_face_create, (which is
> a bit trickier, since it's not backend-specific---but maybe we just
> define a new CAIRO_MUTEX_INITIALIZE macro next to CAIRO_MUTEX_DECLARE
> and the font_face_create and surface_create functions can call it if
> it is not NULL). Thoughts?

This may introduce race conditions. As the following is not atomic, it's
not very safe:
---->8---->8---->8----
if (mutex_handle==NULL) mutex_handle = create_mutex_handle();
mutex_lock(mutex_handle);
---->8---->8---->8----

Another problem is that there is no place where these resources will be
destroyed. I know that mutexes in POSIX are simple variables, but on Win32
and on OS/2 they are system resources, so they have to be freed up.
I know, these resources are freed up by the system automatically when the
process dies, but remember my example above, where the screen saver
modules are dynamically loaded and unloaded from a process which never dies.

Still, I think the only clean solution is to have a global init/fini
function pair.

> > +/* It's also a solution, but experience shows that if this happens*/
> > +/* from a non-PM thread, then it can screw up PM internals.       */
> > +/*                                                                */
> > +/* So, best solution is to set the HWND for the surface after the */
> > +/* surface creation, so every blit will be done from application's*/
> > +/* message processing loop, which is the safest way to do.        */
> > +
> > +cairo_public void
> > +cairo_os2_surface_set_HWND (cairo_surface_t *pSurface,
> > +                            HWND hwndClientWindow);
>
> The description here is confusing me. Is it saying that calling
> cairo_os2_surface_create without also calling
> cairo_os2_surface_set_HWND is fragile and likely not to work?

No, it tries to say that calling cairo_os2_surface_create() without
cairo_os2_surface_set_HWND() is safe and ok to use as long as you do all
the cairo drawing from the thread which processes window events (called
the PM thread in OS/2). This is the case with Mozilla, so everything works
well in there.

However, there might be cases where people want to separate the drawing of
the surface from the PM thread, for example because that takes a lot of
time for a complex drawing, and they don't want to block the processing
of windowing events for that long. In that case it may not be wise to let
Cairo blit into the window directly from that non-PM thread because it can
screw up things, so the user can assign a window handle (HWND) to the Cairo
surface then, and once Cairo would want to blit into the window, then it
would simply post a message to that window to do the blitting (more like
notifying the window that the surface content was changed and it should
update the screen).

So, it's more like an added feature. The HWND is not needed normally, but
if somebody wants to separate the drawing from the windowing thread, then
it can also be done now.


> > +/* cairo_os2_surface_window_resized() :                           */
> > +/*                                                                */
> > +/* When the client window is resized, call this API so the
>
> Similar calls in the other backends, (such as xlib), use "set_size"
> rather than "window_resized". I would recommend that for consistency.

Hmm, they somehow slipped out of my sight before.
Should really be renamed then.

>
> > +/* The timeout value is in milliseconds, and tells how much the   */
> > +/* function should wait on other parts of the program to release  */
> > +/* the buffers. It is necessary, because it can be that Cairo is  */
> > +/* just drawing something into the surface while we want to       */
> > +/* destroy and recreate it.
>
> Couldn't one instead just call cairo_surface_flush() and avoid the
> need for a timeout value here?

Flush would also have to wait for an internal mutex to return, and as
the flush API has no timeout value, it could not be controlled how much to
wait for that operation.

The timeout value is needed for the following reason:
The surface resizing is probably called from the PM thread, when the
window receives a WM_SIZE message, telling that the window has been
resized. It can then tell Cairo to resize the surface. The OS/2 API
documentation says that PM message processing should not block the
execution for more than 100msecs, so we need a timeout parameter here,
again, to be good players on OS/2, and that's why cairo_surface_flush()
with uncontrollable blocking time is not acceptable.

>
> > +/* cairo_os2_surface_repaint_window() :                           */
> > +/*                                                                */
> > +/* This function can be used to force a repaint of a given area   */
> > +/* of the client window. Most of the time it is called from the   */
> > +/* WM_PAINT processing of the window proc. However, it can be     */
> > +/* called anytime if a given part of the window has to be
>
> I don't understand why this function needs to exist. We certainly
> don't have anything like it in the interface of cairo to any other
> window system. (I also may not be understanding why blit_as_changes is
> needed.)

Let me try to explain it. :)

The OS/2 windowing system does not support alpha channel, so windows
cannot be made transparent. RGBA images can be blitted into the window by
ignoring the alpha channel, but we cannot use the window contents directly
as the surface, because it does not support alpha channel and because it
would be very very slow to always query the pixels before modification and
then blit them back later.

So, it was decided that we'll use a window backbuffer on OS/2, which is
basically a cairo_image_surface. Everytime something is modified on the
image surface it is blitted into the window too.

There are special cases however, as always. It can happen that a given
portion of the window gets exposed. In this case the window will receive a
WM_PAINT message, and processing that message it should repaint the
exposed part of the window. Now, as the surface's internals are hidden
from the users, they have no direct access to the backbuffer, so here is
the API which asks Cairo to re-blit that area of the surface into the
window from the backbuffer: cairo_os2_surface_repaint_window().

The "blit_as_changes" functions are only for performance reasons. By
default, the OS/2 surface blits the changed part of the backbuffer into
the window as soon as the change happens (at release_dest_image time).
This way it always keeps the backbuffer and the real contents of the
window in sync. However, it slows down things quite much, to show the
changes "in realtime", so this feature can be turned off for those who
know what they do. This way, one can draw the whole scene with Cairo
without that being shown in the OS/2 Window (in other words, the surface
can be pre-drawn, constructed in the background), and then call
cairo_os2_surface_repaint_window() which will then blit the backbuffer
into the window at once.

This feature is used in the OS/2 screen saver modules to avoid "flickers"
and to make sure that the users won't see how the image is being drawn,
even on very slow computers.

>
> Can the user not "manually" do what repaint_window is doing by using
> window-system specific calls on the HPS and HWND? If the user can,
> then I don't think these API calls belong in cairo's interface.

As the backbuffer itself is not reachable from outside (and should never
be, as it needs quite some synchronization) it can only be done from
inside Cairo.

I hope my explanation about how this backend works was understandable.
Let me know if you have more question or gray areas!

Doodle