[Freeciv-Dev] Re: (PR#3727) Rectangular selection with right-click-and-drag

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

a-l@xxxxxxx wrote:
> On Wed, 19 Mar 2003 00:14:23 -0800
> "Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx> wrote:
> 
> 
>>I haven't fully figured out this code, but I do have some comments.
>>
>>
>>>+/**************************************************************************
>>> ...
>>> **************************************************************************/
>>> gint move_mapcanvas(GtkWidget *widget, GdkEventButton *event)
>>> {
>>>   update_line(event->x, event->y);
>>>+  if (!hover_state) {
>>>+    update_rect_at_mouse_pos();
>>>+  }
>>>   return TRUE;
>>> }
>>
>>Is hover_state really the correct check here?
> 
> 
> I wouldn't want to draw a rectangle and goto lines at the same time.
> Actually, I think I rather want to disallow changing hover_state when
> rectangle is active, and vica versa.

Hmm, ok.  Well, at a minimum you should compare hover_state against its 
proper enumerated value, not just treat it as a boolean.

>>- Calling refresh_tile_mapcanvas here is quite inefficent and results in 
>>many unnecessary draws (up to 7x as many as needed) in iso-view. 
>>update_map_canvas() would be better (but again you have to consider the 
>>range of the rectangle).
> 
> 
> I think maybe.. upto a dozen or two tiles would need refreshing in
> practical cases, assuming ICS. I don't know what's more efficient.

As long as the rectangle is in map coordinates, you can just use 
update_map_canvas(first_x, first_y, w, h, FALSE) to do the refresh.

>>- You need not pass write_to_screen==TRUE here (with the new flush code 
>>it should pass FALSE instead).
> 
> 
> That's a useful patch, didn't realize it was comitted for non-SDL.
> But, wouldn't that patch precisely make using
>       refresh_tile_mapcanvas(x, y, FALSE)
> less inefficient again?

Only little bit.  We still end up drawing 7x as much as needed, but we 
do the flush step only once.

>>>+++ rectangle/common/map.h   Thu Mar 13 14:36:44 2003
>>>@@ -62,6 +62,8 @@
>>>   struct city *worked;      /* city working tile, or NULL if none */
>>>   unsigned short continent;
>>>   signed char move_cost[8]; /* don't know if this helps! */
>>>+  int selected;             /* Area Selection. 1=city. Client only.
>>>+                               Leave as int. */
>>> };
>>
>>I suppose it is most efficient to add a new field to the map structure, 
>>that is only used by the client.  But why is it an 'int' not a 'bool'?
> 
> 
> I also wanted to pave the way for a Mass Orders patch and so on.
> The next step is to have
> 
>     ptile->selected == 2
> 
> to mean a tile containing owned units is highlighted (with a different
> color sprite). Then, maybe I want to do something like "Assign all
> appropriate units in highlighted tiles to Battlegroup 4", as another
> visual tool.

Makes sense.  In that case you should use an enumeration.

>>You may not 
>>understand the different coordinate systems well enough to provide an 
>>implementation that can correctly handle different topologies with both 
> 
> 
> Quite, I'll have to wrap my mind around those, and look at Mike's
> tools again. (I've seen that some of the harshest critics of Freeciv
> in open forums complain the iso map is not like in Civ II.) Uhm.. I'm
> not quite sure what question to ask. Say I make it so that it looks
> rectangular and flat-on-top, instead of rhomboid, in current iso mode;
> would that still be correct with a new topology?

Yes, the problem is determining at what point wrapping has occurred. 
But I think if you track (x0,y0,width,height) for the rectangle you 
won't have to worry about wrapping issues.  Remember that width and 
height can be negative.

I'm pretty sure if you use the right set of coordinates handling 
wrapping issues will be easy.  If the above idea doesn't work perhaps 
using native coordinates is the only alternative.  What we want to avoid 
is encoding any actual knowledge of the topology into the GUI code.

jason