Thoughts on terrain imaging

Thoughts on terrain imaging

[mskala]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 8251 bytes --]

I started taking a look at how to implement the idea I was describing back
in September, of taking a chunk of an image file and displaying it to
replace other terrain images.  Once I started digging around in the code,
I found what I think is probably the best way to do it, which runs as
follows:

* Extend the current "subimages" concept in the image-family system.  You
can already specify special types of images, namely "tile", "border",
"connection", and "transition", and you can specify (for regular images,
which could be cell terrain or units) multiple "subimages".  I propose to
add a new special type of image called "hexgrid".  When you specify a
"hexgrid" image, you specify a subrectangle of an image as you normally do
with other images, and you also specify a number of rows and columns (of
hexagonal cells).  Hexagonal chunks are automatically clipped out of the
source image starting from your specified coordinates, to make up a number
of "subimages" for the image family.

* Add data structures to the map to store, for each cell and cell terrain
type, pointers to image families and subimage numbers for
"override" images.

* Change interfaces so that when they go looking for a cell terrain image
for a given cell, if there is an applicable override image at that cell,
they use it instead.  I'm hoping to keep the amount of per-interface code
for this as small as possible; that should be reasonably easy to do,
because it's mostly just a matter of extending the call interfaces make to
say "Hey, what's the image for this terrain type?" to be "Hey, what's the
image for this terrain type *at these cell coordinates*?"

* Add appropriate forms to GDL to allow setting the override information.

I decided to try implementing these ideas, but I wanted to do it in such a
way that I could test each step and see that it was working before moving
on to the next one, so as a very first step, I wrote a simple game module
which would trigger the "subimage" code, figuring that next I could move
on to making it clip out the subimages in a hex grid instead of the
existing one-dimensional linear offset.  The results are at these URLs:
   http://ansuz.sooke.bc.ca/temporary/maptest.g
   http://ansuz.sooke.bc.ca/temporary/override.gif

Those are supposed to work with the existing code base; the "forest" cells
on the map are supposed to be filled with a random mixture of five
different hexagonal chunks taken out of override.gif (which, you will
note, contains a chunk of the existing map with the colours modified and
some minor edits so that I'll be able to see when it's correctly spliced
in).  In testing that, I discovered a bunch of issues with existing code,
with my test files (I haven't touched the code to actually modify it yet
at all), or issues that will come up soon:

* It doesn't work in the TCL interface except on the highest
magnification.  At other magnifications, the affected cells are just
black.  As far as I can tell, for some reason the automatic scale-down
code isn't running.  Isn't it supposed to?  I thought the idea was you
could specify just one resolution of an image and any others would be
automatically generated if necessary; that certainly works for units, but
at least in this case it doesn't seem to work for terrain.

* It doesn't seem to work in the SDL interface at all - I get a segfault,
see attached backtrace.  I haven't tested this extensively at all, so it's
possible it may be unrelated, but the same build of the SDL interface
seems at least sort of stable with other modules, so I suspect it's
something to do with the same issue as in the TCL interface - I didn't
specify more than one resolution for the terrain images, and there are
issues with generating the other sizes automatically.  This could be a
problem because someone who's doing this clip-and-insert thing will only
naturally have one resolution for the image, and it seems like a bad
thing to force the designer to provide multiple sizes of the same image
from outside the game; the resize code really should work.

* It seems like a shame that images can only have 256 colours, and this'll
be a bigger issue when we have image chunks of serious size, where all the
hexes in the chunk have to share the same palette.  Ideally I think this
would be fixed by making XConq capable of loading PNGs, but it looks like
there are 8-bit assumptions built into a lot of code, and at this point I
don't want to try to change them when that's not my main goal.

* It seems like maybe (I'm not quite clear on this) when there are a bunch
of subimages in an image, the GIF file is being re-read for each one.  
That could already be a performance issue for border/connection terrain;
but with satellite images there could be hundreds, or thousands, of
subimages from the same image, and it would really suck to have to spend
the time to load the GIF a thousand times over.  Better to cache it
somehow; but maybe that's already being done, since border/connection
terrain *doesn't* seem to create a huge loading-time problem, and is
already loading 63 subimages per terrain type.

* There is simply a lot of data involved in covering a whole map with
custom images.  There's not really any way around that.  I think it could
be a problem because there are some warnings I don't understand in the
code (imf.c lines 88-90 and uses of the flag defined there) about some
Windows interfaces having limits on "GDI memory", and there are measures
taken to cut features to save on the amount of imagery loaded.  For
instance, if the flag that says "we have to conserve GDI memory" is set,
then when you specify a lot of alternate images (like in my example file,
which specifies five) only the first three are used.  Well, with a patch
of satellite data you pretty much have to load all the subimages - but if
there are hundreds of them, and you can't afford to load a lot of images
in general, that's going to be a problem.  I don't think there's really
much way to avoid this, short of simply disabling the image-override
mechanism entirely when the "must not load too many images" flag is set.  
No matter what scheme we use, those images have to be loaded if we're
going to use them.  Maybe someone who knows more about Windows and what
that flag is actually for could shed some light on this issue.

* Minor thing, but something I'll include in my patch for sure: the (x n x
y) property for image families doesn't seem to permit negative x/y values,
and I think you'd be likely to want to use those far more often than you'd
want to define both of them to be non-zero, which is behaviour it does
permit.  Easy to change, just by changing ">0" to "!=0".  I'm also not
convinced that this parameter is really 100% correctly implemented in
general, but that's not really my problem and it obviously does at least
sort of work because the terrain in the existing "advanced" game uses it.  
I became interested in this bit of code because it's closely related to
the code I want to add for hex-grid clipping.

