Re: Recent drawing optimizations

[Alexander Malmberg]

Adam Fedor wrote:
> 
> Ben,
> 
> Do you have an updated patch for this, along the lines of what Alexander
> suggested? Otherwise I can try to whip up something myself.

I've looked a bit closer at this, and I don't think that would be
sufficient to fix the problems. Merely protecting [NSWindow
-setViewsNeedDisplay:] means that [NSView -setNeedsDisplayInRect:] and
others will still be changing the invalid rect ivars from several
threads.

Instead, we should decide and document exactly which methods are
thread-safe, and fix those. I think these would be enough:

[NSView -setNeedsDisplay:]
[NSView -setNeedsDisplayInRect:]

I've attached a patch that make these thread-safe.

Can we assume that the -gui drawing thread is the same thread as -base's
"main thread"? This patch does that. Handling of GSAppKitThread should
probably be cleaned up a bit, too.


However, for the GNUMail problems, this is only a workaround. GNUMail is
not supposed to make any of these calls in anything except the main
thread, so the root cause is that something is being run in the wrong
thread. I haven't looked closely at this, though, since I've been unable
to reproduce it. Ludovic?

- Alexander Malmberg

Index: Source/NSApplication.m
===================================================================
RCS file: /cvsroot/gnustep/gnustep/core/gui/Source/NSApplication.m,v
retrieving revision 1.234
diff -u -r1.234 NSApplication.m
--- Source/NSApplication.m      19 Mar 2003 16:15:44 -0000      1.234
+++ Source/NSApplication.m      1 Apr 2003 12:45:06 -0000
@@ -72,6 +72,9 @@
 #include <AppKit/GSGuiPrivate.h>
 #include <AppKit/GSInfoPanel.h>
 
+/* The -gui thread. See the comment in initialize_gnustep_backend. */
+NSThread *GSAppKitThread;
+
 /*
  * Base library exception handler
  */
@@ -156,6 +159,14 @@
   if (first)
     {
       Class backend;
+
+      /*
+      Remember which thread we are running in. This thread will be the
+      -gui thread, ie. the only thread that may do any rendering. With
+      the exception of a few methods explicitly marked as thread-safe,
+      other threads should not do call any methods in -gui.
+      */
+      GSAppKitThread = [NSThread currentThread];
 
       first = 0;
 #ifdef BACKEND_BUNDLE
Index: Source/NSView.m
===================================================================
RCS file: /cvsroot/gnustep/gnustep/core/gui/Source/NSView.m,v
retrieving revision 1.203
diff -u -r1.203 NSView.m
--- Source/NSView.m     23 Feb 2003 04:38:49 -0000      1.203
+++ Source/NSView.m     1 Apr 2003 12:45:06 -0000
@@ -2082,8 +2082,28 @@
   return _visibleRect;
 }
 
+
+extern NSThread *GSAppKitThread;
+
+- (void) _setNeedsDisplay_helper: (NSNumber *)v
+{
+  [self setNeedsDisplay: [v boolValue]];
+}
+
+/**
+ * As an exception to the general rules for threads and rendering, this
+ * method is thread-safe and may be called from any thread.
+ */
 - (void) setNeedsDisplay: (BOOL)flag
 {
+  if ([NSThread currentThread] != GSAppKitThread)
+    {
+      [self performSelectorOnMainThread: @selector(_setNeedsDisplay_helper:)
+       withObject: [NSNumber numberWithBool: flag]
+       waitUntilDone: NO];
+      return;
+    }
+
   if (flag)
     {
       [self setNeedsDisplayInRect: _bounds];
@@ -2095,14 +2115,31 @@
     }
 }
 
+
+- (void) _setNeedsDisplayInRect_helper: (NSValue *)v
+{
+  [self setNeedsDisplayInRect: [v rectValue]];
+}
+
 /**
  * Inform the view system that the specified rectangle is invalid and
  * requires updating.  This automatically informs any superviews of
  * any updating they need to do.
+ *
+ * As an exception to the general rules for threads and rendering, this
+ * method is thread-safe and may be called from any thread.
  */
 - (void) setNeedsDisplayInRect: (NSRect)invalidRect
 {
   NSView       *currentView = _super_view;
+
+  if ([NSThread currentThread] != GSAppKitThread)
+    {
+      [self performSelectorOnMainThread: 
@selector(_setNeedsDisplayInRect_helper:)
+       withObject: [NSValue valueWithRect: invalidRect]
+       waitUntilDone: NO];
+      return;
+    }
 
   /*
    *   Limit to bounds, combine with old _invalidRect, and then check to see