[Freeciv-Dev] Re: (PR#3424) New flush code

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

Raimar Falke wrote:
> On Tue, Feb 25, 2003 at 05:33:09AM -0800, Jason Short wrote:
> 
>>Raimar Falke wrote:
>>
>>>On Mon, Feb 24, 2003 at 11:45:51PM -0800, Jason Short wrote:
>>>
>>>
>>>>OK, here's a new version - a candidate for a final patch.
>>>>
>>>>I did a full audit of the places where dirty_rect/dirty_all was called, 
>>>>and traced them back to ensure that flush_dirty was called afterward. 
>>>>The result is that a lot of flush_dirty calls had to be added to the GUI 
>>>>code - but I'm now pretty confident of the correctness.
>>>
>>>
>>>>Another change is that instead of calling flush_dirty in thaw_hint and 
>>>>handle_server_processing_finished (or whatever it's called), we instead 
>>>>call it when we leave the network loop - from within 
>>>>unqueue_mapview_updates.
>>>
>>>
>>>What is the reason?
>>
>>Because it is more correct this way?  Rafal put the flush_dirty calls 
>>into the above two functions, but when I studied it closely it became 
>>clear this was inferior.
>>
>>Consider:
> 
> 
>>Case 1.  The client receives a thaw_hint in the middle of a network 
>>series.  It thaws and flushes.  Then we may receive more network data, 
>>and we'll have to flush again.  We've just done an unnecessary flush.
> 
> 
> So this is a performance thing.

The idea is to increase performance (for some GUIs) while maintaining 
correctness.

>>The choice of when to draw is entirely client-dependent; there's
>>nothing the server can do to help us with it.  We know to freeze
>>when we enter the network loop and thaw when we exit it.  This
>>differs from agents, which have to make queries and wait for their
>>results...and (arguably) don't want to recalculate when we're about
>>to get more data, even if there's no data there now.
> 
> 
> The difference is that the agents can't cope with inconsistent
> data. The data may be made inconsistent with one packet and fixed with
> another. The two packets get wrapped in freeze/thaw. For the GUI this
> inconsistent data doesn't cause a problem.

Yes, exactly.  All the drawing code cares about is that it doesn't pass 
control back to the user when the canvas is in an inconsistent 
(unflushed) state.

>>>What does this list tell me? You said above that there are two times
>>>when flush is called: after the network (easy to implement) and during
>>>animation (we have animation for unit combat, unit movement and
>>>mushroom). So what is the reason for this list?
>>
>>No, the two times are:
>>
>>- Before we hand control back to the GUI main event loop.
>>- Before any animation.
>>
>>After the network code is only one case of #1.  For instance, when the 
>>user clicks a mouse to recenter the mapview, we call 
>>center_tile_mapcanvas and then we have to flush_dirty (except in gtk2). 
>>  There are about 6 different cases where this type of thing is done 
>>from the GUI code.
>>
>>So, the long and complicated list traces the callers of 
>>dirty_rect/dirty_all back toward the GUI main event loop.  At some point 
>>in between the two, there needs to be a call to flush_dirty.  You can 
>>verify that this is always the case.
> 
> 
> This sounds cumbersome and error-prone. Can't we put a call to
> gui_callback_finished in these callbacks which are called by the GUI?

It probably will be cumbersome and error-prone, unfortunately.

I don't quite understand your suggestion.  You want every GUI callback 
to call gui_callback_finished when it's done?  Then we could get rid of 
most of the other calls...but that seems even more cumbersome and only 
slightly less error-prone.

Another idea was to pass a write_to_screen value to common functions 
which are called by the GUI (and which don't already flush).  Right now 
the only such function (IIRC) is center_tile_mapcanvas.

jason