This feels a bit messy. The scale of an animation must now be publicly knowable 
on the outside for doing math on it, though it feels like it should be a purely 
internal information. Can this not be internalized somehow by changing the 
animations interface? 

Also, if you want bigger images to look good at smaller zoom, we probably 
should enable and create mipmaps while loading them. 

Diff comments:

> === modified file 
> 'data/tribes/buildings/productionsites/barbarians/shipyard/init.lua'
> --- data/tribes/buildings/productionsites/barbarians/shipyard/init.lua        
> 2016-09-03 14:59:10 +0000
> +++ data/tribes/buildings/productionsites/barbarians/shipyard/init.lua        
> 2016-11-13 16:57:34 +0000
> @@ -7,6 +7,7 @@
>     descname = pgettext("barbarians_building", "Shipyard"),
>     helptext_script = dirname .. "helptexts.lua",
>     icon = dirname .. "menu.png",
> +   representative_image = dirname .. "representative_image.png",

So we are back shipping an individual image for the representative image? I 
understand that is for 'pics' in rich text, i.e. in messages and so on?

>     size = "medium",
>     needs_seafaring = true,
>  
> 
> === modified file 'src/graphic/animation.cc'
> --- src/graphic/animation.cc  2016-10-26 10:56:02 +0000
> +++ src/graphic/animation.cc  2016-11-13 16:57:34 +0000
> @@ -194,12 +208,12 @@
>  
>  uint16_t NonPackedAnimation::width() const {
>       ensure_graphics_are_loaded();
> -     return frames_[0]->width();
> +     return frames_[0]->width() / scale_;

These should now return float?

>  }
>  
>  uint16_t NonPackedAnimation::height() const {
>       ensure_graphics_are_loaded();
> -     return frames_[0]->height();
> +     return frames_[0]->height() / scale_;
>  }
>  
>  uint16_t NonPackedAnimation::nr_frames() const {
> @@ -215,20 +229,17 @@
>       return hotspot_;
>  }
>  
> -Image* NonPackedAnimation::representative_image(const RGBColor* clr) const {
> +const Image* NonPackedAnimation::representative_image(const RGBColor* clr) 
> const {

I am confused: why do you need the representative_image lua option?

>       assert(!image_files_.empty());
>       const Image* image = g_gr->images().get(image_files_[0]);
> -
> -     if (!hasplrclrs_ || clr == nullptr) {
> -             // No player color means we simply want an exact copy of the 
> original image.
> -             const int w = image->width();
> -             const int h = image->height();
> -             Texture* rv = new Texture(w, h);
> -             rv->blit(Rectf(0, 0, w, h), *image, Rectf(0, 0, w, h), 1., 
> BlendMode::Copy);
> -             return rv;
> -     } else {
> -             return playercolor_image(clr, image, 
> g_gr->images().get(pc_mask_image_files_[0]));
> +     if (hasplrclrs_ && clr) {
> +             image = playercolor_image(clr, image, 
> g_gr->images().get(pc_mask_image_files_[0]));
>       }
> +     const int w = image->width();
> +     const int h = image->height();
> +     Texture* rv = new Texture(w / scale_, h / scale_);
> +     rv->blit(Rectf(0, 0, w / scale_, h / scale_), *image, Rectf(0, 0, w, 
> h), 1., BlendMode::Copy);
> +     return rv;
>  }
>  
>  const std::string& NonPackedAnimation::representative_image_filename() const 
> {


-- 
https://code.launchpad.net/~widelands-dev/widelands/animation_scaling/+merge/310718
Your team Widelands Developers is requested to review the proposed merge of 
lp:~widelands-dev/widelands/animation_scaling into lp:widelands.

_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to     : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp

Reply via email to