Animation system

beegee · December 29, 2011, 2:20pm

Dear programmers,

before I can really start with refactoring the battle interface, the ground part have to be placed. The first part with the introduction of PCH and coding guidelines and the battle interface/ui reorganization is finsihed, but now let’s talk about the animation system. The question of supporting 32bit images in battle and if it could be possible to merge CCreatureAnim and CCreatureAnimation is not fully resolved.

I’ve started with creating a small project to measure the performance of the various techniques of displaying animations which are present at the moment and could be present in the future. I’ve mainly extracted the parts of the VCMI project to get some animation system working. If anyone is interested in that VS project, please let me know via PM.

Animation performance

I’ve measured three facts: The loading time, the amount of RAM used and the drawing performance.
There are 3 different systems present in VCMI, I call it the standard SDL animation system, the compressed RLE animation system and the direct draw animation system. For the tests, I’ve compared those systems with each other and a system which is not present in VCMI currently the Opengl animation system. It was interesting to see that every system resulted in different values which I measured. Let me show now the results first, I’ll talk about how the techniques work later.

Amount of RAM used:

Objective: I’ve loaded 20x times the same DEF file. Values are the arithmetic average of 3 test runs.

BOAR.DEF (around 88x103 pixels images with 12 frames, HOLDING, STANDING anim type)
Standard SDL: 2800KB Compressed RLE: 2031KB Direct draw: 7112KB

CAVALR.DEF (around 98x103 pixels images with 4 frames, HOLDING, STANDING anim type)
Standard SDL: 1250KB Compressed RLE: 884KB Direct draw: 7864KB

OGRE.DEF (around 43x85 pixels images with 8 frames, HOLDING, STANDING anim type)
Standard SDL: 1106KB Compressed RLE: 1072KB Direct draw: 4160KB

For the Opengl test, I’ve loaded a 1024x1024 pixels image(will explain later) 20x times:
Opengl: 82 764KB

Loading time:

Objective: Same as above, now measuring time.

BOAR.DEF
Standard SDL: 136ms Compressed RLE: 140ms Direct draw: 111ms

CAVALR.DEF
Standard SDL: 131ms Compressed RLE: 133ms Direct draw: 110ms

OGRE.DEF
Standard SDL: 117ms Compressed RLE: 117ms Direct draw: 95ms

Opengl: 897ms

Drawing time:

Objective: 15 000x calls to nextFrame, show,… to draw one frame of the animation

BOAR.DEF
Standard SDL: 1005ms Compressed RLE: 1349ms Direct draw: 1916ms

CAVALR.DEF
Standard SDL: 867ms Compressed RLE: 1428ms Direct draw: 2084ms

OGRE.DEF
Standard SDL: 497ms Compressed RLE: 777ms Direct draw: 954ms

Image to draw had consistently a size of 100x100 pixels.
Opengl: 53ms

Summary:
Compressed RLE has less RAM usage compared to standard SDL, but needs more time to draw. Direct draw has the highest RAM usage and the highest draw time(slow!), but has the fastest loading time. Opengl has a high RAM usage, slow loading time but a very short draw time.

Explanation of those different animation techniques

Standard SDL:

Capabilities: Can be used only for 8bit images. It needs to change the color palette to support player colored images. Can’t be used in battles, because the yellow and blue glowing effect can’t be realized efficiently. 24bit or even 32bit support would theoritically be possible, but definitely not practically. High RAM usage + slow drawing time, here is where hardware-accelerated graphics is needed.
Loading: For every frame of a group it creates a SDL_Surface object. Only one group or frame is loaded within the same time.
Drawing: Simple SDL Blit At

Compressed RLE:

Capabilities: Same as above.
Loading: For every frame of a group it creates a specific CompImage object. Only one group or frame is loaded within the same time.
Drawing: Special Blit At(perhaps decompress image)

Direct draw:

Capabilities: Can be used only for 8bit images. Supports shadow drawing + glue effect + player colored images.
Loading: Loads the whole DEF file (every frame, group)
Drawing: Decompress image, pick color from palette, put pixel

Opengl:

Capabilites: Support for 32bit images. Shadow effect can be directly applied to the image. Glue effect and player colored images has to be done with fragment/pixel shaders. You could even display bloodthirstiness effect and some other effects like display frames with embossed + black and white effects,…
Loading: Loads one big 1024x1024 image + ANIM definition file where all frames are placed. No color palette is needed, no overhead. Specific image+anim data file has to be generated once a time.
Drawing: Fast hardware blit, Shader effects(post processing)