* If I clip chunks out of an image on a hexagonal grid, what I will get
will be overlapping rectangular images (88x96 or similar) without the hex
masks that I think the current system might produce.  I'm not sure how
interfaces use images or whether that's likely to be a problem; I suspect
it isn't a problem because of the success of my test files with the
current code, but if it turns out to be, then I guess I'll have to write
some code to generate a hexagonal clipping mask for the images.  The issue
is that the interface asks for an image for terrain for a given cell, the
imf code hands back an image, but that is a rectangle that covers the
bounding rectangle of the cell and includes pixels outside the bounds of
the cell, because there's not a hex mask to clip it to the cell... Will
the interface do the clipping?  Or do I have to generate a mask to go with
the image?  I guess I'll find that out by experiment later.  It seems like
something that the interface probably already has to do anyway, so it's
probably not a problem I need to solve; and if I did need to solve it, I
could, so it's no biggie.
-- 
Matthew Skala
mskala@ansuz.sooke.bc.ca                    Embrace and defend.
http://ansuz.sooke.bc.ca/

[-- Attachment #2: Type: TEXT/PLAIN, Size: 2944 bytes --]

mskala@opal:~/xcmap$ gdb /usr/local/src/xconq-latest/xconq/sdl/xconq
GNU gdb 5.3
Copyright 2002 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for details.
This GDB was configured as "i386-slackware-linux"...
(gdb) run -g maptest.g
Starting program: /usr/local/src/xconq-latest/xconq/sdl/xconq -g maptest.g
[New Thread 16384 (LWP 28017)]

              Welcome to X11 Xconq version 7.5pre (November 2004)

Xconq is free software and you are welcome to distribute copies of it
under certain conditions; type "o copying" to see the conditions.
There is absolutely no warranty for Xconq; type "o warranty" for details.
Warning: maptest.g:230: Can't set unknown global named `bigicons'.
Making countries; done.
Growing countries; done.
Assigning players to sides; done.
Setting up AIs; done.

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 16384 (LWP 28017)]
0x0815771f in blacken_masked_area(a_image_family*, a_image*, int, int, int) (
    imf=0x825a9f0, img=0x846fb50, rd=65535, g=0, b=65535) at imf.c:1233
1233            img->rawpalette[4 * black + 0] = black;
(gdb) bt
#0  0x0815771f in blacken_masked_area(a_image_family*, a_image*, int, int, int)
    (imf=0x825a9f0, img=0x846fb50, rd=65535, g=0, b=65535) at imf.c:1233
#1  0x0805e3be in sdl_copy_color_image (imf=0x0, img=0x846fb50,
    surface=0x8473248) at sdlimf.cc:125
#2  0x0805e133 in sdl_interp_image_1 (imf=0x825a9f0, img=0x846fb50,
    subimg=0x846fb50, subi=0, force=0) at sdlimf.cc:107
#3  0x0805df77 in sdl_interp_image (imf=0x825a9f0, img=0x846fb50, force=0)
    at sdlimf.cc:58
#4  0x0805deac in sdl_interp_imf(a_image_family*, a_image*, int) (
    imf=0x825a9f0, img=0x846fb50, force=0) at sdlimf.cc:40
#5  0x08156821 in add_shrunken_image (imf=0x825a9f0) at imf.c:863
#6  0x08157176 in best_image(a_image_family*, int, int) (imf=0x825a9f0, w=0,
    h=1) at imf.c:1037
#7  0x0804f616 in init_terrain_images () at sdlinit.cc:179
#8  0x0804f32d in init_display() () at sdlinit.cc:77
#9  0x0804e0c5 in init_all_displays () at sdlmain.cc:2296
#10 0x0804ef8c in launch_game() () at sdlmain.cc:2854
#11 0x08049fd7 in main (argc=136732824, argv=0x1) at sdlunix.cc:115
#12 0x40414bb4 in __libc_start_main () from /lib/libc.so.6
(gdb) list
115                     launch_game();
116             }
117             if (!option_popup_new_game_dialog)
118                 print_instructions();
119             /* At this point we know we can use popups instead of stdio for
120             warnings and messages and such. */
121             use_stdio = FALSE;
122             /* Go into the main play loop. */
123             ui_mainloop();
124
(gdb) quit

