Hi Sven,

On Wed, 2006-06-14 at 20:51 +0200, Sven de Marothy wrote:
> > Nice, but unfortunately that doesn't work for ComponentGraphics which
> > manipulate the X state. In that case we need locking before drawing. We
> > used to get that for free since ComponentGraphics overrides draw(Shape)
> > and fill(Shape). So we need to override these three new public methods
> > so we do correct locking again.
> 
> Correct.
>  
> > We might want to think a bit more about how we do this locking since the
> > current way means we do three separate JNI calls and a little extra
> > bookkeeping in ComponentGraphics to account for the gdk lock not being
> > reentrant. It is probably much more efficient if we had some
> > needs_drawing_lock flag inside CairoGraphics itself that subclasses
> > could set if needed.
> 
> Absolutely not. Never. No way. I refactored the drawing classes precisely
> to get away from bullshit like this. CairoGraphics draws to a generic 
> Cairo context. Most Cairo context do not require any kind of locking. 
> That's the whole idea of why things are done the way they are. 
> 
> Trust me, the existing code is a lot more thought through than your idea.
> What you're proposing here is going to first introduce a gdk dependency in 
> CairoGraphics, which I do not want at all. 
> 
> Second, it will slow down all the other drawing contexts by performing an 
> unncessary check for whether to do locking.
> 
> Third, you're not going to gain much speed regardless, because the 
> overhead of making two extra JNI calls is negligable compared to the time 
> required to release and get the locks, and in comparison to the fact that 
> all those drawing operations have to go through Xlib (and possible 
> client-server communication), which is why they need locks to begin with.

It is not so much about speed as it is about the , correctness and
design. As this example showed it is unclear which methods will actually
do drawing on the surface. We need a way to clearly mark those so that
subclasses that need to do any surface preparation can be sure they
override the right methods for. Especially since as with gtk+ the
locking (of an surface) is not reentrant. Having a
startSurfaceDrawing(), endSurfaceDrawing() pair, that does nothing in
the normal case, and that subclasses can override if they do need to
prepare the surface for real drawing, would be one way. I clear
description/marking of which methods do actual drawing on the surface
would be another. The misdesign atm is that a small change in the base
CairoGraphics method call order, that looks like an harmless speed
optimization can have bad effects on the subclasses if. As in the
original patch. Since it is not easy to detect the start/end of surface
usage by all methods (in all subclasses).

Cheers,

Mark

Attachment: signature.asc
Description: This is a digitally signed message part

Reply via email to