Summary:
We need every technique in the short-mid-to-long term future. Opengl is definitely something we should look into, BUT not before release 1.0 as it’s much much work to fully support Opengl rendering. RAM usage is not that high which is the most critic point of that technique. Look: We have max 14 stacks in battle, that means 102410244*14 = around 57MB. That’s nothing today. Yesterday, I informed me about current graphics card, standard is 1GB video ram, better cards have 2GB RAM and the best even 4GB RAM. On mobile phones we could easily stick to the current animation approach of VCMI. I could talk much more about this topic, but I think it’s not the time yet.
So we can’t get rid of any of those explained animation systems, but it’s necessary to provide a common base for these techniques.

Suggestion to a new class design
Currently we have a IImage interface which gets derived by SDLImage and CompImage. CAnimation creates and manages those animations, it supports loading of frames/groups. Then we have CShowableAnim to show multiple frames of an animation and CAnimImage for displaying onnly one frame of an animation both get derived by CIntObject. CCreatureAnim has the possibility to display different animation types in a queue. CCreatureAnimation gets derived by CIntObject and implements DEF loading on its self.

My approach would be and I just talk about the raw picture of it, no details, would be too much:
IAnimation is an interface with a small bunch of methods: showFrame(frameNr, groupNr), loadFrame, unloadFrame, loadGroup, unloadGroup, loadAll, unloadAll, init(ibstream, will explain later). Those load/unload methods can also be empty e.g. for direct draw and opengl.
SDLAnim, CompAnim, DirectAnim, in the future OpenglAnim derive from that interface. They manage resource and drawing handling. CShowableAnim derives from CIntObject and has an instance of a anim class. This instance cannot be accessed from outside. That CShowableAnim class manages showing one frame, several frames and presenting several different animation types in queue altogether. It implements the IUpdatable interface aswell to support a time based animation system.

Input byte stream(ibstream):
It can be created from file or an ui8 array. It provides safe access and automatic scrolling of the read pointer. It also supports byte order handling(little endian, big endian) and the RAII principle. It could be used in many places, so I think such a class would be useful.

Tow: What’s about inserting using boost::shared_ptr to the Global.h? It’s awful to write boost::shared_ptr all the time, it has a unique identifier and when we switch some time from boost::shared_ptr to std::tr1::shared_ptr or std::shared_ptr it would be no problem.

Ivan: Does the resource handling of IImage with increaseRef and decreaseRef really work? In my tests, no loaded images got used a second time.

That’s for now the first part. A second part with URI handling, resource handling is coming in the next days, I don’t want to blow you away.

Have a nice day,
beegee

Ivan · December 29, 2011, 6:26pm

Amount of RAM used:
That difference is (mostly) coming from different way to create surface from def file - all creatures have large borders (~100px on each side of the image) with same total size (450x400px).
DefHandler will create huge 450x400 image and fill borders with key color. SDLImage will shift borderless surface before blitting.

Loading time:
Try launching profiler - IIRC main bottleneck was deflating data from .lod file. Def-to-surface conversions should be relatively fast.

No decompression here.

Looks OK.

Just wondering - what’s wrong with “typedef boost::shared_ptr MyClassPtr;”?

Yes but only if same CAnimation object is used. Resource handling with URI’s can fix this + it will be better to use shared pointers here instead.

val-gaav · December 29, 2011, 11:08pm

Please at some point consider:

-support for 32 bit graphic
-support for non def graphics as in support for graphics as png directly from folders with some kind of defined name scheme for anims. Def and its tools really sucks as far as working on your own animations goes. I’ve tried that in past with wog and it’s horrid and takes a lot of time that would be better spend on doing real work.
-AFAIK right now there is a set frame number for some animations (If I remember correctly 6 or frames for each creature anim). It would be nice if there was a possibility of having more frames if animation (be it movement or attack) needs that.
-possibility of different size of creature animations, different size of battle hex (for supporting different native resolutions then 800x600 means: larger creature animations, larger battlefields, possibly widescreen battlefields)

This is from a point of view of a modder who would like to insert new graphics and do things with vcmi engine.

Tow · December 30, 2011, 3:50am

Interesting reading. Nice job with measuring performance.

How have you checked the RAM usage? The results seem surprising to me.
I assume that by direct draw you mean usage of CCreatureAnimation. That class basically loads the whole def file into RAM and the purpose for its creation was basically saving RAM (CDefHandler would create uncompressed SDL_Surfaces for every frame in creature animation def). Even if now we have SDLImage and CompImage it should be bigger or comparable in memory usage to CCreatureAnimation.
And there certainly should not be 2x differences. By using 7zip I can compress def file by ~30%, how can RLE make it three times smaller then?

