[cairo] Conditionally include debug functions with ENABLE_DEBUG_FUNCS

[Behdad Esfahbod]

On Mon, 2007-10-22 at 23:36 +0000, BJörn Lindqvist wrote:
> On 10/18/07, Behdad Esfahbod <behdad at behdad.org> wrote:
> > On Thu, 2007-10-18 at 17:13 -0400, BJörn Lindqvist wrote:
> > > Hello,
> > >
> > > Here is a patch that introduces a ENABLE_DEBUG_FUNCS macro define. Its
> > > purpose is to allow for debug stuff to be conditionally included in
> > > Cairo proper. Things that must be exposed for testing to work, but
> > > that should not be included in the final and really public API.
> > >
> > > An example is the cairo_boilerplate_xlib_surface_disable_render()
> > > function. It is one of those functions that it is much easier to deal
> > > with if it is inside Cairo rather than outside since it must mock
> > > around with Cairo internals to work.
> > >
> > > Other debug functions than cairo_xlib_surface_disable_render() can be
> > > added in the future.
> >
> > So, what's the justification for this?  Boilerplate functions used to be
> > in libcairo and it took me two or three days of work to move them out to
> > get to the clean state that it is currently.  Moving them back is just
> > regressing.
> 
> My fault, I thought this problem had already been discussed,
> sorry. Quick recap (you already know this), Cairo tries to put up a
> fence around its implementation so that you are forced to rely on the
> public API. The C language is pretty crappy when it comes to
> encapsulation and only has two modes: everyone can use or no one can
> use. Cairo has no public API for disabling Xrender, but if you really,
> really want to use it anyway, and you can change the implementation,
> then there are 3 options:
> 
> A. Add a cairo_xlib_surface_disable_render() function to the public
>    API.
> B. Cheat your way through the encapsulation using private headers.
> C. Conditionally compile Cairo for code that uses
>    cairo_xlib_surface_disable_render().
> 
> I prefer A, because that is the option you would have used in Java and
> it puts the test suite on an equal footing with other 3rd party code
> (where it should be).

No.  First, in Java you'll write test cases in the same file as the
class source and have access to all its private parts.  You can do the
same in C, and D-BUS does that.  It also means that you need a special
build for testing.  I don't like that.  Regardless, we are writing in C,
not Java.  It's not interesting what option one would have used in Java.

> It is also easy to extend with
> cairo_xlib_surface_disable_shm() and similar features. But I know that
> is not what you want.
> 
> B is fragile because it relies on internals.

No, it doesn't rely on internals.  As far as libcairoboilerplate is
concerned, those structs are public and accessible.  It's not like
blindly guessing struct members based on an old obsolete copy of the
header.


> It breaks if you happen to include cairoint.h.

That's a problem with our broken include-everything-here cairoint.h.
I've fixed part of it, the rest remains.  It's fragile, but solution is
not going back to what it was.


>  For example, I tried to move the render_major
> and render_minor attributes to the screen_info struct, but that
> doesnt' work because the
> cairo_boilerplate_xlib_surface_disable_render() doesn't know about the
> cairo_xlib_screen_info_t struct. And it's hard to fix because
> including cairo-xlib-private.h in cairo-boilerplate-xlib.c drags in
> cairoint.h too.

I tried to do that move too and gave up for the same reason.  However,
these are the sensible resolutions I see:

  - Push cairoint.h out of cairo-xlib-private.h.  It just is there
because I didn't need that header in boilerplate at the time I was doing
this.  I created at least 10 new headers to move the interesting bits
out of cairoint.h, and it just happened that the stuff in
cairo-xlib-private.h wasn't interesting back then.

  - Just don't remove render_minor and render_major from
cairo_xlib_surface_t.


> So that leaves us with C as the lesser of the evils. But maybe there
> is a better way to fix the problem?

Yes, just leave it as is.  What's the problem you are trying to solve
here?  Save 8 bytes in a 368-byte surface?


-- 
behdad
http://behdad.org/

"Those who would give up Essential Liberty to purchase a little
 Temporary Safety, deserve neither Liberty nor Safety."
        -- Benjamin Franklin, 1759