On Tue, 2006-02-21 at 14:56 +0100, Mark Wielaard wrote: > Hi Tom, > > On Mon, 2006-02-20 at 18:10 -0500, Thomas Fitzsimmons wrote: > > On Mon, 2006-02-20 at 23:31 +0100, Mark Wielaard wrote: > > > My main question is whether the "loopback" protection is correct. I got > > > the idea from GtkCheckboxPeer. > > > > I think there is a deadlock risk with this patch and with > > GtkCheckboxPeer. postAdjustmentEvent is called with the GDK lock held > > and calls the synchronized method Scrollbar.setValues. > > Scrollbar.setValues calls GtkScrollbarPeer.setValues which acquires the > > GDK lock. So there's the potential for mutual embrace if a thread other > > than the GTK main thread calls Scrollbar.setValues at the right time. > > I see. Good point. gtk signal handlers should do as little as possible. > Only posting events from them seems like a good design. > > > Why not just set the new value upon receiving the posted AdjustmentEvent > > in Scrollbar.processAdjustmentEvent rather than directly in > > GtkScrollbarPeer.postAdjustmentEvent? > > There were 2 problems with doing it that way. processAdjustmentEvent() > isn't called in the 1.0 model (only when there are actual listeners). > And I didn't know how to prevent "loopback" when the scrollbar was moved > by the user and (because the system was a bit slow) the event was > triggering "late" making the scrollbar jump a bit back and forth while > it was being scrolled quickly. > > But I believe I found solutions for both of these. I do the adjusting in > the package private dispatchEventImpl() just before checking whether or > not AdjustmentEvents need to be fired. And in the peer we set > ValueIsAdjusting true to mark the event (and the component) as being > under adjustment of the user. This makes my test application and the vte > happy. > > 2006-02-21 Mark Wielaard <[EMAIL PROTECTED]> > > * java/awt/Component.java (translateEvent): Translate > AdjustmentEvents to 1.0 Events. > * java/awt/Scrollbar.java (dispatchEventImpl): Set valueIsAdjusting. > Call setValue() before processing event. > * gnu/java/awt/peer/gtk/GtkScrollbarPeer.java (setValues): Check > whether we are currently changing and being called back from the > Scrollbar component. > (setBarValues): New native method. > (postAdjustmentEvent): Mark AdjustmentEvent as user generated. > * native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkScrollbarPeer.c > (Java_gnu_java_awt_peer_gtk_GtkScrollbarPeer_setValues): Renamed to > Java_gnu_java_awt_peer_gtk_GtkScrollbarPeer_setBarValue > * include/gnu_java_awt_peer_gtk_GtkScrollbarPeer.h: Regenerated.
Looks good, thanks. Please commit. > > If you think propagating value changes to awt components through the > event queue and dispatchEventImp() is a good idea then I will change > GtkCheckboxPeer accordingly and audit the other awt gtk-peers. Yes, I've thought about different approaches to this and this seems like the simplest one that doesn't contain race conditions. Tom