Not necessarily: add commanders, war machines, arrow turrets and summoned creatures.

I agree, this issue has been raised several times. Basically modern CPUs are fast enough to run current SDL-based drawing. The slow-downs are an issue on mobile devices but there is not enough GPU memory to support a naive OpenGL implementation.
I guess we’ll eventually have to make transition to OpenGL but the time is not right yet.

Ok. I guess smart pointers are a good practice now and good practices should be possibly convenient to use.

AFAIK there is no such limit in VCMI.

Ivan · December 31, 2011, 10:39am

Drawing time:
Not a completely valid comparison - “Standard SDL” can’t use alpha-channel in 8-bit surfaces. This one of the reasons why we have CompImage, CCreatureAnimation and “blit8bppto24” or something like that. (Another reason is memory usage)

That’s how that 32-bit menu was implemented. More complicated for battles but possible.

All VCMI code loops over all frames in the group. If not - then this is a bug.

All of this was mentioned before. Really complicated even codewise-only. Not to mention required number of hi-res content (scaling is either slow or ugly) Probably should go in post-post-1.0 stuff.

Not to mention that def files are already RLE-compressed… And memory usage looks too similar to the one of DefHandler.

@beegee:
Can you tell more about URI and input byte stream plans? (part about filesystem interaction)
What your plans regarding DefHandler? It definitely doesn’t fit in your system ATM.
Assuming that DirectAnim is current CCreatureAnimation - do we really need those? If you need access to palette you can use CompAnim and loading won’t be limited to def’s.

beegee · December 31, 2011, 2:42pm

@Ivan

SDLImage loads DEF file via LodHandler and creates only images of real sprite frame sizes(not 450x400px) the same way how CompImage does. The difference is coming from Debug and Release build of Visual Studio.

I’ll measure the amount of RAM used a second time with Debug build configuration, this seems to be much more accurate. Perhaps in release build a part of memory usage information gets lost. I did some tests to find out what exactly is the problem here. In release build malloc(…) doesn’t adjust actual used memory, to be more precise that proprietary ms function, but the task manager gives the same results too.

void * mem = malloc(100*100*12*20*3);
memset(mem, 0, 100*100*12*20*3);

Test this simple code once with memset and once without in release configuration under VS. Then have a look into TaskManager you’ll see that once this simple app have a RAM usage of 400-500 KB and another time 7400KB or so. CompImage allocates enough RAM when creating its images and calls memset or memcpy only at specific areas / locations of the allocated RAM. That’s why it shows less RAM then the standard SDL method, normally Comp method should have a higher RAM usage. As said above I’ll measure every technique again in debug mode this seems to be accurate.

I’m loading .def files directly, so no deflating from .lod file.

@Tow

SDL and Comp technique only load a group of frames as I mentioned in my first post. 12 Frames for CBOAR.DEF and so on. So comparison between those 2 techniques and Direct animation wasn’t really valid, because Direct anim loads more or less all animation frames as a DEF file. This is needed because in battles loading group and unloading group is just not efficient and reasonable. When I’ll measure RAM usage again I’ll load all frames with SDL and Comp technique so comparison should be much better. Direct anim should have much less RAM usage.

@Ivan

Regarding defHandler I have no plans. I’ll tell you soon, but not before 2nd january.

Yes this is true. We don’t really need those, when we change CompAnim functionality that it also renders shadows and the glowing effect correctly which can be easily done as you already mentioned with changing the palette. But then RAM usage will be higher in Battles then before, but as I said I’ll show the results of the correct measurements soon. And then we should also think about how 32bit graphics can be realized with no opengl efficiently and good.

beegee · January 2, 2012, 7:23pm

These results should show the correct amount of RAM used (1st value in KB) and the loading time (2nd value in ms). The third value below is the average drawing time in ms.
B for BOAR.DEF
C for CAVLR.DEF
O for OGRE.DEF

         SDL          Comp             Direct			OpenGL
B     19464/ 430	12460/ 840		7092/ 190		81920/ 930
C     20628/ 460	12400/ 850		7844/ 190
O     13176/ 390	9684/ 615		 4148/ 180
        790		  1185			  1650			      53

Summary:
Comp and direct anim have support for alpha channel. SDL doesn’t have the capability of such a feature for 8bit images. Comp and SDL anim can load non-DEF animation files like PNG/TGA animation.
SDL has the highest RAM usage, loads reasonably fast and has the fastest drawing time. But to say it again, it doesn’t support alpha channel.
Comp has a medium RAM usage, loads a little bit slowly and has a medium drawing time.
Direct has the lowest RAM usage, loads fastest and has the slowest drawing time.

