A new design for image::type

Discussion of all aspects of the game engine, including development of new and existing features.

Moderator: Forum Moderators

Post Reply
Alink
Inactive Developer
Posts: 181
Joined: March 5th, 2007, 6:45 am
Location: Belgium

A new design for image::type

Post by Alink »

When working with image::type, I was annoyed by the present system, I found it :
- rigid : adding a new effect cause a change in all the type organization and all the calls in the main code must be checked.
- wasting : The hierarchical organization cause to flush half of the cache when just a parameter change.
- confusing : some type name hide the effect applied on image (the "brightened" type do all the possible effects: scaling, hex-masking, coloring, time of the day masking and finally brightening)
- limited : a lot of image manipulation are done in display.cpp (on hex like fog, on units like the poison green effect) without real organization or optimization. And from the main site's road map :

" One example of th[is] is the display class, which is bloated with all kinds of methods which are remotely related to drawing on the screen. We need to identify this code and create a separate module for it."

So, instead I propose to use a more flexible system which use bitwise operations, like the SDL library does. A typical call will be:
get_image(HEX | SCALED | COLORED | BRIGHTENED) or get_image(UNIT | SCALED | POISONED | INVISIBLE)
If needed maybe with shortcut like TYPICAL_TILE = HEX | SCALED | COLORED
Display.cpp may just progressively adds the effect to the image::type before calling get_image.
Image.cpp will be in charge of apply these effects in a nice and optimal sequence, cache the expensive or frequent combinations and generate the others from these. I think that do a recursive call when just removing the last applied effect will be simple.

And for the transition from the present system, it can be done effect by effect, with shortcut like OLD_TYPE=EFFECT_1 | EFFECT_2

A good design ?
Boucman
Inactive Developer
Posts: 2119
Joined: March 31st, 2004, 1:04 pm

Post by Boucman »

hmm yes and no..

first you should know that some work has been done in trunk wrt the image cache and your assumptions about the old case is not exactly true...

we now have two scalings: SCALED_TO_HEX and SCALED_TO_ZOOM

moreover you seem to miss the point of the image cache...

when a transformation is done very often on an image, keep the transformed version directly in cache instead of the original image.

so the transformations should have very specific purpose, for very common cases...

the idea of flags is a good idea, but you should be very careful with cache cleaning, will you clean only images with a given cache on zoom changing, for instance, and the ones with another flag on TOD change ?

let's review the current TYPE that are currently available

* UNSCALED typically used for messages and some GUI elements
* SCALED_TO_HEX used to draw terrain
* SCALED_TO_ZOOM used to draw units
* BRIGHTENED to show selected elements
* SEMI_BRIGHTENED so show highlighted hex (reachable...)

there is also a COLOUR_ADJUSTMENT flag which is used to choose if the image should be changed according to time of day

I think brightened images shouldn't be kept in cache, but created on the fly... semi-brightened is arguable...

all other types should be kept

in your post you propose new types like POISONED and INVISIBLE this is a bad idea, both cases are too unit specific and too uncommon to require a cache. Don't forget that once an image is in cache it will probably stay there for quite some time, so you should only put images that are really usefull, and adjusting alpha or colour for invisible/poison should be done on the fly...

last, you want to difine OLD_TRANS=... I don't think that's usefull, just grep the code and replace everywhere, no backward compatibility in code is needed....

cheers
Fight key loggers: write some perl using vim
Alink
Inactive Developer
Posts: 181
Joined: March 5th, 2007, 6:45 am
Location: Belgium

Post by Alink »

I'm aware of the new types. I followed their introduction and my post is partially about that.

I just precise that the flag like POISONED will be used for organize all the image transformation in one place (image.cpp), and must not store the resulting image in cache, only find a previous image in the good cache and apply the effect on it. Also, if an effect (new or old) become more often used or more cpu-expensive, you can reorganize the cache system there.

About the cache, maybe I mistake, but currently some intermediate steps of image transformation are cached (recursive calls with the "add to cache" parameter set to default=true), so when changing just the color, we don't rescale the image, only get the scaled image from cache and color it. It also prevent a reload from the disk. I think it's a good balance between cpu and memory.

There is room for optimization here in the present system, but I think it will help to split the effects (the current types combine different effects), again without caching all these steps but only the important ones. And flags seems more handy for this: with N flags you have 2^N types.

