[Freeciv-Dev] Re: Splitting the tileset PNGs

["Eric S. Raymond" <esr@xxxxxxxxxxx>]

Jason Dorje Short <vze49r5w@xxxxxxxxxxx>:
> I don't see how you can possibly fix this using data reorganization. 
> This has everything to do with changing the low-level drawing code 
> (tilespec.c and gui-xxx/mapview.c) to be able to handle non-0-offset 
> draws without the "special-case" currently used by coasts.

Yes, the low-level code needs to be changed to handle offsets.  The data
reorganization I am talking about is moving the offset declarations into the
tilespec files, so sprites don't have to carry their own padding any more.
That's what will enable us to end duplication.

> And this can be done just as easily with or without having the PNG files 
> split.
> 
> Perhaps I am missing something?

I'm afraid you are.  You're focusing, understandably, on the rendering
stage rather than on the pragmatics of data management.  More
about this below,

> >1. Every single number in the current graphics specfile format --
> >x_top_left, y_top_left, dx, dy, is_pixel_border, and all the
> >coordinate pairs -- is implementation cruft having *nothing* to do
> >with the actual job of rendering the tile.  I find that offensive.
> 
> Indeed.  Perhaps the solution is to redesign the data access method so 
> the specfiles are not used?

Yes.  I've already done this -- it's the whole point of my stage 1 patch.

You have now implicitly agreed with premise one of my design:

        1. Graphics specfiles must die!
 
Remember this for later. :-)

> >2. Tilesets duplicate the hell out of a lot of common icons.
> >This is wasteful of machine resources (which doesn't matter)
> >and of human bandwidth (which does). It's hard for people thinking
> >about improving the tilesets to grok the relationships between them,
> >and hard for artists to be properly credited.
> 
> As I said above, this has nothing to do with having the sprites grouped 
> into large PNGs.

Not in a theoretical sense, no.  But they turn out to be practically connected.
As long as the delivery mechanism for new tilesets is an aggregate, the
natural thing for artists to do will be to *hand* us aggregates with
duplicates in them, exacerbating the problem. We need to break them
of this habit, otherwise the data maintainer (e.g. *me*) is going to spend
a lot of time breaking up these lumps and eliminating duplication. 

This is one argument for my second premise:

        2. Graphics-array PNGs must die!

While there is theoretically a technical fix for the problem of
artists delivering aggregates with duplicates. it would involve
writing a whole extra layer of resolution tools and my doing extra work
on each incoming tileset.  I am not willing to do it this way, because
that would be compensating for complexity by adding more complexity.
Rather, I insist on solving the problem at its source by fixing the
representation and simplifying the code.

> >3. Overlay variants use transparent padding to control where the
> >payload will get dropped on a tile.  This makes them brittle, both
> >with respect to the tileset's cell size and each other.
> 
> Same as above.

Again, you need to think less about rendering and more about the
data-management problem.

> >4. Because tile arrays are big lumps of binary data, merging in
> >incremental changes is hard.  The 12-nations patch is a perfect example
> >of the problems this causes.  I have his flags only as diffs to 
> >an XPM.  To have any chance of recovering them, I'll have to figure
> >out what the version of his XPM has, retrieve it, apply patch(1)
> >and hope the result isn't garbage.
> 
> Good point; ouch.

Remember what I just said about the pragmatics of data management?
This is where they punch you in the face.

Note well:  this problem by itself is sufficient to establish that
graphics-array PNGs must *die*!  It is what convinced me that the
existing representation is broken and started me thinking about
a redesign.

The key point is that the delivery format for flag and shield sprites
must be able to specify incremental updates to the tilesets without
breaking if the tileset has changed since the contributor copied it.

Now, I challenge you to think up a tileset representation that has
this property, is simpler than the existing graphics specfiles, and
does not at some point resolve to one file per tile.

Go ahead.  Try. :-)

> >These are symptoms.  Now let's look one level deeper:
> >
> >A. All the graphics specfile syntax is, is a stuff is an elaborate,
> >clanking set of machinery for implementing name lookup into a tile
> >array.  It's a complicated kluge to implement what should be a 
> >simple name-to-sprite mapping.
> 
> Yes.  But I have't yet seen how the split-PNG system implements this. 
> In the file graphics/trident/unit/awacs.png, how is the lookup done? 
> How is this better than having the same information in a specfile?