I think it would be a good decision to replace Comp anim with Direct anim in battles. Comp anim has a slightly increased RAM usage and a reasonably loading time, which is OK for smart phones too. The most important point is that Comp anim has a faster drawing time, which can speed up battle performance under smart phones.

@Ivan:

When I was testing Comp anim technique to draw several animations(more than 1) in the same time which shouldn’t be a problem, my test app crashes. Could you please check this in VCMI? Perhaps it is a fault in my anim test program.

Ivan · January 3, 2012, 11:35am

Currently largest part of memory usage (50-75%) comes from adventure map. Replacing DefHandler with CAnimation should decrease it but this can be done later.

More precisely - CAnimation can load 32-bit images. But they will be always loaded as SDL Image.

It shouldn’t crash - for example towns have ~20 of Comp anim at once and no crash reports. Can I see your test program? Can be bug on CompAnim side.

beegee · January 4, 2012, 10:13am

Support for Opengl and support for 32bit in general should be done later as 32bit is most interesting for modders some day => post 1.0

We should talk some day about our own VCMI animation format. @Ivan: You implemented already such a format, it’s seems to be very nice, but we should write a doc and a spec for modding purposes. Additionally the format with the file extension .anim should be expanded to support Opengl animations where all frames are in one image file. (Positions should be specified) => post 1.0

DefHandler can be replaced with CShowableAnim then, but this can be done after the animation reorganization step. We should specify then the default behaviour for selecting the correct animation technique which is the decision of the resource manager/handler(will explain later). It’s better to have fast rendering time instead of low RAM usage if we don’t exceed any RAM range (512MB should be max) and if it’s technically possible. Some objects in the adventure map which don’t need alpha channel can be created with SDLAnim(fastest rendering), the rest with CompAnim. If this seems to complicated to do, then we just stick to CompAnim. => later

Resource Manager or better Resource Handler

The resource handler is a new class with manages loading and unloading of any resources whether bitmap, text, video,… and so on. One feature of this class perhaps the most important is that resources which are used at several places several times can be shared and don’t get loaded and destructed many times. You don’t have to worry about freeing SDL surfaces; this gets recognized and executed automatically through smart pointers. This class should be easy to use and is highly resource optimized.

That resource sharing can be supported every resource needs a unique URI. The URI for requesting a LOD entry and a file is based on URL syntax and looks like this:
Lod://[LOD NAME]/[ENTRY]
File://[rel. path to File from VCMI_DIR]

To load a resource object you need therefore a URI object. It has a private ctor, but can be created through Factory methods.
FromBitmapLOD(ENTRY)
FromFilesystem(FILE)
FromDatadir(FILE)
And so on

The URI object holds the complete URI string as a private field.

There are a few methods for loading a resource.
loadResource(URI) delivers a shared_ptr
loadBitmap(URI) delivers a shared_ptr
loadAnimation(URI) delivers a shared_ptr

