[Freeciv-Dev] Re: different iso drawing algorithms

[Raimar Falke <rf13@xxxxxxxxxxxxxxxxx>]

On Thu, Dec 12, 2002 at 01:07:05PM +0100, Rafa³ Bursig wrote:
> >> > Note that update_map_canvas() hasn't been unified yet (because of
> >> > the drawing algorithm differences).  Nor has one of the drawing
> >> > algorithms been chosen yet...
> >> >
> >> Yes I know about it but I want make SDLClient working with it.
> >> This change make draw code woking.
> >>
> >> Problem here was unification of positioning system ( have diferent
> >> result that my ).
> >
> >> Before unification I have my own system ( only iso ) and code work.
> >> When you make unification part of code use common system ( exp.
> >> drawing city names ) and rest my stystem ( drawing map ).
> >
> >In the medium term (next months) the SDL client should also be able to
> >use non-iso tilesets. This is a goal.
> >
> I rather plan it in long term after fixing draw code :)
> 
> >> This patch drop all code of my draw and positioning systems and use
> >> only common systems but for cost of speed.
> >>
> >> I remove from patch all buffering code ( in common ) and leave only
> >> two new functions  draw_map_widgets() and update_iso_map().
> >>
> >> >> I made here some try to speed up draw full screen map.
> >> >>
> >> >> I think that we should make dedicatet funct. to draw one tile,
> >> >> full map, etc not one unificated function to draw all cases.
> >> >
> >> > I think you are not looking far enough ahead.  IMO the drawing
> >> > primitives the GUI provides should be lower-level than that.  But
> >>> one step at a time...
> 
> >> First I speak here about ISO maps.
> >
> >> Secound :
> >
> >Do you mean "Second point"?.
> >
> no "Secoundo point" :)

Looks like Secoundo is French?!

> >> Yes I know that I don't see all aspect of this probleb but I know
> >> one key in mapdraw code is speed.
> >
> >Yes but speed isn't the only key aspect of map drawing.
> >
> Major key is speed !!!
> Resently I saw Civ3 in action and with all nice gfx and animation speed 
> of this game was poorly.
> This was on celeron 333 and what hapend when I run this game on my 
> P233MMX :)

Yes Civ3 is an example where the game was too slow. But we have
variety. The non-iso mode is on all three tested clients very fast:
4-10ms for a complete redraw. The iso case is slower by an factor of
30!!! So you can have a fast freeciv.

Other key aspects are correctness (the diagonal-road issue is in this
issue), maintainability (no code duplication please), flexibility (no
hard-coded tileset size) and so on.

> >> 1) When we draw more than 1 tile and less that full screen ( I know
> >> about city map )
> >>
> >> 2) As I say before we don't need one unificated draw dunction but
> >more
> >> specialised functions like :
> >>   - draw full screen map. - common
> >>   - draw city map - common or GUI
> >>   - draw one tile ( used by "draw full screen" funct, draw only one 
> >tile ) - GUI
> >>   - draw single tile ( here we must use one of two know algoritms
> >> to draw neighbours tiles ) - GUI
> >>   - we can leave update_map_canvas( ... ) to support other cases.
> >
> >> 3) I'm not much experienced programmer byt have  we any speed up if
> >we
> >> change client api API
> >> From :
> >>
> >> int draw(...) {
> >> ...
> >> if (isometric)
> >> do something
> >> else
> >> do something
> >> }
> >>
> >> To
> >>
> >> int (*DRAW)(...);
> >>
> >> int iso_draw(...)
> >> {
> >> do something
> >> }
> >>
> >> int noiso_draw(...)
> >> {
> >> do something
> >> }
> >>
> >> void init()
> >> {
> >> if (isometric)
> >>   DRAW = iso_draw;
> >> else
> >>   DRAW = noiso_draw;
> >> }
> >
> >This won't you buy anything. You trade one branch miss prediction
> >against a function call (just look at the prologue and epilogue of a
> >normal function).
> >
> Yes and No :)
> This reduce size of function and in some cases like positionig system 
> it is better

Yes.

> becouse we can make them INLINE.

You will find that it is hard to convince the people that inline is a
good thing.

> In draw loop we ofen use something like that
> 
> for( x .... )
>   for( y ... )
>       if (get_canvas_xy(x,y, canvas_x , canvas_y )) or if ( 
> map_pos_to_canvas_pos( ... ) )
>        {
>           draw(...);
>        }
> 
> IMO those functions ( get_canvas_xy( ... ) or 
> map_pos_to_canvas_pos(...) ) should be small and inlined becouse this 
> will reduce jup outside loop.

All this is nice and so but isn't the problem of the factor of 30
mentioned above.

> >> 4) BUFFERING, as i say before we need funct. that save updatet rect
> >and
> >> then flush those rects.
> >> exp.
> >>
> >> #define MAX_RECT 50;
> >> struct rect {
> >>   int x, y, w ,h ;
> >> }saved_rects[MAX_RECTS];
> >> int count_of_rects;
> >>
> >> void save_rect( int x , int y , int w , int h )
> >> {
> >>   if ( count_of_rects < MAX_RECTS ) {
> >>     saved_rects[count_of_rects].x = x;
> >>     saved_rects[count_of_rects].y = y;
> >>     saved_rects[count_of_rects].w = w;
> >>     saved_rects[count_of_rects++].h = h;
> >>   }
> >> }
> >>
> >> flush_rects(void)
> >> {
> >>   if ( count_of_rects > MAX_RECTS ) {
> >>      /* flush fullscreen */
> >>   }
> >>   else
> >>   {
> >>      /* flush rect array */
> >>   }
> >>   count_of_rects = 0;
> >> }
> >
> >I agree we need something to dirty specific regions. While I also
> >agree that the implementation may use an array or a list of rectangles
> >I think that the interface should contain these functions:
> > void mapcanvas_dirty_tile(map_x,map_y);
> map_x , map_y - can be tile position on map.

These are map positions.

> > void mapcanvas_dirty_rectangle(map_x,map_y,width,height);
> if this function work like  "save_rect( int x , int y , int w , int h 
> )" then
> map_x , map_y should be pixel tile positon on screen.( canvas_x , 
> canvas_y )

Since mapcanvas_dirty_tile works on map positions and not canvas
positions mapcanvas_dirty_rectangle should also be. However we can
also add a

   void 
mapcanvas_dirty_canvas_rectangle(canvas_x,canvas_y,pixel_width,pixel_height);

Note that I don't know how often these functions will used. It is
quite possible that one function isn't needed at all.

> > void mapcanvas_dirty_visible();
> > void mapcanvas_dirty_all();
> This is't needend becouse we can have "mapcanvas_flush_fullscreen(void)"

This isn't a good interface design. We have a set of dirty_something
functions and _one_ flush function.

> >than there is a
> > void mapcanvas_dirty_flush();
> >which does the updating. The nice thing is that this can be coupled
> >with FREEZE_HINT/THAW_HINT to given maximum performance (minimal
> >number of refreshes). One problem is that this doesn't work with
> >animations. In this case we have to replace gdk_flush with
> >mapcanvas_dirty_flush.
> >
> No we need another funct to do that :
> void mapcanvas_flush_rectangle(canvas_x,canvas_y,width,height);
> this should be used with animation and make flush of rect.

Why? mapcanvas_dirty_flush can quite well calculate for itself which
canvas area needs to be refreshed.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "SIGDANGER - The System is likely to crash soon"