Presently the way it works is that the files declaration in the toplevel
tilespec is replaced by a list of directories to search.  To get the sprite
corresponding to the u.awacs tag, you walk through the specified directories 
until you find the last file named u.awacs.png.  That's it.

This is better because there are no magic numbers, just a name lookup.
Graphics specfiles go away completely.  The change to top-level
tilespecs is trivial.  The code is quite a bit simpler.

Simpler code is better.  When you can get rid of a whole layer of complexity
without losing functionality, that's *good*.

Note that the code is simpler precisely because I drafted the filesystem 
to handle my name-to-data mapping.  That's what filesystems are for.

> Once the extra pixel border is not necessary and tx.village is the same 
> between trident and isotrident, additional information will be needed to 
> tell the drawing code where to render the sprite.  This information will 
> be the (x,y) offset from the tile origin.  For instance if we crop a 
> 64x32 sprite down to a *centered* 20x20 sprite, the drawing code could 
> just center it.  But most graphics are not centered, and to require 
> transparent padding to center them is an even bigger hack.  So we need a 
> way to tell the drawing code to offset the sprite by (22,3) (for instance).
> 
> Doesn't this require more specfiles?  If we're going to have specfiles, 
> isn't it easier to have them in small groups - like the current 
> specfiles, but streamlined?  Or alternately: how do you think this could 
> be handled in the split-PNG system?

Your analysis is quite correct.  Two points, though:

1. This will not require reviving graphics specfiles.  The declarations
we'll need can live in the tileset specfiles, where they belong because
they are per-tileset properties.

2. Even if it did involve reviving second-level specfiles, that wouldn't
bother me.  At least this way the data in them would actually *mean*
something!

Basically, there are three cases we need to handle: tile has no offset
(the default), tile is centered, tile has specified offset.  Note that
the variation in offset is largely a function of the sprite's class,
which can be deduced from its name prefix.  So most of this problem
could be handled with declarations like this:

[placement]