Example:

 shared_ptr<IImage> image = RH.loadBitmap(URI.fromBitmapLOD('BLA');

A few shared_ptr + class combos could have a typedef declaration.

LOD files have to be added with addLODFile and resources can be checked for existence with hasResource(URI).

Memory management is realized with shared_ptr + weak_ptr + custom deleters, but you haven’t to know about this, just get the resource and be happy.

Every resource can also be loaded with shared flag set to false, you’ll get a copy of the resource and it doesn’t get used by other clients.

If a resource wasn’t found you’ll get a log + console message and the program terminates automatically. As exception handling can be a performance bottleneck and as you can always check before if the resource is there if it’s a optional resource, program termination should be the default behaviour here.

IImage is the base class for our images. This is needed some day when we want to support both Opengl texture images, SDL_Surfaces and Comp Images. IImage holds declarations for some common methods which every image class should have like draw, width and height. But also some more specialized things like setPalette which is needed for CompImage and SDL Image. Yes SDL_Surface should be wrapped too, it’s not that bad thing as you can have then const SDL_Surfaces where pixels can’t be changed for example.

Bitmap creation looks like this:

Call to loadBitmap
Resource handler decides which image should be created on settings and on default behaviours.
Saves resource and returns a smart pointer

Animation creation is like the same as above, but normally you don’t load raw IAnimation objects. This is wrapped by CShowableAnim. LoadAnimation could therefore be a friend function.

To get the most out of the resource sharing, IAnimation objects have support for lazy evaluation like string objects. Often it’s needed to have player colored frames or glued frames in battles this only changes the palette not the pixels data. CShowableAnim then creates firstly a copy of IAnimation to get unbound from the sharing functionality and secondly changes the palette through IAnimation which therefore calls setPalette on the specified frame IImages. IImage then creates only a copy of the palette and resets the this pointer. The pointer to the raw pixels data remains unchanged.

Ibstream which stands for input byte stream could also be renamed to memstream, as we can add support for writing at a later time more easily.

Plan

ResourceHandler will get its own compilation unit to strictly distinguish it from other handlers.
LodHandler and so on will stay so long as they are used. No hard switch here.
@Tow. If we will need a extra branch, I’ll inform you before it gets too complicated. But I’ll try not to break anything.
Changes will affect animations and battle interface at first.

Ivan · January 4, 2012, 2:22pm

Ok. But I’d rather stick with .json extension for anything with JSON data in it. Sprite maps for OpenGL can be added into existing format without problems.

Completely agree.

What about video and sounds? Both have custom archive format different from Lod. Two more “protocols”? I think that virtual file system with mount points will work better here. Not going to insist on it though.
What about overriding files from lod? This is how LodHandler works now - no need to modify lod’s to replace something in it. And make sure to keep current handling of extensions - request for image “img.pcx” is identical to “img.png”.
In future we may need multiple “levels” of overriding. For example:

Check file in lod
check file in “global” VCMI dir (/usr/share/vcmi on Linux, C:\Program Files\vcmi on Win)
check file in “local” VCMI dir (~/.vcmi on Linux, C:\Users… on Win)
Not needed at the moment of course but would be nice to allow something like this later without one more rewrite.

Creating empty image\animation\etc instead? It’s OK to crash if essential txt file is missing but I don’t think that missing image should result in crash. IIRC this is how H3 works.

You can replace LodHandler with small proxy to ResourceHandler. Your choice.

Tow_dragon · January 4, 2012, 2:44pm

Resource manager is a good idea but it’s not quite trivial. In several places in the code loaded surfaces are modified and you need a unique (not shared) copy of resource there.

Tow · January 5, 2012, 10:32am

That sounds rather as an argument for replacing Direct anim with Comp anim. Shouldn’t the first sentence be the other way round?

I believe that direct anim is basically slower because it has more logic (all these borders stuff). If you want to replace Direct anim with a Comp anim, that logic will have to be added. And then the difference in blitting speed will likely disappear. I don’t think there is any other reason for any method to be significantly faster or slower than the other one, both blitting procedures are implemented in our code and are based on RLE.

Nevertheless it can be good idea to merge both classes and make it available from some common interface. (while keeping the both methods implementation)

For PCs. On mobile device we have less memory. But we also have slower CPUs. I’m not even sure what is bigger problem for the VCMI ports now: memory or CPU.

As Ivan said, it won’t work with current overriding system (which I’d like to keep). Besides, it introduces additional, reduntant logic at all load* calls: why should some GUI object (or whatever needs the bitmap) care in which LOD (or other place) the required resource is stored? Usage should be possibly simple.

I suggest going in the opposite direction. One, global resource manager should be introduced and it should know about all the resources (LODs, Data/, Sprites/ and so on). That way the calls to various lodhandlers can be replaced with calls to the Resource Handler, simplifying them (no need to remember which lodhandler contains the resource) and making it easier to extend with new resource archives of any format.

At the moment it’s enough to know the base file name (without extension) and the type of file (bitmap/text/…) to uniquely identify a resource file. And it seems to be a better, more flexible solutions than URIs you propose. An introduction of optional “package” parameter can also be useful to be able to access the overridden resource.

The exception handling cost should be negligible. Especially when they’re rarely thrown. I guess the smart pointers have bigger overhead.
But instead of exception, you can just return an empty pointer (AFAIR it’s how it works now). Then it’s possible to use convenient constructs like

if(TImagePtr img = RH.loadBitmap("foo"))
{}

beegee · January 5, 2012, 2:11pm

I forgot about that point. Yeah, this is very important and should definitely be retained. I’ve thought it would be better to declare exactly from which location the resource should be accessed but this has two disadvantages. First you have to know where the resource is located and it makes modding much more complicated. So the idea about automatic resource detection without any config files is really good. If you want to get/load a resource you have to know the name, the resource type gets automically injected by using factory methods like loadBitmap so it have to be a pcx,tga,png and so on and or loadVideo so it have to be mpg, avi, smk,bnk,… This should be really simple.

Implementation will be slightly different from current impl. At startup a table/fast boost hashmap will be created which stores the relationship between resource name and the exact overriden resource location to save lookup time whether the resource is located in lod or filesystem for example. To summarize it the key is the res name and the value is a pair of the replaced res and the new res. When loading a res you can force to load the origin/former res from LOD for example. Standard dir of new graphics is Data and standard dir of new defs/anims is Sprites. But I think this design should be kept just for the time being. After 1.0 release I think it would be better to allow that several mods can be installed on a vcmi install the same time and therefore mods should be placed in a folder called /Mods. There could be a mod for a new GUI which could be located in /Mods/NewUI/(resource are placed here) whether in this dir or any subdir, it’s the decision of the modder, the lookup process crawls every res. Then there could be a second mod for a new animation of the Ogre creature which could be found in /Mods/OgreMod/… That process of quering all resource and their location needs time that why it’s good to have at startup a resource registration process. You could perform a check if no resource is overriden twice times and so on, but this can be discussed later, not important now.

Good idea. Yeah, I’ll find some way for sure how to make this transistion as smooth as possible.

This is possible, you can load a resource without any sharing.

I would say it’s the other way round, yes.

Ok, I’ll try my best.

CPU is always a problem, RAM should only be a problem if memory size is too less. If this happens then execution will take much more time as data have to be copied from HDD and back. More memory usage results also in longer construction and destruction of objects, but this only will longer loading times for starting battle, opening town window and so on and this more likely to be forgiven by players than slow rendering time in battles or adventure map for example. Look at this wikipedia article about android devices:
en.wikipedia.org/wiki/Comparison_of_Android_devices
You’ll see that most modern devices have at least 512 MB RAM and I really don’t think that we exceed that border if we switch from direct anim to comp anim.

I absolutely agree.

According to the book more effective c++ every try catch block can reduce performance by 10% which can be a high amount if you enclose complete program execution with such a clause. BUT as always this is just based on guessing, we should have to profile this. This is a task which we can do when VCMI is completely finsihed and we’re only on improving performance and fixing bugs.

That’s fine. Should I then show just a message to the logger that the res wasn’t found?

Ivan · January 5, 2012, 8:50pm

I vote for CPU as bigger problem
I remember checking where VCMI uses most memory. Around 50 Mb comes from loaded heroes + boats + flags for adventure map. Large empty areas on them along with rotating every one of them is quite costly in both CPU (10-20% of loading time) and memory. Replacing DefHandler here with CompImage should decrease memory usage to 5-10 Mb. Even more if we won’t load flags for unused players or animation of unused heroes.
Another source for memory usage is all that components in Graphics - loaded on game start but mostly unused during game.
Fixing these should make memory usage more reasonable. If we’re lucky we can decrease RAM usage twice without some huge optimization effords.
And high CPU usage on ALL portable devices results in faster battery discharge so it’s better to keep it as low as possible.

We have several common image transformations (player coloring, flipping for creatures in battles or objects on adventure map). Creating separate copy each time can be costly memory-wise. Imagine 7 stacks of neutral creatures and rotate each one of them. With disabled sharing this will create 7 identical copies of the same resource. Some way to share this common transformations would be useful.

Yup. Send message to logger along with resource name.

My opinion on how Resource Handler should look like. Should be similar to what Tow proposed.

class ResourceHandler
{
private:
	//Global map of resources located in ResourceHandler, filled by resource loaders during initialization
	//Usage of vector allows multiple overrides of same resource (don't see any reason to use std::pair<> here)
	boost::hashmap<Identifier, std::vector<Locator> > resources;

	//All other methods\fields to load resources or to add overrides
	...
}

struct Identifier
{
	// Resource name (for example "DATA/MAINMENU")
	// No extension so .pcx and .png can override each other, always in upper case
	std::string name;
	// resource type, (FILE_IMAGE), required to prevent conflicts if files with different type (text and image)
	// have same basename (we had this problem with garrison.txt and garrison.pcx in h3bitmap.lod)
	EType type; 
};

struct Locator
{
	// interface that does actual resource loading (LodHandler and some FileHandler)
	// in such way we can easily add support for multiple archive formats (and we already have filesystem, lod, vid, snd)
	// can be replaced by pointer to function-loader
	IResourceLoader *loader;

	// name of resource ("MainMenu.pcx" for lod, /full/path/to/file/mainmenu.png for filesystem)
	// can be replaced with numerical ID of resource to avoid slow string comparison
	std::string resourceName;
}

//Resource initialization example:
resh.addHandler(new LodHanlder("H3Bitmap.lod", "DATA/")) //will add all resources from lod to global map, with DATA/ prefix
resh.addHandler(new FileHandler(DATA_DIR + "Data/", "DATA/")) //add all files from Data directory into global map, will override any identical files from H3Bitmap.lod

//Load other directories (sprites, music, sounds, video, configs, user dir), each directory have its own prefix

What we’ll get with such structure compared to current one:

Possibility to load any of overrided versions (not requred now, will be useful later)
New overrides can be added easily (mods, logical merging of H3Bitmap.lod and H3ab_bmp.lod needed for some H3 releases)
One handler instead of current multiple lod handlers and direct access to filesystem
Identical loading from lod, filesystem, whatever (for example there was proposition to use .zip for mods)

beegee · January 8, 2012, 12:49pm

@Ivan:
This idea looks very good, I’ll implement it in the way you described it.

Resource handling when it comes to image transformations

No problem. Player coloring… and everything which applies to altering the image palette can be realized in the following manner (lazy evaluation):
When you call setPlayerColor on IImage it always returns a shared_ptr to the newly created IImage object as a player colored image in this case. I can’t just internally copy its data members as this modification would ruin the sharing process. So on modify functions you’ll get a new shared_ptr. What it makes is simple: 1. Copy raw data of the IImage object 2. Pack it into a shared_ptr 3. Copy palette 4. Modify palette 5. Return newly created shared_ptr 6. Caller has to reassign its data member
Roughly the same happens when you want to change the player color of an IAnimation/CShowableAnim object for example.
Rotating image isn’t much harder. All you have to do is instead of copying image palette copying the pixels data.

To share modified images it’s needed to manually add the new resource to the resource handler. Additionally I think it would be good to have derived Identifiers called ImageIdentifier for example. This struct has an additional member called IsRotated = false(default value).
getRH().setResource(shared_ptr<…> data, ImageIdentifier) // hashmap for images, animations, binary data,…
The other side can get such resources with a appropriate Identifier.

Load methods of the resource handler
At first there will be three load methods:
loadResource => delivers a shared_ptr of MemoryStream(formerly known as ibstream, explanation about it in the first post, you save position & size parameter passing when using that class) (if you want to have a string object instead you can get a string object via a method(this is then unshared, can be easily modified), memoryStream can’t be used for writing operations in the first instance)
loadImage => delivers a shared_ptr to IImage object
loadAnimation => delivers a shared_ptr to IAnimation object (to display animations: CShowableAnim takes an IAnimation object as a ctor parameter)

Resource handler construction
A little big problem occurred at the very beginning of writing the resource handler. Former LodHandler gets also used in lib + perhaps server + client. LodHandler in lib is used to load text/config files. This means I have to define a subset of our ResourceHandler in the lib project, because I don’t want to construct IAnimation/IImage objects as they’re using SDL image and so on(SDL_extensions) in that project. That’s why we need a BaseResourceHandler in Lib which holds the loadResource method and perhaps a loadResourceAsText method for convenient usage. In Client we then have a derived class of the resource handler called CVCMIResourceHandler. This name looks a bit cryptic when there is the C in front of it, perhaps a better name would be CExtResourceHandler in the style of SDL_extensions.
This means in the lib project you need access to the BaseResourceHandler and in Client access to the ExtResourceHandler. There will be a method called getRH()/RH()/getResh() or something like that to get access to the correct resource handler.

File: CExtResourceHandler.h
#define OVERRIDE_BASE_RH

File: CBaseResourceHandler.h
class DLL_LINKAGE CBaseResourceHandler
{
protected:
	static CBaseResourceHandler * rh; // no public access, force to only use getRH(). you have to init RH with the init method once

public:
	int data;

    virtual ~CBaseResourceHandler() { }; // to make class virtual
	void init(CBaseResourceHandler * rh);

#ifndef OVERRIDE_BASE_RH
	friend DLL_LINKAGE CBaseResourceHandler * getRH();
#endif
};

#ifndef OVERRIDE_BASE_RH
DLL_LINKAGE CBaseResourceHandler * getRH();
#endif


File: CExtResourceHandler.h
class CExtResourceHandler: public CBaseResourceHandler
{
public:
	friend CExtResourceHandler * getRH();
};

CExtResourceHandler * getRH(); // gets RH and does a dynamic cast


File: ...
getRH() = new CExtResourceHandler; // construct the class
getRH()->loadImage(...); // access via getRH()

Is this implementation OK(it works with VS, not tested with GCC => different class definitions may cause error, but normally it should work as it’s only a friend declaration difference)? Are there any other (better) solutions?

Ivan · January 8, 2012, 4:28pm

I’ve been thinking about such use case:

ImagePtr img = loadImage("Whatever"); //loaded random image
ImagePtr img1 = img->rotate(); //create new shared_ptr with rotated image
ImagePtr img2 = img->rotate(); //apply same transformation on original image again, data in img1 and img2 is identical

if (img1 == img2)
    return "Transformed images are shared"
else
    return "Transformed images are not shared"

Making images with same transformations shared will be more effective but more complicated. Can be implemented later if we’ll need more speed\less memory.

Would be useful to have this as well:
loadUnpackedResource() => delivers a shared_ptr of MemoryStream (campaigns and maps are stored in compressed state and must be decompressed before parsing, check CLodHandler::getUnpackedFile)

I don’t like global getRH() - it may be difficult to find unless you know exact name. And we already have classes for global handlers.

1)rename base to something like CFileSystemHandler, remove Ext prefix in client part
2)add FileSystem to LibClasses\CGameInfo and ResourceHandler to CClientState
3)In CResourceHandler - remove inheritance, instead access FileSystem via CGI->filesystemh or pass FileSystem in constructor\initializer

