Removed duplicate ivar _backgroundColor and started to clean up code in

this file. Better support for key equivalent display.


git-svn-id: svn+ssh://svn.gna.org/svn/gnustep/libs/gui/trunk@24420 72102866-910b-0410-8b05-ffd578937521

ChangeLog

@@ -1,3 +1,29 @@
+2007-01-27  Fred Kiefer <FredKiefer@gmx.de>
+
+	* Headers/AppKit/NSMenuItemCell.h: Remove ivar
+	_backgroundColor. Add ivar _needs_display.
+	* Source/NSMenuItemCell.m (-drawingRectForBounds:): Move push in
+	adjustment to here.
+	* Source/NSMenuItemCell.m (-drawBorderAndBackgroundWithFrame:):
+	Draw the background here and not in [drawInteriorWithFrame:inView:]
+	* Source/NSMenuItemCell.m (-backgroundColor:, -tag,
+	-setNeedsDisplay:, -needsDisplay): New methods.
+	* Source/NSMenuItemCell.m (-setMenuItem:, -setHighlighted:,
+	-init): Mark as needing sizing.
+	* Source/NSMenuItemCell.m (-setMenuView:): Switch off border and
+	move image to left side for horizontal menus.
+	* Source/NSMenuItemCell.m (-titleRectForBounds:,
+	-imageRectForBounds:): Add a few more calls to [calcSize].
+	* Source/NSMenuItemCell.m (-drawTitleWithFrame:inView:): Remove
+	setting of enable state.
+	* Source/NSMenuItemCell.m (-drawInteriorWithFrame:inView:): Add
+	call to separator drawing. Share more of the code between
+	horizontal and vertical menus.
+
+	* Source/NSMenuItemCell.m (-_keyEquivalentString): Determine
+	string to display for key equivalent. 
+	Code by Nikolaus Schaller <hns@computer.org>.
+
 2007-01-25  Fred Kiefer <FredKiefer@gmx.de>
 
 	* Source/NSButtonCell.m (-encodeWithCoder:, -initWithCoder:):

Headers/AppKit/NSMenuItemCell.h

@@ -29,7 +29,6 @@
 #define _GNUstep_H_NSMenuItemCell
 #import <GNUstepBase/GSVersionMacros.h>
 
-#include <AppKit/NSActionCell.h>
 #include <AppKit/NSButtonCell.h>
 #include <AppKit/NSMenuItem.h>
 
@@ -42,9 +41,14 @@ typedef void (*DrawingIMP)(id, SEL, NSRect, NSView*);
   NSMenuItem *_menuItem;
   NSMenuView *_menuView;
 
+  /* If we belong to a popupbutton, we display image on the extreme
+     right */
+  BOOL _mcell_belongs_to_popupbutton;
+
   // Cache
   BOOL _needs_sizing;
-  char _pad1[3];
+  BOOL _needs_display;
+  char _pad[1];
 
   float _imageWidth;
   float _titleWidth;
@@ -55,14 +59,6 @@ typedef void (*DrawingIMP)(id, SEL, NSRect, NSView*);
   NSImage *_imageToDisplay;
   NSString *_titleToDisplay;
   NSSize _imageSize;
-
-  /* If we belong to a popupbutton, we display image on the extreme
-     right */
-  BOOL _mcell_belongs_to_popupbutton;
-  char _pad2[3];
-
-@private
-  NSColor *_backgroundColor;
 }
 
 - (void)setHighlighted:(BOOL)flag;
@@ -77,6 +73,8 @@ typedef void (*DrawingIMP)(id, SEL, NSRect, NSView*);
 - (void)calcSize;
 - (void)setNeedsSizing:(BOOL)flag;
 - (BOOL)needsSizing;
+- (void)setNeedsDisplay:(BOOL)flag;
+- (BOOL)needsDisplay;
 
 - (float)imageWidth;
 - (float)titleWidth;

Source/NSMenuItemCell.m

