Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
[albireo-dev] Refactoring to a single display filter

It's always been confusing to me that we install display filters for every SwingControl that gets created. This is mostly because the filters both maintain global state and do some work specific to the SwingControl instance. I've refactored the code so that there is a single set of display filters. It's currently part of AwtEnvironment, but that could easily be changed later.

There is still per-SwingControl code that runs on display events, but it is now called from the global filter, rather than by installing a separate filter. This keeps the ordering of event processing correct.

### Eclipse Workspace Patch 1.0
#P org.eclipse.albireo.core
Index: src/org/eclipse/albireo/core/AwtEnvironment.java
===================================================================
RCS file: /cvsroot/technology/org.eclipse.albireo/org.eclipse.albireo.core/src/org/eclipse/albireo/core/AwtEnvironment.java,v
retrieving revision 1.25
diff -u -r1.25 AwtEnvironment.java
--- src/org/eclipse/albireo/core/AwtEnvironment.java	24 Jun 2008 19:22:15 -0000	1.25
+++ src/org/eclipse/albireo/core/AwtEnvironment.java	1 Aug 2008 20:30:38 -0000
@@ -46,6 +46,7 @@
 import org.eclipse.swt.widgets.Event;
 import org.eclipse.swt.widgets.Listener;
 import org.eclipse.swt.widgets.Shell;
