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.
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.
Cheers,
Mark
Index: java/awt/Component.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/awt/Component.java,v
retrieving revision 1.100
diff -u -r1.100 Component.java
--- java/awt/Component.java 15 Feb 2006 20:55:07 -0000 1.100
+++ java/awt/Component.java 21 Feb 2006 13:55:42 -0000
@@ -1,5 +1,6 @@
/* Component.java -- a graphics component
- Copyright (C) 1999, 2000, 2001, 2002, 2003, 2004 Free Software Foundation
+ Copyright (C) 1999, 2000, 2001, 2002, 2003, 2004, 2006
+ Free Software Foundation
This file is part of GNU Classpath.
@@ -40,6 +41,7 @@
import java.awt.dnd.DropTarget;
import java.awt.event.ActionEvent;
+import java.awt.event.AdjustmentEvent;
import java.awt.event.ComponentEvent;
import java.awt.event.ComponentListener;
import java.awt.event.FocusEvent;
@@ -4876,6 +4878,25 @@
0, 0, oldKey, oldMods);
}
}
+ else if (e instanceof AdjustmentEvent)
+ {
+ AdjustmentEvent ae = (AdjustmentEvent) e;
+ int type = ae.getAdjustmentType();
+ int oldType;
+ if (type == AdjustmentEvent.BLOCK_DECREMENT)
+ oldType = Event.SCROLL_PAGE_UP;
+ else if (type == AdjustmentEvent.BLOCK_INCREMENT)
+ oldType = Event.SCROLL_PAGE_DOWN;
+ else if (type == AdjustmentEvent.TRACK)
+ oldType = Event.SCROLL_ABSOLUTE;
+ else if (type == AdjustmentEvent.UNIT_DECREMENT)
+ oldType = Event.SCROLL_LINE_UP;
+ else if (type == AdjustmentEvent.UNIT_INCREMENT)
+ oldType = Event.SCROLL_LINE_DOWN;
+ else
+ oldType = type;
+ translated = new Event(target, oldType, new Integer(ae.getValue()));
+ }
else if (e instanceof ActionEvent)
translated = new Event (target, Event.ACTION_EVENT,
((ActionEvent) e).getActionCommand ());
Index: java/awt/Scrollbar.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/awt/Scrollbar.java,v
retrieving revision 1.27
diff -u -r1.27 Scrollbar.java
--- java/awt/Scrollbar.java 2 Jul 2005 20:32:25 -0000 1.27
+++ java/awt/Scrollbar.java 21 Feb 2006 13:55:42 -0000
@@ -1,5 +1,5 @@
/* Scrollbar.java -- AWT Scrollbar widget
- Copyright (C) 1999, 2000, 2001, 2002, 2003, 2004, 2005
+ Copyright (C) 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006
Free Software Foundation, Inc.
This file is part of GNU Classpath.
@@ -593,13 +593,33 @@
adjustment_listeners.adjustmentValueChanged(event);
}
+ /**
+ * Package private method to determine whether to call
+ * processEvent() or not. Will handle events from peer and update
+ * the current value.
+ */
void dispatchEventImpl(AWTEvent e)
{
if (e.id <= AdjustmentEvent.ADJUSTMENT_LAST
- && e.id >= AdjustmentEvent.ADJUSTMENT_FIRST
- && (adjustment_listeners != null
- || (eventMask & AWTEvent.ADJUSTMENT_EVENT_MASK) != 0))
- processEvent(e);
+ && e.id >= AdjustmentEvent.ADJUSTMENT_FIRST)
+ {
+ AdjustmentEvent ae = (AdjustmentEvent) e;
+ boolean adjusting = ae.getValueIsAdjusting();
+ if (adjusting)
+ setValueIsAdjusting(true);
+ try
+ {
+ setValue(((AdjustmentEvent) e).getValue());
+ if (adjustment_listeners != null
+ || (eventMask & AWTEvent.ADJUSTMENT_EVENT_MASK) != 0)
+ processEvent(e);
+ }
+ finally
+ {
+ if (adjusting)
+ setValueIsAdjusting(false);
+ }
+ }
else
super.dispatchEventImpl(e);
}
Index: gnu/java/awt/peer/gtk/GtkScrollbarPeer.java
===================================================================
RCS file: /cvsroot/classpath/classpath/gnu/java/awt/peer/gtk/GtkScrollbarPeer.java,v
retrieving revision 1.19
diff -u -r1.19 GtkScrollbarPeer.java
--- gnu/java/awt/peer/gtk/GtkScrollbarPeer.java 2 Jul 2005 20:32:12 -0000 1.19
+++ gnu/java/awt/peer/gtk/GtkScrollbarPeer.java 21 Feb 2006 13:55:42 -0000
@@ -1,5 +1,5 @@
/* GtkScrollbarPeer.java -- Implements ScrollbarPeer with GTK+
- Copyright (C) 1998, 1999, 2005 Free Software Foundation, Inc.
+ Copyright (C) 1998, 1999, 2005, 2006 Free Software Foundation, Inc.
This file is part of GNU Classpath.
@@ -39,6 +39,7 @@
package gnu.java.awt.peer.gtk;
import java.awt.Adjustable;
+import java.awt.EventQueue;
import java.awt.Scrollbar;
import java.awt.event.AdjustmentEvent;
import java.awt.peer.ScrollbarPeer;
@@ -69,12 +70,25 @@
public native void setLineIncrement(int amount);
public native void setPageIncrement(int amount);
- public native void setValues(int value, int visible, int min, int max);
+ public void setValues(int value, int visible, int min, int max)
+ {
+ Scrollbar sb = (Scrollbar) awtComponent;
+ if (!sb.getValueIsAdjusting())
+ setBarValues(value, visible, min, max);
+ }
+
+ private native void setBarValues(int value, int visible, int min, int max);
+
+ /**
+ * Called from the native site when the scrollbar changed.
+ * Posts a "user generated" AdjustmentEvent to the queue.
+ */
protected void postAdjustmentEvent (int type, int value)
{
- q().postEvent (new AdjustmentEvent ((Adjustable)awtComponent,
+ Scrollbar bar = (Scrollbar) awtComponent;
+ q().postEvent(new AdjustmentEvent(bar,
AdjustmentEvent.ADJUSTMENT_VALUE_CHANGED,
- type, value));
+ type, value, true));
}
}
Index: native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkScrollbarPeer.c
===================================================================
RCS file: /cvsroot/classpath/classpath/native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkScrollbarPeer.c,v
retrieving revision 1.3
diff -u -r1.3 gnu_java_awt_peer_gtk_GtkScrollbarPeer.c
--- native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkScrollbarPeer.c 9 Feb 2006 16:00:49 -0000 1.3
+++ native/jni/gtk-peer/gnu_java_awt_peer_gtk_GtkScrollbarPeer.c 21 Feb 2006 13:55:42 -0000
@@ -1,5 +1,5 @@
/* gtkscrollbarpeer.c -- Native implementation of GtkScrollbarPeer
- Copyright (C) 1998, 1999 Free Software Foundation, Inc.
+ Copyright (C) 1998, 1999, 2006 Free Software Foundation, Inc.
This file is part of GNU Classpath.
@@ -179,7 +179,7 @@
}
JNIEXPORT void JNICALL
-Java_gnu_java_awt_peer_gtk_GtkScrollbarPeer_setValues
+Java_gnu_java_awt_peer_gtk_GtkScrollbarPeer_setBarValues
(JNIEnv *env, jobject obj, jint value, jint visible, jint min, jint max)
{
void *ptr;
Index: include/gnu_java_awt_peer_gtk_GtkScrollbarPeer.h
===================================================================
RCS file: /cvsroot/classpath/classpath/include/gnu_java_awt_peer_gtk_GtkScrollbarPeer.h,v
retrieving revision 1.6
diff -u -r1.6 gnu_java_awt_peer_gtk_GtkScrollbarPeer.h
--- include/gnu_java_awt_peer_gtk_GtkScrollbarPeer.h 8 Oct 2004 22:16:09 -0000 1.6
+++ include/gnu_java_awt_peer_gtk_GtkScrollbarPeer.h 21 Feb 2006 13:55:42 -0000
@@ -14,7 +14,7 @@
JNIEXPORT void JNICALL Java_gnu_java_awt_peer_gtk_GtkScrollbarPeer_connectSignals (JNIEnv *env, jobject);
JNIEXPORT void JNICALL Java_gnu_java_awt_peer_gtk_GtkScrollbarPeer_setLineIncrement (JNIEnv *env, jobject, jint);
JNIEXPORT void JNICALL Java_gnu_java_awt_peer_gtk_GtkScrollbarPeer_setPageIncrement (JNIEnv *env, jobject, jint);
-JNIEXPORT void JNICALL Java_gnu_java_awt_peer_gtk_GtkScrollbarPeer_setValues (JNIEnv *env, jobject, jint, jint, jint, jint);
+JNIEXPORT void JNICALL Java_gnu_java_awt_peer_gtk_GtkScrollbarPeer_setBarValues (JNIEnv *env, jobject, jint, jint, jint, jint);
#ifdef __cplusplus
}
signature.asc
Description: This is a digitally signed message part