The COLOR_ADJUSTMENT coded with a bool seems a good example for the flag system (currently, it is badly implemented, it's used only on some type and not on others). Notice that it is already a sort of flag. But why use a bool for this and a new type for brightened ? And if I want to brighten an unit or an unscaled image of the UI (highlight a button)? Create a UNSCALED_BRIGHTENED type? With a flag system, all this will come automatically. Only the optimization of cache usage will need work.

I'll be happy to help to correct these little things without flags, but since I think it can be easier with, I ask here before.
Darth Fool
Retired Developer
Posts: 2633
Joined: March 22nd, 2004, 11:22 pm
Location: An Earl's Roadstead

Re: A new design for image::type

Post by Darth Fool »

Alink wrote:When working with image::type, I was annoyed by the present system, I found it :
- rigid : adding a new effect cause a change in all the type organization and all the calls in the main code must be checked.
Not true. The image path functionality and caching done with it adds a perfect place to add new effects. It is actually quite simple to add a new image path function effect if you have the code that does the underlying image manipulation.
- wasting : The hierarchical organization cause to flush half of the cache when just a parameter change.
- confusing : some type name hide the effect applied on image (the "brightened" type do all the possible effects: scaling, hex-masking, coloring, time of the day masking and finally brightening)
true, but then again, your proposal for TYPICAL_TILE below would have the same effect.
- limited : a lot of image manipulation are done in display.cpp (on hex like fog, on units like the poison green effect) without real organization or optimization. And from the main site's road map :

" One example of th[is] is the display class, which is bloated with all kinds of methods which are remotely related to drawing on the screen. We need to identify this code and create a separate module for it."

So, instead I propose to use a more flexible system which use bitwise operations, like the SDL library does. A typical call will be:
get_image(HEX | SCALED | COLORED | BRIGHTENED) or get_image(UNIT | SCALED | POISONED | INVISIBLE)
If needed maybe with shortcut like TYPICAL_TILE = HEX | SCALED | COLORED
Display.cpp may just progressively adds the effect to the image::type before calling get_image.
Image.cpp will be in charge of apply these effects in a nice and optimal sequence, cache the expensive or frequent combinations and generate the others from these. I think that do a recursive call when just removing the last applied effect will be simple.

And for the transition from the present system, it can be done effect by effect, with shortcut like OLD_TYPE=EFFECT_1 | EFFECT_2

A good design ?
Well, I think that we are better off having only the very basic most common effects, such as scaling, as part of the different types/flags. More complicated effects (such as poisoned/invisible) I think would be better if moved into being entirely image path functions. This would make them accesable to WML, add flexibility to the engine and assuming the image path function names are not long and similar, I don't think it would significantly impact performance.
Alink
Inactive Developer
Posts: 181
Joined: March 5th, 2007, 6:45 am
Location: Belgium

Post by Alink »

Indeed maybe my TYPICAL_TILE was not a good idea.

I never thought about using the image path for the effects. It seems really great, specially if it allows use in WML. But you mean something like:
get_image("terrain/image.png+BRIGHTENED"), with parsing for knowing the effect to apply?

If so, it can be use to easily define new units and terrains without editing images (a baby monster? just use his image+SMALL in the WML).

In the continuity of this idea the file path can be view as an "effect" (loading and blitting an image) and allow to combine images (add an aura image behind a unit, or flowers on some tiles).

I like it but seems harder to code, and maybe the parsing part will be a problem.
Darth Fool
Retired Developer
Posts: 2633
Joined: March 22nd, 2004, 11:22 pm
Location: An Earl's Roadstead

Post by Darth Fool »

Alink wrote:Indeed maybe my TYPICAL_TILE was not a good idea.

I never thought about using the image path for the effects. It seems really great, specially if it allows use in WML. But you mean something like:
get_image("terrain/image.png+BRIGHTENED"), with parsing for knowing the effect to apply?

If so, it can be use to easily define new units and terrains without editing images (a baby monster? just use his image+SMALL in the WML).

In the continuity of this idea the file path can be view as an "effect" (loading and blitting an image) and allow to combine images (add an aura image behind a unit, or flowers on some tiles).

I like it but seems harder to code, and maybe the parsing part will be a problem.
Mostly yes. The syntax for get_image isn't quite right, but you seem to have the general idea. I imagine that eventually the image path functions will enable many kinds of image manipulations including all the ones that you mentioned. For now, I have put adding any new functions until I finish some other projects and the open-gl conversion that was being worked on (is it still?) is either merged into mainline or officially abandoned, since I didn't want to make the change even more difficult.

The only current image path functions are TC (deprecated) and RC which recolors a palette within an image. This is how team coloring and other recolorings is currently done. See http://www.wesnoth.org/forum/viewtopic. ... 535#194535 and
http://www.wesnoth.org/forum/viewtopic. ... 201#216201
for an example of how an image path function can be added as a modification to all images of a unit. Alternatively an individual image can have an IPF added to it. You can look in images.cpp to see how the RC function is made available. ooh... someone has added a fliplayer function :) . In any case, I imagine adding a couple of other functions including "C" to recolor all of an image, "O"verlay, "B"ackground. "A"lpha, and others, but they will have to wait.
User avatar
Sapient
Inactive Developer
Posts: 4453
Joined: November 26th, 2005, 7:41 am
Contact:

Post by Sapient »

TC is not deprecated as far as I am concerned. Since RC doesn't account for changes in Side settings, it can't handle those cases correctly.
http://www.wesnoth.org/wiki/User:Sapient... "Looks like your skills saved us again. Uh, well at least, they saved Soarin's apple pie."
Alink
Inactive Developer
Posts: 181
Joined: March 5th, 2007, 6:45 am
Location: Belgium

Post by Alink »

Darth fool >Sorry, didn't notice / forget this part (quick looked as the parsing of the file path). Thanks for your info, indeed this method will be more powerful at the end.
Post Reply