← Back to team overview

widelands-dev team mailing list archive

Re: [Merge] lp:~widelands-dev/widelands/animation_scaling into lp:widelands

 

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.


References