Tow_dragon · January 8, 2012, 6:31pm

We could even have one shared resource + individual transformation and abstract blitting methods. Some parts of terrainRect blitting are already done like this.

Something like

class ImageHandle : public Identifier
{
  SDL_Surface * sharedSurf;
  SDL_Palette * substPalette;
  enum {ID, ROT90, ROT180, ...} transformation;
public:
  blit(...);
};

beegee · January 9, 2012, 7:56am

I’ll stick it into my todo list. You could have a data member in Identifier called userdata/tag/whatever which is of type String. For Image types there exist an additional struct which is needed to load an image and which gets serizialized auto. into a string and then used as part of the Identifier. Don’t know how you can do this in cpp, but there is a solution for it certainly.

OK, perhaps your proposal seems to be easier to understand and to access. But then you have to use CGI->filesystemh to load text/binary data and CCS->resh to load images,… whatever and not only getRH()->. All in all I think it’s equally good. I’ll stick with your solution:)

Finally, I think it’s now time to start programming as nearly everything was dicussed.

beegee · January 10, 2012, 3:45pm

So, I’m now finished with the first fourth of the animation rewrite: The file system handler and all its stuff.
Ivan, please have a look on it, if everything is fine.

Next steps are:

IAnimation + IImage + all related things to animations
CResourceHandler
Integration with VCMI(will be a very hard job).