@@ -62,25 +62,24 @@ static NSImage	*arrowImage = nil;	/* Cache arrow image.	*/
 - (id) init
 {
   [super init];
-  _target = nil;
+  [self setButtonType: NSMomentaryLightButton];
-  _highlightsByMask = NSChangeBackgroundCellMask;
-  _showAltStateMask = NSNoCellMask;
-  _cell.image_position = NSNoImage;
   [self setAlignment: NSLeftTextAlignment];
   [self setFont: [NSFont menuFontOfSize: 0]];
+  [self setNeedsSizing: YES];
 
   return self;
 }
 
 - (void) dealloc
 {
-  RELEASE (_menuItem);
+  RELEASE(_menuItem);
   [super dealloc];
 }
 
 - (void) setHighlighted:(BOOL)flag
 {
   _cell.is_highlighted = flag;
+  [self setNeedsSizing: YES];
 }
 
 - (BOOL) isHighlighted
@@ -99,10 +98,41 @@ static NSImage	*arrowImage = nil;	/* Cache arrow image.	*/
   return [super textColor];
 }
 
+- (NSColor *) backgroundColor
+{
+  NSColor *backgroundColor = nil;
+  unsigned  mask;
+
+  if (_cell.is_highlighted)
+    {
+      mask = _highlightsByMask;
+
+      if (_cell.state)
+	mask &= ~_showAltStateMask;
+    }
+  else if (_cell.state)
+    mask = _showAltStateMask;
+  else
+    mask = NSNoCellMask;
+
+  // Determine the background color
+  if (mask & (NSChangeGrayCellMask | NSChangeBackgroundCellMask))
+    {
+      backgroundColor = [NSColor selectedMenuItemColor];
+    }
+
+  // TODO: Should we also check the ivar _backgroundColor?
+  if (backgroundColor == nil)
+    backgroundColor = [NSColor controlBackgroundColor];
+  
+  return backgroundColor;
+}
+
 - (void) setMenuItem: (NSMenuItem *)item
 {
-  ASSIGN (_menuItem, item);
+  ASSIGN(_menuItem, item);
   [self setEnabled: [_menuItem isEnabled]];
+  [self setNeedsSizing: YES];
 }
 
 - (NSMenuItem *) menuItem
@@ -110,6 +140,11 @@ static NSImage	*arrowImage = nil;	/* Cache arrow image.	*/
   return _menuItem;
 }
 
+- (int) tag
+{
+  return [[self menuItem] tag];
+}
+
 - (void) setMenuView: (NSMenuView *)menuView
 {
   /* The menu view is retaining us, we should not retain it.  */
@@ -120,11 +155,15 @@ static NSImage	*arrowImage = nil;	/* Cache arrow image.	*/
   if ([_menuView isHorizontal] == YES)
     {
       [self setAlignment: NSCenterTextAlignment];
+      [self setBordered: NO];
+      [self setImagePosition: NSImageLeft];
     }
   else
     {
       [self setAlignment: NSLeftTextAlignment];
+      [self setBordered: YES];
     }
+  [self setNeedsSizing: YES];
 }
 
 - (NSMenuView *) menuView
@@ -132,6 +171,35 @@ static NSImage	*arrowImage = nil;	/* Cache arrow image.	*/
   return _menuView;
 }
 
