Re: Fix: NSTextView, -drawRect:

[Fred Kiefer]

Hi Georg,

I will have to reject the changes you suggest for NSTextView the same as
I did some time ago for a similar change by Ludovic. Here is the mail I
did send at that time to Ludovic:

> 
> Hi Ludovic,
> 
> Ludovic Marcotte wrote:
> > 
> > Hi Fred,
> > 
> > I'm playing around GNUstep's text engine in order to fix some bugs in
> > it (so that GNUMail.app may work better).
> > 
> > In NSTextView: - (void) drawRect: (NSRect)rec
> > 
> > we verify if we need to draw the background and if yes, we call:
> > 
> >  [_layoutManager drawBackgroundForGlyphRange: drawnRange
> >                       atPoint: _textContainerOrigin];
> > 
> > in this method, we calculate our bounding rect for clearing the
> > background. Sadly, it doesn't calculate the right bounding rect and it
> > creates severy flickers when you scroll very fast in a NSTextView.
> > 
> > I've not searched for a long time but I made a simple fix that fix the
> > drawing problem.
> > 
> > In NSTextView - (void) drawRect: (NSRect)rec
> > 
> > I've replaced:
> > 
> >   if (_tf.draws_background)
> >     {
> >       [_layoutManager drawBackgroundForGlyphRange: drawnRange
> >                       atPoint: _textContainerOrigin];
> >     }
> > 
> > by
> > 
> >   if (_tf.draws_background)
> >     {
> >       [[self backgroundColor] set];
> >       NSRectFill(rect);
> >     }
> > 
> > Of course, we clear more than what we should do.. but I think it's still
> > faster than calling a lot of methods in GSTextLayout.
> > 
> > If you don't want to use this technique, please tell me so I'll try to
> > find the real problem.
> > 
> 
> I will have to reject your patch. Your patch will result in a better
> display of text, I know that, but still it is wrong.
> First Nicola, and before that myself, did spend a lot of time to get all
> the hard coded drawing stuff out of NSTextView (and NSText).
> GSSimpleLayoutManager is not great, but as soon as someone will come up
> with a better release, everything to use it will already be there.
> Second, the rectangle where we will need to draw the background is
> different from the one handed into drawRect:. 
> (Just insert the follwoing few lines into drawRect: and you will see: 
>         NSLog(@"DrawRect with rectangle %@", NSStringFromRect(rect));
>         NSLog(@"DrawBackground with rectangle %@", 
>               NSStringFromRect([_layoutManager boundingRectForGlyphRange:
> drawnRange 
>                                                inTextContainer: 
> _textContainer]));
> )
> But it is not smaller as you think, and the current code suggests, it
> should be bigger. There might be glyph at the border of the rectangle
> and to redraw this glyph we will have to clear all its background, not
> only part of it. So the real problem is that
> [boundingRectForGlyphRange:inTextContainer:] produces a too small range.
> This is caused by a patch from Georg Fleichmann from July, 19. I don't
> understand why he changed that code, but I normaly not undo changes by
> others even if I think they are nonsens.
> 
> If you still are willing to correct this problem, which I hope is the
> case, you should have a closer look at this method. At least for the
> case where the range include the last character of a line fragment the
> full width up to the end of a line should be used. And for multi lined
> ranges the full line width is needed.
> I cc: this mail to Nicola, perhaps he finds some time to fix this. He
> currently has the best understanding of you text engine.
> 

As you see from this mail the method in NSTextView is the wrong place to
correct this wrong drawing behaviour. GNUstep is already full of those
quick fixes in the wrong place. Such changes are hard to undo later on,
as there may be nobody to remember why it was added in the first place.
On the other hand the change that resulted in this wrong behaviour was
made by you, so perhaps you could find a patch for the method
[boundingRectForGlyphRange:inTextContainer:] that will result in the
correct background being drawn.

Best wishes
 
Fred