I think this should be the best way, that way I’m relative free to develop things and there doesn’t occur much compile or runtime problems/errors. That way I can integrate those new classes into current VCMI source code at one go. Instead of integrating everything after every step, this won’t work properly.

Is it okay to log warnings about faults concerning file reading,… to tlog2?

Ivan · January 10, 2012, 6:52pm

Looks good. Some notes:

Header:
EResourceType - remove FILE_ prefix - no need to have both prefix and namespace

ResourceLocator & operator= - looks to be identical to autogenerated. Remove?

ResourceLocator::hash_value - Not needed? In unordered map only key must have hash

class IResourceLoader
boost::mutex * mutex;
created in constructor, deleted in destructor. Remove pointer?

CMemoryStreamPtr getUnpackedFile(const std::string & path) const;
CMemoryStreamPtr getUnpackedData(CMemoryStreamPtr memStream) const;

Replace with one function which accept ResourceIdenifier?
So it will be possible to write something like this:

getUnpackedResource(Identifier("MAPS/ARROGANCE", EResType::Map))
getUnpackedResource(Identifier("DATA/NEUTRAL1", EResType::Campaign))

Code:

if(seekPos == length)
	return NULL;

gcc disapproves - should be 0 instead NULL
reading past end of file is obviously error. Replace “if” with assertion or at least add console message (no crash and will give us location for breakpoint to find the source of problem)

IResourceLoader::addResourceToMap
Correct me if I’m wrong but this can be replaced with one line:

resources[identifier].push_back(locator);

If missing, operator ] will insert entry with default-constructed value (empty list in this case)

Got compile error here:
std::ifstream fileInput(filePath.c_str(), std::ios::in | std::ios::binary | std::ios::beg);
Remove std::ios::beg to fix (stackoverflow)

Several use-cases to check:

Music\Video players may require access to file by chunks or by name (needs checking)
Maps\Custom campaigns menus - require access to all files in directory. You’ll need to add something like this:
std::list getFilteredFiles(std::string prefix, EResourceType type);

My personal opinion, ignore it if you wish:
Not a fan of placing code in .h. Especially weird is MemoryStream - one constructor located in .cpp, several others including destructor - in .h