+- (NSString*) _keyEquivalentString
+{
+  NSString *key = [_menuItem keyEquivalent];
+  unsigned int m = [_menuItem keyEquivalentModifierMask];
+  NSString *ucKey = [key uppercaseString];
+  BOOL shift;
+  
+  if ((key == nil) || [key isEqualToString: @""])
+    return key;
+  
+  if ([key isEqualToString: @"\\r"])
+    key = @"RET";
+  else if ([key isEqualToString: @"\\e"])
+    key = @"ESC";
+  else if ([key isEqualToString: @"\\d"])
+    key=@"DEL";
+
+  // shift mask and not an upper case string?
+  shift = (m & NSShiftKeyMask) & ![key isEqualToString: ucKey];
+  
+  key = [NSString stringWithFormat:@"%@%@%@%@%@",
+		  (m & NSControlKeyMask) ? @"^" : @"",
+		  (m & NSAlternateKeyMask) ? @"+" : @"",
+		  shift ? @"/" : @"",
+		  (m & NSCommandKeyMask) ? @"#" : @"",
+		  key];
+  return key;
+}
+
 - (void) calcSize
 {
   NSSize   componentSize;
@@ -203,7 +271,7 @@ static NSImage	*arrowImage = nil;	/* Cache arrow image.	*/
   _titleWidth = componentSize.width;
   if (componentSize.height > neededMenuItemHeight)
     neededMenuItemHeight = componentSize.height;
-  componentSize = [self _sizeText: [_menuItem keyEquivalent]];
+  componentSize = [self _sizeText: [self _keyEquivalentString]];
   _keyEquivalentWidth = componentSize.width;
   if (componentSize.height > neededMenuItemHeight)
     neededMenuItemHeight = componentSize.height;
@@ -234,6 +302,16 @@ static NSImage	*arrowImage = nil;	/* Cache arrow image.	*/
   return _needs_sizing;
 }
 