Re: Thoughts on terrain imaging

[Eric McDonald]

Hi Matthew,

  So far, I've had only a brief time to look at your proposal. I 
will give it a more in-depth glance tonight. Some of the areas you 
touched on, I have very little knowledge about, because I was 
content to let Hans deal with them.

  If you want, I will set up a branch in CVS for you sometime in 
the next few days or week. You will need to have a Sourceforge 
user account so that you can be added to the project as a 
developer. I figure this might be easier than you submitting a 
bunch of patches which people would have to apply if they wanted 
to test out your work.

  A couple of quick responses:

On Tue, 23 Nov 2004 mskala@ansuz.sooke.bc.ca wrote:

> * Add data structures to the map to store, for each cell and cell terrain
> type, pointers to image families and subimage numbers for
> "override" images.

For the per-cell case, you may want to implement new layers. I 
believe that the 'aref' and 'aset' macros in 'world.h', IIRC, 
would be worth looking at. (Layer allocation is handled elsewhere.)

> * Change interfaces so that when they go looking for a cell terrain image
> for a given cell, if there is an applicable override image at that cell,
> they use it instead.  I'm hoping to keep the amount of per-interface code
> for this as small as possible; that should be reasonably easy to do,

Yes. The informal UI API exists primarily in 'ui.h', 'kpublic.h', 
and 'ui.c'. The more code that can kept within the common API, the 
better. As I work on the SDL interface, I will probably be moving 
more code into the common API. Just yesterday, I identified 
another chunk that is common, but currently distributed amongst 
the interfaces. The hope is that, eventually, we will have a 
well-defined, formalized UI API, and will thus be able to attract 
more UI developers.

>    http://ansuz.sooke.bc.ca/temporary/maptest.g
>    http://ansuz.sooke.bc.ca/temporary/override.gif

I will look at it tonight.

Regarding the bug reports: if you're offering to fix the bugs, 
then that's great. If you would like some help fixing them, please 
add them to the project's bug tracking system on Sourceforge, and 
I will try to help you clear out some of them.

As far as GDI memory goes, this is something that Hans has been 
complaining about for quite some time. From what little reading 
that I have done, there appears to be an upper limit on the number 
of GDI handles available (and possibly the amount of GDI object 
heap available) in Win32. It varies from version to version. So, 
this is strictly a Win32 consideration, and it may only be 
applicable to the Tcl/Tk interface, because Tk does a lot of 
things behind ones back. I think that we have more control of our 
situation in SDL. And, of course, Curses is irrelevant in this 
regard.

  Thanks,
    Eric

Re: Thoughts on terrain imaging

[mskala]

On Tue, 23 Nov 2004, Eric McDonald wrote:
> user account so that you can be added to the project as a 
> developer. I figure this might be easier than you submitting a 
> bunch of patches which people would have to apply if they wanted 
> to test out your work.

Okay - I'll sign up for one.

> For the per-cell case, you may want to implement new layers. I 
> believe that the 'aref' and 'aset' macros in 'world.h', IIRC, 
> would be worth looking at. (Layer allocation is handled elsewhere.)

Yes - that's one of the things I'm looking at.

> Regarding the bug reports: if you're offering to fix the bugs, 
> then that's great. If you would like some help fixing them, please 
> add them to the project's bug tracking system on Sourceforge, and 
> I will try to help you clear out some of them.

I'd like to spend a little more time trying to figure out what's going on
with these before I decide whether I can fix them easily myself, or want
to file them for other people to look at; I was just commenting about them
on the list in the hope that maybe you (or some other reader) would
instantly recognize either the bug or the mistake I'm making in the GDL to
cause the incorrect behaviour.

That image-rescaling code (which I've looked at a little more since
writing my last message here) seems to be a real dog's breakfast.  There
are a couple of points where it seems to be testing for error conditions
in the very next line after that code that guarantees that the error
condition can't happen, and there are other points where a comment
explaining what's about do be done is followed by a line of code that
clearly does something else.  I think those are signs it's been tweaked
and patched several times in the past, and it may be that the best thing
to do might be to simply rewrite those routines - not a project I had
contemplated as part of the map-image feature addition, but one I'm
willing to tackle if it turns out to be necessary.
-- 
Matthew Skala
mskala@ansuz.sooke.bc.ca                    Embrace and defend.
http://ansuz.sooke.bc.ca/

