[cairo] Conditionally include debug functions with ENABLE_DEBUG_FUNCS

[Behdad Esfahbod]

On Sun, 2007-11-04 at 05:52 +0000, BJörn Lindqvist wrote:
> On 10/30/07, Behdad Esfahbod <behdad at behdad.org> wrote:
[snip]
> > > 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?

Yes, but that's irrelevant.  We are talking about libcairoboilerplate.
Don't try to invent problems that don't exist.


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

Quite simple: all headers matching *-private.h are private, cairoint.h
is private, all other headers are public.


> >   - Just don't remove render_minor and render_major from
> > cairo_xlib_surface_t.
> 
> That is fragile and ugly.

How is it fragile, care to explain?  How is "surface->render_minor"
uglier than "surface->screen_info->display->render_minor"?


> It is ugly because Xrender's version isn't a
> property of an Xlib surface.

Abstractions are nice, yes, but not mandatory.  How would you implement
cairo_xlib_surface_disable_xrender() if you don't have a per-surface
Xrender version then?  You have to turn it off at the cairo level, and
then turn it on after you are done.  May I suggest that *that* is ugly?

> Plus, when Xshm is implemented, should
> the surface struct acquire shm_major and shm_minor with a matching
> cairo_boilerplate_xlib_surface_disable_shm()?

If and when we add Xshm support we'll consider that.  That's been the
approach taken in cairo.  Carl didn't start cairo with a backend struct
even.  It just grew as needed.


> Should every state
> property of the X display be present in every Xlib surface because
> test code needs to get hold of it?

Not necessarily.  Stop generalizing please.


> I don't think that is a good idea in the long run.

Yeah, I don't think it is either.


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

I don't think any patches are needed there right now.


>  To fix conceptual problems that makes the code more involved than
> needed.

I'm really more interested in practical problems.  Specially when
dealing with public API.


> XRenderQueryVersion() shouldn't be executed every time a new
> Xlib surface is created

Why?  It's fast.

>  only once when the display is opened. You
> would have the same ugliness with XShmQueryVersion() when it is
> implemented.

If you really want to call those functions once, sure, cache the result
in the display struct, like Chris did for buggy_repeat.  But that's
orthogonal to whether the surface has those attributes.  For
buggy_repeat for example, the surface just copies it from the display
struct upon creation.  Now if you want to clean, go remove buggy_repeat
which has no use staying in the surface struct.  render_minor and
render_major right now have a good reason to live in the surface struct.


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