[Freeciv-Dev] (PR#8830) update_map_canvas is called way too much

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=8830 >

update_map_canvas is called entirely too often.

No doubt this has been the case for a long time.  But now may be a good 
time to fix it, since only now does (almost) all drawing go through 
update_map_canvas.

For instance, with my current set of options (which includes the map 
grid on), if I change the freelog in update_map_canvas to LOG_NORMAL I 
can find such things as...


1.  A flickering focus unit:

2: update_map_canvas(pos=(223,118), size=(64,48))
2: last message repeated 2 times
2: last message repeated 2 times (total 4 repeats)
2: last message repeated 4 times (total 8 repeats)
2: last message repeated 8 times (total 16 repeats)
2: last message repeated 16 times (total 32 repeats)

This one's not so bad.  But if other units had idle animations we need a 
better way.


2.  Moving a unit:

2: update_map_canvas(pos=(223,92), size=(64,48))
2: update_map_canvas(pos=(223,124), size=(64,48))
2: update_map_canvas(pos=(255,108), size=(64,48))
2: update_map_canvas(pos=(223,92), size=(64,48))
2: update_map_canvas(pos=(287,124), size=(64,48))
2: update_map_canvas(pos=(223,124), size=(64,48))
2: update_map_canvas(pos=(255,140), size=(64,48))
2: update_map_canvas(pos=(287,124), size=(64,48))
2: update_map_canvas(pos=(191,108), size=(64,48))
2: update_map_canvas(pos=(223,92), size=(64,48))
2: update_map_canvas(pos=(255,108), size=(64,48))
2: update_map_canvas(pos=(223,124), size=(64,48))
2: update_map_canvas(pos=(191,140), size=(64,48))
2: update_map_canvas(pos=(159,124), size=(64,48))
2: update_map_canvas(pos=(191,108), size=(64,48))
2: update_map_canvas(pos=(223,124), size=(64,48))
2: update_map_canvas(pos=(191,140), size=(64,48))
2: update_map_canvas(pos=(159,156), size=(64,48))
2: update_map_canvas(pos=(223,156), size=(64,48))
2: update_map_canvas(pos=(223,156), size=(64,48))
2: update_map_canvas(pos=(223,124), size=(64,48))
2: update_map_canvas(pos=(223,124), size=(64,48))
2: update_map_canvas(pos=(287,156), size=(64,48))
2: update_map_canvas(pos=(255,172), size=(64,48))
2: update_map_canvas(pos=(159,156), size=(64,48))
2: update_map_canvas(pos=(191,172), size=(64,48))
2: update_map_canvas(pos=(223,188), size=(64,48))

You'll notice a few repeats here.  Of course the unit is being animated 
here with a bunch of frames; the time spent is user-controlled and 
should be constant.  In this case unit move animation time is 30ms. 
Most of these could be cut out, increasing the animation speed (=> 
number of frames) by a good factor.  (I think only 2 updates should be 
necessary.  Plus the animation, of course, which is just a few copies.)


3.  Moving a unit, with animation disabled:

2: last message repeated 30 times (total 62 repeats)
2: update_map_canvas(pos=(255,204), size=(64,48))
2: update_map_canvas(pos=(255,172), size=(64,48))
2: update_map_canvas(pos=(287,188), size=(64,48))
2: update_map_canvas(pos=(223,188), size=(64,48))
2: update_map_canvas(pos=(191,204), size=(64,48))
2: update_map_canvas(pos=(223,220), size=(64,48))
2: update_map_canvas(pos=(223,188), size=(64,48))
2: update_map_canvas(pos=(255,204), size=(64,48))
2: update_map_canvas(pos=(191,204), size=(64,48))
2: update_map_canvas(pos=(127,204), size=(64,48))
2: update_map_canvas(pos=(127,172), size=(64,48))
2: update_map_canvas(pos=(159,188), size=(64,48))
2: update_map_canvas(pos=(191,204), size=(64,48))
2: update_map_canvas(pos=(159,220), size=(64,48))
2: update_map_canvas(pos=(159,188), size=(64,48))
2: update_map_canvas(pos=(191,204), size=(64,48))
2: update_map_canvas(pos=(223,220), size=(64,48))
2: update_map_canvas(pos=(127,204), size=(64,48))
2: update_map_canvas(pos=(191,236), size=(64,48))
2: update_map_canvas(pos=(191,204), size=(64,48))
2: update_map_canvas(pos=(223,220), size=(64,48))
2: update_map_canvas(pos=(159,220), size=(64,48))
2: update_map_canvas(pos=(191,172), size=(64,48))
2: update_map_canvas(pos=(191,172), size=(64,48))
2: update_map_canvas(pos=(191,204), size=(64,48))
2: update_map_canvas(pos=(191,204), size=(64,48))
2: update_map_canvas(pos=(191,140), size=(64,48))
2: update_map_canvas(pos=(223,156), size=(64,48))
2: update_map_canvas(pos=(255,172), size=(64,48))
2: update_map_canvas(pos=(159,156), size=(64,48))
2: update_map_canvas(pos=(127,172), size=(64,48))
2: update_map_canvas(pos=(0,0), size=(510,386))

I especially like the last one (see #6 below).  This is basically 
unchanged when the map grid is turned off.


4.  Moving a worker:

2: update_map_canvas(pos=(159,81), size=(256,128))
2: update_map_canvas(pos=(246,161), size=(83,22))

Hey, this one's right!  One call to redraw the worker and another to 
redraw the city description.


5.  Adding something to the end of a city's worklist:

2: update_map_canvas(pos=(191,97), size=(256,128))

this one is disturbing because it makes me suspect that any time a city 
packet is received, the city text is redrawn.  Which is bad since the 
current network model is based on the assumption that you can safely 
send a packet many times, and only the "changes" will be sent.


6.  Sentrying a unit:

2: update_map_canvas(pos=(287,124), size=(64,48))
2: update_map_canvas(pos=(0,0), size=(510,386))

Enough said.  This one happens because the mapview window actually gets 
resized and so a total redraw is done.  The resize occurs because the 
panel text and graphics change and so the panel size changes slightly. 
Bad.  This is probably what happened with the unit move (#3) above as well.


7. Scrolling to the right (via scrollbar):

2: update_map_canvas(pos=(478,0), size=(32,386))

Again this is right.


8. Founding a city:

OK, I'm not even going to cut-and paste this one.  It involves 239 
single-tile, almost all of which are single-tile (64x48).  Of course 
each such redraw draws 9 separate tiles, each with a dozen or so 
sprites.  So we have to redraw TENS OF THOUSANDS of sprites when a city 
is founded.  This is beyond amazing.  We could get by with a single 
update() call.


9.  Dying:

Same as #8, not even worth listing.  In this case every tile on the 
mapview is redrawn many times.  The total is 1029 single-tile calls to 
update (from wc -l).  Hundreds of thousands of sprite draws.

2: update_map_canvas(pos=(511,0), size=(-1,48))

this update is entirely off of the mapview.  However because of a bug in 
the clipping within update_map_canvas it actually gets drawn.  And the 
drawing isn't a no-op since some of the tiles that overlap with this 
tile actually _are_ on the mapview.


10.  End-of-turn:

2: update_map_canvas(pos=(127,86), size=(256,128))
2: update_map_canvas(pos=(209,166), size=(92,25))
2: update_map_canvas(pos=(0,0), size=(510,386))

this is with one city and no units.  This is one redraw for the city 
grid, one redraw for the city text (entirely contained in the first one 
of course), and one redraw for no reason whatsoever.


11.  Apollo.

Comparable to #9.  This only seems to happen on the first turn, however 
(I guess delta just skips the packets entirely thereafter?  Or do we not 
get updates each turn in the default ruleset?).

In the past people have complained about how much network bandwidth this 
update takes.  Well, the network bandwidth is nothing compared to the 
redraw time...


12.  Making the mapview larger:

2: update_map_canvas(pos=(0,0), size=(514,386))

meaning this is a full redraw.  The tile_size has changed so some redraw 
is necessary.  But as in update_map_canvas it need only be very small.


13.  Making the mapview smaller:

Same as #12.  Of course in this case no update is needed.


Well, that should be enough for a while.

jason