[cairo] Conditionally include debug functions with ENABLE_DEBUG_FUNCS

[BJörn Lindqvist]

On 10/30/07, Behdad Esfahbod <behdad at behdad.org> wrote:
> On Mon, 2007-10-22 at 23:36 +0000, BJörn Lindqvist wrote:
> > 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.

No you don't. Private attributes can only be accessed from within the
enclosing class meaning that you would have to place test methods
within the class you are trying to test to do it like you describe.

In the Java "industry" much is driven by specifications so that there
may be multiple implementations of the same API all sharing the same
test suite, Sun's JCK for example. Very often it is completely
different programmers that write the implementation and the test
suite. The test writers may not even have access to the source code so
relying on extra knowledge of the internals is not possible.

That is a good thing because it puts pressure on the specifications to
become more testable.

> Regardless, we are writing in C, not Java.  It's not interesting
> what option one would have used in Java.

How can you learn if you do not look at how others do it?

> > 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.

Semantic issue I guess, but surely other code is verboten from poking
in those places?

> > 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.

I'll try that then. Is there some kind of general scheme for how the
headers should be named and what functions and structs they should
contain? It looks like almost all *.c files have a matching *.h file
and optionally a *-private.h file too, but there are exceptions to
that rule.

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

That is fragile and ugly. It is ugly because Xrender's version isn't a
property of an Xlib surface. Plus, when Xshm is implemented, should
the surface struct acquire shm_major and shm_minor with a matching
cairo_boilerplate_xlib_surface_disable_shm()? Should every state
property of the X display be present in every Xlib surface because
test code needs to get hold of it? I don't think that is a good idea
in the long run.

> > 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?

I was trying to implement xshm (Chris Wilson already did that) but
thought that a few patches was needed first. To fix conceptual
problems that makes the code more involved than
needed. XRenderQueryVersion() shouldn't be executed every time a new
Xlib surface is created only once when the display is opened. You
would have the same ugliness with XShmQueryVersion() when it is
implemented.


-- 
mvh Björn