+import org.eclipse.swt.widgets.Widget;
 
 
 /**
@@ -123,6 +124,7 @@
         if (!display.isDisposed()) {
             display.addListener(SWT.Dispose, new Listener() {
                 public void handleEvent(Event event) {
+                    
                     removeInstance(display);
                 }
             });
@@ -142,6 +144,7 @@
 
     private final Display display;
     private final AwtDialogListener dialogListener;
+    
 
     // Private constructor - clients use getInstance() to obtain instances
     private AwtEnvironment(final Display display) {
@@ -197,6 +200,8 @@
 
         // Dismiss AWT popups when SWT menus are shown
         initSwingPopupsDismissal();
+        
+        initializeEventFilter();
     }
 
     void dispose() {
@@ -205,6 +210,7 @@
             popupParent.setVisible(false);
             popupParent.dispose();
         }
+        disposeEventFilter();
     }
 
     // ======================= Look&Feel initialization =======================
@@ -537,5 +543,148 @@
         }
     }
 
+    // ----------------------- Event Listening ------------------------------------------
+    
+    private SwtEventFilter swtEventFilter;
+    private final List listeners = new ArrayList();
+    
+    public int getCurrentSwtTraversal() {
+        assert Display.getCurrent() != null; // On SWT event thread
+        return swtEventFilter.currentSwtTraversal;
+    }
+
+    public Widget getActiveWidget() {
+        assert Display.getCurrent() != null; // On SWT event thread
+        return swtEventFilter.activeWidget;
+    }
+
+    public Shell getActiveShell() {
+        assert Display.getCurrent() != null; // On SWT event thread
+        return swtEventFilter.activeShell;
+    }
+
+    public SwingControl getActiveEmbedded() {
+        assert Display.getCurrent() != null; // On SWT event thread
+        return swtEventFilter.activeEmbedded;
+    }
+
+    public int getLastSwtTraversal() {
+        assert Display.getCurrent() != null; // On SWT event thread
+        return swtEventFilter.lastSwtTraversal;
+    }
+
+    public Widget getLastActiveWidget() {
+        assert Display.getCurrent() != null; // On SWT event thread
+        return swtEventFilter.lastActiveWidget;
+    }
+
+    public SwingControl getLastActiveEmbedded() {
+        assert Display.getCurrent() != null; // On SWT event thread
+        return swtEventFilter.lastActiveEmbedded;
+    }
+    
+    public void addEventFilter(Listener filter) {
+        listeners.add(filter);
+    }
+    
+    public void removeEventFilter(Listener filter) {
+        listeners.remove(filter);
+    }
+    
+    protected void fireEvent(Event event) {
+        for (Iterator iterator = listeners.iterator(); iterator.hasNext();) {
+            Listener listener = (Listener)iterator.next();
+            listener.handleEvent(event);
+        }
+    }
+    
+    protected void initializeEventFilter() {
+        swtEventFilter = new SwtEventFilter();
+        display.addFilter(SWT.Activate, swtEventFilter);
+        display.addFilter(SWT.Deactivate, swtEventFilter);
+        display.addFilter(SWT.Traverse, swtEventFilter);
+    }
+
+    protected void disposeEventFilter() {
+        display.removeFilter(SWT.Activate, swtEventFilter);
+        display.removeFilter(SWT.Deactivate, swtEventFilter);
+        display.removeFilter(SWT.Traverse, swtEventFilter);
+    }
+
+    protected class SwtEventFilter implements Listener {
+        
+        int currentSwtTraversal = SWT.TRAVERSE_NONE;
+        Widget activeWidget;
+        Shell activeShell;
+        SwingControl activeEmbedded;
+
+        int lastSwtTraversal = SWT.TRAVERSE_NONE;
+        Widget lastActiveWidget = null;
+        SwingControl lastActiveEmbedded = null;
+        
+        public SwtEventFilter() {
+            activeWidget = display.getFocusControl();
+            activeShell = display.getActiveShell();
+            if ((activeWidget instanceof SwingControl) && ((activeWidget.getStyle() & SWT.EMBEDDED) != 0)) {
+                activeEmbedded = (SwingControl)activeWidget;
+            }
+        }
+        
+        public void handleEvent(Event event) {
+            Widget widget = event.widget;
+            switch (event.type) {
+            case SWT.Activate:
+                activeWidget = widget;
+                
+                // Track the currently active shell. This is more reliable than
+                // depending on Display.getActiveShell() which sometimes returns an 
+                // inactive shell. 
+                if (widget instanceof Shell) {
+                    activeShell = (Shell)widget;
+                }
+                
+                if ((widget instanceof SwingControl) && ((widget.getStyle() & SWT.EMBEDDED) != 0)) {
+                    activeEmbedded = (SwingControl)widget;
+                }
+                
+                if (currentSwtTraversal != SWT.TRAVERSE_NONE) {
+                    lastSwtTraversal = currentSwtTraversal;
+                }
+                currentSwtTraversal = SWT.TRAVERSE_NONE;
+                break;
+
+            case SWT.Deactivate:
+                if (activeWidget != null) {
+                    lastActiveWidget = activeWidget;
+                    activeWidget = null;
+                }
+                
+                if (event.widget instanceof Shell) {
+                    activeShell = null;
+                }
+                
+                if ((widget instanceof SwingControl) && ((widget.getStyle() & SWT.EMBEDDED) != 0)) {
+                    if (activeEmbedded != null) {
+                        lastActiveEmbedded = activeEmbedded;
+                        activeEmbedded = null;
+                    }
+                }
+
+                break;
+                
+            case SWT.Traverse:
+                currentSwtTraversal = event.detail;
+                
+                break;
+            }
+            
+            // Propagate to any listeners
+            fireEvent(event);
+            
+            
+        }
+
+    }
+
     // ========================================================================
 }
Index: src/org/eclipse/albireo/internal/FocusHandler.java
===================================================================
RCS file: /cvsroot/technology/org.eclipse.albireo/org.eclipse.albireo.core/src/org/eclipse/albireo/internal/FocusHandler.java,v
retrieving revision 1.22
diff -u -r1.22 FocusHandler.java
--- src/org/eclipse/albireo/internal/FocusHandler.java	31 Jul 2008 21:19:00 -0000	1.22
+++ src/org/eclipse/albireo/internal/FocusHandler.java	1 Aug 2008 20:30:38 -0000
@@ -27,6 +27,7 @@
 import javax.swing.text.Caret;
 import javax.swing.text.JTextComponent;
 
+import org.eclipse.albireo.core.AwtEnvironment;
 import org.eclipse.albireo.core.SwingControl;
 import org.eclipse.albireo.core.ThreadingHandler;
 import org.eclipse.swt.SWT;
@@ -55,10 +56,6 @@
     private static boolean synthesizeMethodInitialized = false;
     private static Method synthesizeMethod = null;
 
-    private static Composite activeBorderless = null;
-    private static Composite activeShell = null;
-    private static boolean firstInstance = true;
-
     // ========================================================================
 
     private final Frame frame;
@@ -69,9 +66,9 @@
     private int pendingTraverseOutSeqNum = 0;
     private int currentTraverseOutSeqNum = 0;
     private int extraTabCount = 0;
-    private boolean isActiveSwt;
     private boolean isFocusedSwt;
     private boolean pendingDeactivate = false;
+    private AwtEnvironment env;
     
     // Listeners
     private KeyEventDispatcher keyEventDispatcher = new AwtKeyDispatcher();
@@ -89,23 +86,11 @@
         this.borderless = borderless;
         this.frame = frame;
         display = swingControl.getDisplay();
+        env = AwtEnvironment.getInstance(display);
         
         getSynthesizeMethod(frame.getClass());
 
-        // TODO: optimize by adding just one pair of filters for all SwingControls
-        display.addFilter(SWT.Activate, swtEventFilter);
-        display.addFilter(SWT.Deactivate, swtEventFilter);
-        display.addFilter(SWT.Traverse, swtEventFilter);
-        if (firstInstance) {
-            activeShell = display.getActiveShell();
-            firstInstance = false;
-        }
-        
-        // We won't get an Activate event for the newly created control, so update
-        // the static tracking field here. 
-        if (display.getFocusControl() == borderless) {
-            activeBorderless = borderless;
-        }
+        env.addEventFilter(swtEventFilter);
         
         frame.addWindowFocusListener(awtWindowFocusListener);
         KeyboardFocusManager.getCurrentKeyboardFocusManager().addKeyEventDispatcher(keyEventDispatcher);
@@ -115,9 +100,7 @@
     }
     
     public void dispose() {
-        display.removeFilter(SWT.Activate, swtEventFilter);
-        display.removeFilter(SWT.Deactivate, swtEventFilter);
-        display.removeFilter(SWT.Traverse, swtEventFilter);
+        env.removeEventFilter(swtEventFilter);
         frame.removeWindowFocusListener(awtWindowFocusListener);
         KeyboardFocusManager.getCurrentKeyboardFocusManager().removeKeyEventDispatcher(keyEventDispatcher);
         borderless.removeFocusListener(swtFocusListener);
@@ -247,6 +230,7 @@
         if (verboseTraverseOut) {
             trace("SWT: traversing, control=" + focusControl);
         }
+        SwingControl activeBorderless = env.getActiveEmbedded();
         if ((focusControl == null) && (activeBorderless != null)) {
             focusControl = activeBorderless;
             if (verboseTraverseOut) {
@@ -294,7 +278,7 @@
             // the SwingControl to be skipped while tabbing. Try to fix it here by resetting
             // the forceFocus result back to true when SWT events indicate we really do have 
             // focus. 
-            if (!result && isActiveSwt && isFocusedSwt) {
+            if (!result && (env.getActiveWidget() == borderless) && isFocusedSwt) {
                 // Force focus should have returned true
                 result = true;
                 if (verboseTraverseOut) {
@@ -432,6 +416,9 @@
     public void activateEmbeddedFrame() {
         assert Display.getCurrent() != null;
         
+        Shell activeShell = env.getActiveShell();
+        SwingControl activeBorderless = env.getActiveEmbedded();
+        
         if (!borderless.isDisposed() &&
                 
             // Make sure that this control is in the active shell, so focus is not stolen from other windows.
@@ -503,35 +490,13 @@
 
     protected class SwtEventFilter implements Listener {
         
-        private int currentSwtTraversal = SWT.TRAVERSE_NONE;
-
         public void handleEvent(Event event) {
-            // Track Traversals
-            if (event.type == SWT.Traverse) {
-                currentSwtTraversal = event.detail;
-                // No further handling needed
-                return;
-            }
-            
-            // Track the currently active shell. This is more reliable than
-            // depending on Display.getActiveShell() which sometimes returns an 
-            // inactive shell. 
-            if ((event.type == SWT.Activate) && (event.widget instanceof Shell)) {
-                activeShell = (Shell)event.widget;
-            }
-            if ((event.type == SWT.Deactivate) && (event.widget instanceof Shell)) {
-                activeShell = null;
-            }
-            
             // Handle activation of the SWT.EMBEDDED composite. Track the currently active one.  
             if (event.widget == borderless) {
                 switch (event.type) {
                 case SWT.Activate:
-                    isActiveSwt = true;
-                    activeBorderless = borderless;
-                    
-                    // The currentSwtTraversal will change below. Save its value for the asyncExecs
-                    final int swtTraversal = currentSwtTraversal;
+                    // The lastSwtTraversal may change before it is used. Save its value for the asyncExecs
+                    final int swtTraversal = env.getLastSwtTraversal();
                     
                     // We use asyncExec to defer the activation and focus setting in the underlying AWT frame. 
                     // This allows proper handling of the case where focus is briefly 
@@ -560,9 +525,6 @@
                     break;
 
                 case SWT.Deactivate:
-                    isActiveSwt = false;
-                    activeBorderless = null;
-                    
                     // On Windows, when the SwingControl temporarily loses focus to an ancestor, and 
                     // that ancestor then assigns focus right back to it, the SwingControl receives only a 
                     // Deactivate event and not a subsequent Activate event. This causes the embedded 
@@ -583,9 +545,6 @@
                     
                 }
             }
-            if (event.type == SWT.Activate) {
-                currentSwtTraversal   = SWT.TRAVERSE_NONE;
-            }
         }
 
     }

Back to the top