Re: Some diffs to optimize drawing

[Benhur Stein]

Sorry for taking so much time to answer.
I was writing a huge mail monday and just before hitting 'Send',
electricity went off :-(
I'll try to rewrite it.

Citando Nicola Pero <address@hidden>:

> > So ... I agree that this change is the 'right' thing to to, but it's quite
> likely it
> > has still been masking some other problems, and we need to look out for
> > and fix them as soon as we can.
> 
> Definitely in the sense that all these changes to locking/drawing are very
> delicate.  Probably the lockFocus didn't break much (unless we've
> forgotten to update a caller); what might have broken things in
> non-trivial ways is that Benhur also tried to remove some of the
> inefficient redrawing in NSMatrix, where a lot of code was redisplay to
> screen immediately instead of marking stuff as needing redisplay (and
> letting them be redisplayed later in a single sweep).  This sort of
> changes is very delicate, selections were involved etc.
> 

I think that it should not be so difficult..
I tried to organize my thoughts on this.
There surely are problems that didn't occur to me.
Please tell me what you think.

I see three kinds of methods involved in drawing (mostly in views/cells):

- methods that do the real drawing (mostly named drawSomething);
- methods that change the state of something so that its visual representation
  should be changed. These methods know what has to be drawn, but not when.
- methods that need things to be on screen immediately.
  Typical case is before asking for an event (tracking mouse) or before sending
  an action.

Of course there are the exceptions or special cases or things I didn't think 
of...
But it seems to me that most methods that affect the display of things can be
classified in one of these types. The classification of a method in the second
or third kind is what poses more problems, as sometimes it is difficult to
identify what should be done. Sometimes the documentation of openstep is not
clear enough and some of the methods (mainly newer ones) are not very 'NeXTish'.

Methods of the first kind are easy: they are called with focus locked,
they just do the drawing and do not call flushWindow. In most cases, they
are called from display.. methods, that do the focus locking and flushing.
They can be directly called by other methods of the same kind (that are drawing
in the same view), but methods of a different kind must lock/unlock focus and
have the window flushed. 

Methods of the second kind are the ones where time can be gained by accumulating
the drawings that need to be done and doing them all at once. So, I think that
these methods should only call setNeedsDisplay... and not do any real drawing.

Methods of the third kind are a bit more complicated and are much less
optimizable. An easy way to implement them is to use methods of the second kind
and call displayIfNeeded when everything must be on screen. Another way is doing
the drawing (directly or calling draw methods). In this case it must take care
of focus and flushing. Yet another way is using a mix of the two (which I think
should be avoided)..

So, when are the views updated?
- automatically: all views that needDisplay are drawn by their window, in two
  different automatic ways (that I could find in current code):
  - when the window is marked as needing display, it asks the runloop to perform
    _handleWindowNeedsDisplay before running the next time. This method calls
    displayIfNeeded, that calls view's displayIfNeeded that does all the
    lockFocus drawRect unlockFocus flushWindow for each view in the hierarchy
    marked as needing display.
  - after each event is processed, NSApplication sends -update to each window
    that needs to be updated. This method also calls displayIfNeeded (if the
    window is marked as needing display), but brackets it with
    disableFlushWindow and enableFlushWindow, flushWindowIfNeeded. (And
    unconditionally flushes the current context, I'm not sure why, probably to
    correct some bug in flushing elsewhere).
  Here I see a problem, the two methods behave differently. I propose (and it is
  in the patch I've sent) to put this bracketing to disable flushwindow inside
  the displayIfNeeded method, because the way it is the flushing in each subview
  will be performed, causing delays and flashing. With this bracketing in
  displayIfNeeded, it is not necessary in -update. The call to
  flushWindowIfNeeded is outside the conditional probably because of the same
  bug in flushing. The problem is that sometimes when these update methods are
  called, the window does not need to be displayed, but needs to be flushed, and
  this seems to be a bug (problably methods of the third kind above that are not
  calling flushWindow). Sometimes in this situation (when flushing is necessary,
  and displaying is not), the update methods are not called, and the flushing
  bug shows. Because of this, I suggested to change NSWindow to ask for the
  execution of this _handleWindowNeedsDisplay method not only when display is
  needed, but also when only flushing is needed (unlockFocus know when flushing
  is needed and already change the window's state to reflect this). The other
  solution is to correct all code (of the third kind) to make sure the window is
  flushed.
  I also wrongly suggested in the patch to remove the -flushWindows from the
  view's display methods. This part of the patch is wrong.
  
- manually: these are the methods of the third kind. They should do the whole
thing and not expect for some automatic displaying or flushing (unless we
implemented the automatic flushing, that would solve some (but not all) of the
cases.)

Time-out, I must go. Hope this is somewhat useful.

Comments?

Benhur