+- (void) setNeedsDisplay:(BOOL)flag
+{
+  _needs_display = flag;
+}
+
+- (BOOL) needsDisplay
+{
+  return _needs_display;
+}
+
 - (float) imageWidth
 {
   if (_needs_sizing)
@@ -269,8 +347,19 @@ static NSImage	*arrowImage = nil;	/* Cache arrow image.	*/
 //
 // Sizes for drawing taking into account NSMenuView adjustments.
 //
-- (NSRect) imageRectForBounds:(NSRect)cellFrame
+- (NSRect) imageRectForBounds: (NSRect)cellFrame
 {
+  if (_needs_sizing)
+    [self calcSize];
+
+  if (_mcell_belongs_to_popupbutton && _cell.image_position)
+    {
+      // Special case: draw image on the extreme right 
+	cellFrame.origin.x  += cellFrame.size.width - _imageWidth - 4;
+	cellFrame.size.width = _imageWidth;
+	return cellFrame;
+    }
+
   if ([_menuView isHorizontal] == YES)
     {
       switch (_cell.image_position)
@@ -305,14 +394,6 @@ static NSImage	*arrowImage = nil;	/* Cache arrow image.	*/
     }
   else
     {
-      if (_mcell_belongs_to_popupbutton && _cell.image_position)
-	{
-	  // Special case: draw image on the extreme right 
-	  cellFrame.origin.x  += cellFrame.size.width - _imageWidth - 4;
-	  cellFrame.size.width = _imageWidth;
-	  return cellFrame;
-	}
-
       // Calculate the image part of cell frame from NSMenuView
       cellFrame.origin.x  += [_menuView imageAndTitleOffset];
       cellFrame.size.width = [_menuView imageAndTitleWidth];
@@ -369,6 +450,9 @@ static NSImage	*arrowImage = nil;	/* Cache arrow image.	*/
 
 - (NSRect) titleRectForBounds:(NSRect)cellFrame
 {
+  if (_needs_sizing)
+    [self calcSize];
+
   if ([_menuView isHorizontal] == YES)
     {
       /* This adjust will center us within the menubar. */
@@ -442,14 +526,68 @@ static NSImage	*arrowImage = nil;	/* Cache arrow image.	*/
   return cellFrame;
 }
 
+- (NSRect) drawingRectForBounds: (NSRect)theRect
+{
+  if ([_menuView isHorizontal] == YES)
+    {
+      // FIXME: No idea why this adjustment is needed.
+      return NSMakeRect (theRect.origin.x, theRect.origin.y + 2,
+	theRect.size.width, theRect.size.height - 2);
+    }
+  else
+    {
+      if (_cell.is_bordered)
+        {
+	  float yDelta = [_control_view isFlipped] ? 1. : 2.;
+	  unsigned mask;
+	  NSRect interiorFrame;
+	  
+	  if (_cell.is_highlighted)
+	    {
+	      mask = _highlightsByMask;
+		
+	      if (_cell.state)
+		mask &= ~_showAltStateMask;
+	    }
+	  else if (_cell.state)
+	    mask = _showAltStateMask;
+	  else
+	    mask = NSNoCellMask;
+ 
+	  /*
+	   * Special case:  Buttons have only three different paths for border.
+	   * One white path at the top left corner, one black path at the
+	   * bottom right and another in dark gray at the inner bottom right.
+	   */
+	  interiorFrame = NSMakeRect(theRect.origin.x + 1.,
+				     theRect.origin.y + yDelta,
+				     theRect.size.width - 3.,
+				     theRect.size.height - 3.);
+
+	  // pushed in buttons contents are displaced to the bottom right 1px
+	  if ((mask & NSPushInCellMask))
+	    {
+	      interiorFrame
+		  = NSOffsetRect(interiorFrame, 1., [_control_view isFlipped] ? 1. : -1.);
+	    }
+	  return interiorFrame;
+	}
+      else
+        {
+	  return theRect;
+	}
+    }
+}
+
 //
 // Drawing.
 //
 - (void) drawBorderAndBackgroundWithFrame: (NSRect)cellFrame
 				  inView: (NSView *)controlView
 {
-  if ([_menuView isHorizontal] == YES)
+  // Set cell's background color
-    return;
+  [[self backgroundColor] set];
+  NSRectFill(cellFrame);
 
   if (!_cell.is_bordered)
     return;
@@ -471,6 +609,7 @@ static NSImage	*arrowImage = nil;	/* Cache arrow image.	*/
   NSPoint	position;
 
   cellFrame = [self imageRectForBounds: cellFrame];
+  // FIXME: We could call drawImage:withFrame:inView: here, if we knew that the code there is correct.
   size = [_imageToDisplay size];
   position.x = MAX(NSMidX(cellFrame) - (size.width/2.), 0.);
   position.y = MAX(NSMidY(cellFrame) - (size.height/2.), 0.);
@@ -516,13 +655,9 @@ static NSImage	*arrowImage = nil;	/* Cache arrow image.	*/
    * equivalent would be, so we do not display the key equivalent,
    * else they would be displayed one over the other one.
    */
-  else if (![[_menuView menu] _ownedByPopUp])
+  else if (![[_menuView menu] _ownedByPopUp] || (_imageToDisplay == nil))
     {    
-      [self _drawText: [_menuItem keyEquivalent] inFrame: cellFrame];
+      [self _drawText: [self _keyEquivalentString] inFrame: cellFrame];
-    }
-  else if (_imageToDisplay == nil)
-    {
-      [self _drawText: [_menuItem keyEquivalent] inFrame: cellFrame];
     }
 }
 
@@ -562,8 +697,9 @@ static NSImage	*arrowImage = nil;	/* Cache arrow image.	*/
       return;
     }
   
-  size = [imageToDisplay size];
   cellFrame = [self stateImageRectForBounds: cellFrame];
+  // FIXME: We could call drawImage:withFrame:inView: here, if we knew that the code there is correct.
+  size = [imageToDisplay size];
   position.x = MAX(NSMidX(cellFrame) - (size.width/2.),0.);
   position.y = MAX(NSMidY(cellFrame) - (size.height/2.),0.);
   /*
@@ -583,6 +719,8 @@ static NSImage	*arrowImage = nil;	/* Cache arrow image.	*/
 {
   if ([_menuView isHorizontal] == YES)
     {
+      // FIXME: Why do we have separate drawing coce here. The alignement already
+      // gets adjusted when the menu view is set and the text colour looks wrong here.
       id value = [NSMutableParagraphStyle defaultParagraphStyle];
       NSDictionary *attr;
       NSRect cf = [self titleRectForBounds: cellFrame];
@@ -596,11 +734,6 @@ static NSImage	*arrowImage = nil;	/* Cache arrow image.	*/
 	[NSColor controlTextColor], NSForegroundColorAttributeName,
 	nil];
 
-      if ([_menuItem isEnabled])
-	_cell.is_disabled = NO;
-      else
-	_cell.is_disabled = YES;
-
       [[_menuItem title] drawInRect: cf withAttributes: attr];
 
       RELEASE(attr);
@@ -640,59 +773,58 @@ static NSImage	*arrowImage = nil;	/* Cache arrow image.	*/
   if (_buttoncell_is_transparent)
     return;
 
+  if ([_menuItem isSeparatorItem])
+    {
+      [self drawSeparatorItemWithFrame: cellFrame inView: controlView];
+      return;
+    }
+
+  cellFrame = [self drawingRectForBounds: cellFrame];
+  
+  // FIXME: I think all this code belongs into calcSize.
+  if (_cell.is_highlighted)
+    {
+      mask = _highlightsByMask;
+
+      if (_cell.state)
+	mask &= ~_showAltStateMask;
+    }
+  else if (_cell.state)
+    mask = _showAltStateMask;
+  else
+    mask = NSNoCellMask;
+
+  /*
+   * Determine the image and the title that will be
+   * displayed. If the NSContentsCellMask is set the
+   * image and title are swapped only if state is 1 or
+   * if highlighting is set (when a button is pushed it's
+   * content is changed to the face of reversed state).
+   * The results are saved in two ivars for use in other
+   * drawing methods.
+   */
+  if (mask & NSContentsCellMask)
+    {
+      _imageToDisplay = _altImage;
+      if (!_imageToDisplay)
+	_imageToDisplay = [_menuItem image];
+      _titleToDisplay = _altContents;
+      if (_titleToDisplay == nil || [_titleToDisplay isEqual: @""])
+	_titleToDisplay = [_menuItem title];
+    }
+  else
+    {
+      _imageToDisplay = [_menuItem image];
+      _titleToDisplay = [_menuItem title];
+    }
+       
+  if (_imageToDisplay)
+    {
+      _imageWidth = [_imageToDisplay size].width;
+    }
+
   if ([_menuView isHorizontal] == YES)
     {
-      NSColor *backgroundColor = nil;
-
-      cellFrame = [self drawingRectForBounds: cellFrame];
-
-      if (_cell.is_highlighted)
-	{
-	  mask = _highlightsByMask;
-
-	  if (_cell.state)
-	    mask &= ~_showAltStateMask;
-	}
-      else if (_cell.state)
-	mask = _showAltStateMask;
-      else
-	mask = NSNoCellMask;
-
-      /* 
-       * Determine the background color and cache it in an ivar so that the
-       * low-level drawing methods don't need to do it again.
-       */
-      if (mask & (NSChangeGrayCellMask | NSChangeBackgroundCellMask))
-	{
-	  backgroundColor = [NSColor selectedMenuItemColor];
-	}
-      if (backgroundColor == nil)
-	backgroundColor = [NSColor controlBackgroundColor];
-
-      // Set cell's background color
-      [backgroundColor set];
-      NSRectFill(cellFrame);
-      if (mask & NSContentsCellMask)
-	{
-	  _imageToDisplay = _altImage;
-	  if (!_imageToDisplay)
-	    _imageToDisplay = [_menuItem image];
-	  _titleToDisplay = _altContents;
-	  if (_titleToDisplay == nil || [_titleToDisplay isEqual: @""])
-	    _titleToDisplay = [_menuItem title];
-	}
-      else
-	{
-	  _imageToDisplay = [_menuItem image];
-	  _titleToDisplay = [_menuItem title];
-	}
-       
-      if (_imageToDisplay)
-	{
-	  _imageWidth = [_imageToDisplay size].width;
-	  [self setImagePosition: NSImageLeft];
-	}
-	  
       // Draw the image
       if (_imageWidth > 0)
 	[self drawImageWithFrame: cellFrame inView: controlView];
@@ -703,71 +835,6 @@ static NSImage	*arrowImage = nil;	/* Cache arrow image.	*/
     }
   else
     {
-      cellFrame = [self drawingRectForBounds: cellFrame];
-
-      if (_cell.is_highlighted)
-	{
-	  mask = _highlightsByMask;
-
-	  if (_cell.state)
-	    mask &= ~_showAltStateMask;
-	}
-      else if (_cell.state)
-	mask = _showAltStateMask;
-      else
-	mask = NSNoCellMask;
-
-      // pushed in buttons contents are displaced to the bottom right 1px
-      if (_cell.is_bordered && (mask & NSPushInCellMask))
-	{
-	  cellFrame
-	    = NSOffsetRect(cellFrame, 1., [controlView isFlipped] ? 1. : -1.);
-	}
-
-      /*
-       * Determine the background color and cache it in an ivar so that the
-       * low-level drawing methods don't need to do it again.
-       */
-      if (mask & (NSChangeGrayCellMask | NSChangeBackgroundCellMask))
-	{
-	  _backgroundColor = [NSColor selectedMenuItemColor];
-	}
-      if (_backgroundColor == nil)
-	_backgroundColor = [NSColor controlBackgroundColor];
-
-      // Set cell's background color
-      [_backgroundColor set];
-      NSRectFill(cellFrame);
-
-      /*
-       * Determine the image and the title that will be
-       * displayed. If the NSContentsCellMask is set the
-       * image and title are swapped only if state is 1 or
-       * if highlighting is set (when a button is pushed it's
-       * content is changed to the face of reversed state).
-       * The results are saved in two ivars for use in other
-       * drawing methods.
-       */
-      if (mask & NSContentsCellMask)
-	{
-	  _imageToDisplay = _altImage;
-	  if (!_imageToDisplay)
-	    _imageToDisplay = [_menuItem image];
-	  _titleToDisplay = _altContents;
-	  if (_titleToDisplay == nil || [_titleToDisplay isEqual: @""])
-	    _titleToDisplay = [_menuItem title];
-	}
-      else
-	{
-	  _imageToDisplay = [_menuItem image];
-	  _titleToDisplay = [_menuItem title];
-	}
-
-      if (_imageToDisplay)
-	{
-	  _imageWidth = [_imageToDisplay size].width;
-	}
-
       // Draw the state image
       if (_stateImageWidth > 0)
 	[self drawStateImageWithFrame: cellFrame inView: controlView];
@@ -783,37 +850,6 @@ static NSImage	*arrowImage = nil;	/* Cache arrow image.	*/
       // Draw the key equivalent
       if (_keyEquivalentWidth > 0)
 	[self drawKeyEquivalentWithFrame: cellFrame inView: controlView];
-
-      _backgroundColor = nil;
-    }
-}
-
-- (NSRect) drawingRectForBounds: (NSRect)theRect
-{
-  if ([_menuView isHorizontal] == YES)
-    {
-      return NSMakeRect (theRect.origin.x, theRect.origin.y + 2,
-	theRect.size.width, theRect.size.height - 2);
-    }
-  else
-    {
-      if (_cell.is_bordered)
-        {
-	  /*
-	   * Special case:  Buttons have only three different paths for border.
-	   * One white path at the top left corner, one black path at the
-	   * bottom right and another in dark gray at the inner bottom right.
-	   */
-	    float yDelta = [_control_view isFlipped] ? 1. : 2.;
-	    return NSMakeRect (theRect.origin.x + 1.,
-			       theRect.origin.y + yDelta,
-			       theRect.size.width - 3.,
-			       theRect.size.height - 3.);
-	}
-      else
-      {
-	  return theRect;
-      }
     }
 }