[cairo] Conditionally include debug functions with ENABLE_DEBUG_FUNCS
behdad at behdad.org
Wed Nov 14 16:17:53 PST 2007
On Sun, 2007-11-04 at 05:52 +0000, BJörn Lindqvist wrote:
> On 10/30/07, Behdad Esfahbod <behdad at behdad.org> wrote:
> > > 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
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
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
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.
"Those who would give up Essential Liberty to purchase a little
Temporary Safety, deserve neither Liberty nor Safety."
-- Benjamin Franklin, 1759
More information about the cairo