centered = {"ts.","tx.","r."} ; Center all resource, extra, & road/rail tiles.
offsets = {"x", "y", "sprite",
           14, 14, "battlemech" "hover}

Later on it might be good to introduce more placement classes than
just "centered".  There's one other natural location: the upper left
corner of the tile frame.  In a non-isometric square tileset this is
0, 0, same as the tile corner, In an isometric or hex tileset, it's
going to be a fixed offset computable from the base tile size.  I 
think of it as the "flag spot", because it's where you want to pin the
upper left hand corner of a flag.   So:

[placement]

centered = {"ts.","tx.","r."} ; Center all resource, extra, & road/rail tiles.
flagspot = {"f."}             ; Pin all flags to the flag spot
offsets = {"x", "y", "sprite",
           14, 14, "battlemech" "hover}

The rendering code should never see these declarations, just computed offsets.
See my next point.

> >B. In the sprites, information about the sprite's role and meaning (the
> >actual pixel bits) is mixed with information about a tileset's rendering 
> >tactics (the padding).
> 
> Yes, this is not good.  But it is not easy to fix, since the parts that 
> need to be changed lie deep in the drawing code.

I don't think it's going to be that hard.  The drawing-code unification
going on now will help.  I've looked into this.  

From your side (the rendering code), the key change will be in how you
interpret the return from your sprite lookup into the hash table.
Instead of getting a bare sprite, you'll get a pointer to struct
containing an offset pair and a sprite.  (Note: later on we may want
to add more hints to this structure.  Extensibility is one sign that a
design is moving in the right direction.)

An immediate consequence of this will be that the special-case iso
flag cruft now in the rendering code can go away.  It will be *my* job
(that is, the tile-loading code's job) to pre-crop the image and compute
the offsets at sprite load time.

> >A database implements a mapping from (role, tileset) to 
> >(spritename, xoff, yoff),  The xoff/yoff is an offset relative to the
> >upper left-hand corner of the tile's base cell.
> 
> OK.  How is this better than mapping (role, tileset) to (spriteloc, 
> xoff, yoff), where spriteloc gives the sprites's position and dimensions 
> in a larger PNG file?  You just think this is unnecessary (it is a lot 
> to edit)?

Remember the previous points:

1. Graphics specfiles must die, because all those otherwise meaningless
numbers are an extra source of places to get things wrong.

2. Graphics-array PNGs must die, because of the incremental-updating problem.

Remember point #4. Speaking as the person who has to deal with the 
consequences, that is in itself sufficient to justify a redesign.

> >My use of sprite directories is just a way to get fast (role, tilespec)
> >lookup in a transparent way. It's better than a binary database when
> >performance is not a pressing issue -- and for our purposes maybe
> >better even when it is.
> 
> But the directories cannot be used for the (role, tilespec) lookup, can 
> they?  Take graphics/trident/unit/u.awacs.  One might think this is 
> (tileset, role, name) = (trident, unit, u.awacs).  But what about all 
> the shared files?  How does a tileset specify using them (or *not* using 
> them)?

Answer: Yes, directories *can* be so used -- I've already implemented this.

The fact that the sprite whose true name is ("trident", "u.awacs")
lives in graphics/trident/units/u.awacs is a historical accident.
Because of the way sprite search works, it could live in a
graphics/common/units directory shared by all tilesets.  The name
u.awacs is the primary key.

So a tileset could declare a list of directories beginning with "common.units" 
and later listing its own directories. The right thing will happen.  Again,
I get a lot of leverage from using the filesystem to do my namespace
mapping.

We can't do this right now because most of the existing sprites have 
padding mixed in and thus are not sharable. But I'll fix that.

> >The new design is better because it's clean.  It gets rid of all the
> >pointless, bogus magic numbers.  It will allow me to properly separate 
> >per-tileset information from per-spritename information, which will
> >be essential for intelligent lookup later on.  And, as a nice site
> >effect, it will eliminate duplication of sprites.
> 
> Hmm, I am still not convinced (although getting closer; #4 is a symptom 
> of the general problem of having everything stuffed together and I am 
> wondering what other symptoms there are).

I've pointed out one other symptom -- having the delivery mechanism for 
tiles be aggregates encourages duplication.  Now I'll point out a third;
it encourages contributors to mix sprite information with padding.

But even if I could think of no other reasons, #4 is a show-stopper
for the existing design.  Combine that with the fact that I need a cleaner
infrastructure to base intelligent lookup on, and this should not
be a difficult call.

Now let's put the shoe on the other foot.  I challenge you -- and
others here -- to either think of a simpler way to solve these
problems or let me do it my way. If I'm willing to do the work
and deliver a clean interface to the rest of the code, the presumption
ought to be that I get to do things in the way I think is correct.

I ask you to notice that the stage 1 patch, which is a long
step in the right direction, changes just one C file.  I
expect my other patches to be almost equally non-intrusive.
I have a plan mapped out in my head for how to get there 
from here by easy stages:

1. Stage one reorganization.  Graphics specfiles go away, sprites
   go to one per file in search directories.

2. I publish the new graphics-hacking HOWTO (already written).  

3. Merge-in of all contributed nations using the fact that incremental
   sprite updates are easy now.

4. Change the drawing code's sprite retrieval to get a sprite+offsets
   structure and use the offsets.  Offsets will initially be returned
   as zero, so this will be a formal change only at first.

5. Move the special cropping for the flags to the load code.   Take
   any other actions needed so that the only knowledge the rendering code
   needs about a sprite is what's in the sprite+offsets structure, which
   is set up by the tilespec-loading code.  (It's okay if the structure
   ends up carrying other hints supplied by the load code; the real point 
   is to not have *any* tileset-dependent logic in the drawing code.)

6. Implement placement declarations in the top-level specfile.  

7. Write a tool to analyze the padding around existing sprites, and translate
   it to placement declarations. Apply it , put the declarations in the
   top-level specfiles, and strip the padding from all sprites.

8. Clean up duplicate sprites.  They'll be easy to detect now that the
   padding is gone.
--
                <a href="http://www.catb.org/~esr/";>Eric S. Raymond</a>