[Freeciv-Dev] Re: mapview doesn't update properly when we get a city update (PR#1303)

[Raimar Falke <hawk@xxxxxxxxxxxxxxxxxxxxxxx>]

On Tue, Mar 05, 2002 at 09:54:19PM -0800, jdorje@xxxxxxxxxxxxxxxxxxxxx wrote:
> Mike pointed this one out to me a little while ago, but the best fix I 
> can come up with isn't very good.  Nonetheless, it's a pretty 
> significant bug.
> 
> The problem is that any time a city update is sent, if the map grid is 
> enabled the packet code (in client/packhand.c) calls a 
> update_map_canvas() to redraw the city map.  This is sometimes 
> necessary: if the citizens have moved the red-white grid will be drawn 
> differently.  But, update_map_canvas does not redraw the city 
> descriptions.  This means at a minimum that the updated city's 
> description will be overdrawn, and may mean that other cities' 
> descriptions are overdrawn too.
> 
> At end-of-turn, all cities seem to be updated from the server (no doubt 
> this is usually one of those "unnecessary updates" Raimar is planning to 
> get rid of), so all city descriptions vanish.  Ouch!
> 
> It is not enough just to call show_desc_at_tile when we update the city. 
>    Descriptions of other cities may be overdrawn as well - in fact those 
> other cities might not even lie within the city map for the updated 
> city; only their descriptions have to.  This would be difficult to 
> implement in any case, since show_desc_at_tile is a statically declared 
> GUI-dependent function that does not exist for all GUIs.
> 
> We could call show_city_descriptions on these updates, but that would be 
> extremely wasteful.  In particular, at end-of-turn if we get updates for 
> 100 cities it doesn't make much sense to call show_city_descriptions 100 
> times.
> 
> The fundamental problem here is a problem in the design of the drawing 
> system: tiles (including the map grid, etc) are drawn with one of the 
> slew of put_***_tile*** functions (which all draw a _single_ tile), 
> while city names are drawn with the show_city_descriptions() function 
> (which draws _all_ city descriptions).  Most tile updating is done 
> through update_map_canvas_visible(), which draws the city descriptions 
> as you'd expect (albeit with a bit of flicker).  But a fair amount is 
> done outside, and this usually doesn't call show_city_descriptions(). 
> (For instance, moving units and nuke mushrooms do not update the city 
> descriptions.)
> 
> 
> I see two possible solutions.
> 
> The easy one (patch attached) doesn't call the updating code immediately 
> when the city update is received, but queues it for later redrawing. 
> When we're ready to leave the packet-reading code, if there are any 
> queued updates we just redraw the _entire_ visible mapview, and all city 
> descriptions.  (Although we do redraw the entire mapview, which isn't 
> always necessary, it's probably still more efficient since we'll never 
> redraw a single tile more than once.)
> 
> The harder, but more correct, one would hold the city descriptions (and 
> any other "floating" graphics) in a separate pixmap.  Then any time we 
> redraw a tile, we can just redraw the floating-graphics part of that 
> tile over it.  This would require some changes to the structure of the 
> drawing code, and would also involve having a masked pixmap (which may 
> not be possible under all GUIs?).  But it would (help) clean up a lot of 
> other problems, too (like the city descriptions being overdrawn by 
> moving units, city description updates sometimes overwriting themselves, 
> etc.).

The patch is ok. The second solution is a lot harder with no great
advantage.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "- Amiga Y2K fixes (a bit late, wouldn't you say?)"
    -- Linus Torvalds about linux 2.4.0 at 4 Jan 2001