Re: Thoughts on terrain imaging

[Eric McDonald]

mskala@ansuz.sooke.bc.ca wrote:

> That image-rescaling code (which I've looked at a little more since
> writing my last message here) seems to be a real dog's breakfast.  There
> are a couple of points where it seems to be testing for error conditions
> in the very next line after that code that guarantees that the error
> condition can't happen, and there are other points where a comment
> explaining what's about do be done is followed by a line of code that
> clearly does something else.  I think those are signs it's been tweaked
> and patched several times in the past, and it may be that the best thing
> to do might be to simply rewrite those routines - not a project I had
> contemplated as part of the map-image feature addition, but one I'm
> willing to tackle if it turns out to be necessary.

Yes, parts of that code seemed a bit gnarled, __the few exposures that 
I've had to it. One of the reasons that I've been content to avoid it in 
the past....

I appreciate the fact that you are being open-minded about what sorts of 
  preparatory work may need to be done before getting down to "just 
writing the damned feature". I think that that is a healthy attitude to 
have when dealing with the Xconq sources.

   Best regards,
     Eric

Re: Thoughts on terrain imaging

[Eric McDonald]

mskala@ansuz.sooke.bc.ca wrote:

> I decided to try implementing these ideas, but I wanted to do it in such a
> way that I could test each step and see that it was working before moving
> on to the next one, so as a very first step, I wrote a simple game module
> which would trigger the "subimage" code, figuring that next I could move
> on to making it clip out the subimages in a hex grid instead of the
> existing one-dimensional linear offset.  The results are at these URLs:
>    http://ansuz.sooke.bc.ca/temporary/maptest.g
>    http://ansuz.sooke.bc.ca/temporary/override.gif

> * It doesn't work in the TCL interface except on the highest
> magnification.  At other magnifications, the affected cells are just
> black.  As far as I can tell, for some reason the automatic scale-down
> code isn't running.  Isn't it supposed to?  

Yes, I thought so. I think it does with unit images.

I played around with 'maptest.g' for a little while. If you reduce the 
image cutout size down to 44x48 and change the subimage selection 
offsets to 44, then the subimages show up at normal resolution. It does 
not appear to matter whether the 'terrain' keyword is used or not.

Other thing that I noticed is that the documentation still refers to a 
'bigicons' gvar, but this is not to be found in 'keyword.def' or 
'gvar'.def', and Xconq warns about it. Guess that needs to be taken out 
of the documentation (unless we plan on trying to scale 44x44 don to 
32x32, etc..., if it is not set).

> * It doesn't seem to work in the SDL interface at all - I get a segfault,
> see attached backtrace.  I haven't tested this extensively at all, so it's

I looked at your backtrace; I will attempt to track down the problem 
soon. Unless you take care of it first....

Eric

Re: Thoughts on terrain imaging

[mskala]

On Tue, 23 Nov 2004, Eric McDonald wrote:
> > black.  As far as I can tell, for some reason the automatic scale-down
> > code isn't running.  Isn't it supposed to?  
> 
> Yes, I thought so. I think it does with unit images.

The bug may not be in the scale-down code, it turns out (althogh, as I
said, there *are* plenty of problems with the scale-down code... this just
might not be one of them).  It seems to be in the TK stuff, and I'm
probably going to be able to find and fix it myself.  The general idea
seems to be that there is an extra step that needs to happen to convert an
XConq "image" structure into a "TkImage".  I'm not sure exactly *when*
that happens, because I've found the code that does it but not the code
that calls that code, but the overall result seems to be that the
conversion happens for explicitly specified cell-terrain images but not
for ones generated by scaling.  As a result, the code in tkmap.c gets all
the way to the bottom, where it's about to draw the cell terrain, and then
aborts (leaving black space) because the TkImage pointer is null.

There is some kind of callback that gets set for doing the image-to-Tk
(or, hypothetically, image-to-someotherinterface) conversion, so it may be
that the scale-down code isn't calling that callback when it should, but
the scale-down code *seems* to be calling it, so I think the problem may
really be that the callback isn't doing its job when called; possibly
because it's already been called before, when the explicit images were
loaded, and it sets some kind of flag incorrectly telling itself "I don't
need to run again" - but that's only a guess as to what's going on.

> I played around with 'maptest.g' for a little while. If you reduce the 
> image cutout size down to 44x48 and change the subimage selection 
> offsets to 44, then the subimages show up at normal resolution. It does 
> not appear to matter whether the 'terrain' keyword is used or not.

Yes, and the scaled-*up* images (88x96 generated when you only specify
44x48) seem to work, so the problem seems to be specific to scaling
*down*.
-- 
Matthew Skala
mskala@ansuz.sooke.bc.ca                    Embrace and defend.
http://ansuz.sooke.bc.ca/