widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

Review: Approve

Excellent! Thanks for fixing.

Diff comments:

> === modified file 'src/graphic/graphic.cc'
> --- src/graphic/graphic.cc	2014-11-24 07:10:03 +0000
> +++ src/graphic/graphic.cc	2014-11-30 10:00:23 +0000
> @@ -300,7 +300,7 @@
>   * @param sw a StreamWrite where the png is written to
>   */
>  void Graphic::save_png(const Image* image, StreamWrite * sw) const {
> -	save_surface_to_png(image->texture(), sw);
> +	save_surface_to_png(image->texture(), sw, COLOR_TYPE::RGBA);

COLOR_TYPE is a type, should be ColorType.

>  }
>  
>  uint32_t Graphic::new_maptexture(const std::vector<std::string>& texture_files, const uint32_t frametime)
> @@ -326,7 +326,7 @@
>  {
>  	log("Save screenshot to %s\n", fname.c_str());
>  	StreamWrite * sw = g_fs->open_stream_write(fname);
> -	save_surface_to_png(screen_.get(), sw);
> +	save_surface_to_png(screen_.get(), sw, COLOR_TYPE::RGB);
>  	delete sw;
>  }
>  
> 
> === modified file 'src/graphic/image_io.cc'
> --- src/graphic/image_io.cc	2014-11-28 18:41:13 +0000
> +++ src/graphic/image_io.cc	2014-11-30 10:00:23 +0000
> @@ -71,7 +71,7 @@
>  	return sdlsurf;
>  }
>  
> -bool save_surface_to_png(Surface* surface, StreamWrite* sw) {
> +bool save_surface_to_png(Surface* surface, StreamWrite* sw, COLOR_TYPE color_type) {
>  	png_structp png_ptr = png_create_write_struct(
>  	   PNG_LIBPNG_VER_STRING, static_cast<png_voidp>(nullptr), nullptr, nullptr);
>  
> @@ -104,7 +104,7 @@
>  	             surface->width(),
>  	             surface->height(),
>  	             8,
> -	             PNG_COLOR_TYPE_RGB,
> +	             (color_type == COLOR_TYPE::RGB) ? PNG_COLOR_TYPE_RGB : PNG_COLOR_TYPE_RGBA,
>  	             PNG_INTERLACE_NONE,
>  	             PNG_COMPRESSION_TYPE_DEFAULT,
>  	             PNG_FILTER_TYPE_DEFAULT);
> @@ -114,7 +114,7 @@
>  	{
>  		const uint16_t surf_w = surface->width();
>  		const uint16_t surf_h = surface->height();
> -		const uint32_t row_size = 3 * surf_w;
> +		const uint32_t row_size = (color_type == COLOR_TYPE::RGB) ? 3 * surf_w : 4 * surf_w;
>  
>  		std::unique_ptr<png_byte[]> row(new png_byte[row_size]);
>  
> @@ -127,17 +127,21 @@
>  			for (uint32_t x = 0; x < surf_w; ++x) {
>  				RGBAColor color;
>  				color.set(fmt, surface->get_pixel(x, y));
> -				row[3 * x] = color.r;
> -				row[3 * x + 1] = color.g;
> -				row[3 * x + 2] = color.b;
> +				if (color_type == COLOR_TYPE::RGB) {

I think this is fine as is. You could pull out a pixel_pitch integer that is either 3 or 4 and only have one if for the alpha component. But I think you cannot get around the if.

> +						row[3 * x] = color.r;
> +						row[3 * x + 1] = color.g;
> +						row[3 * x + 2] = color.b;
> +				} else {
> +					row[4 * x] = color.r;
> +					row[4 * x + 1] = color.g;
> +					row[4 * x + 2] = color.b;
> +					row[4 * x + 3] = color.a;
> +				}
>  			}
> -
>  			png_write_row(png_ptr, row.get());
>  		}
> -
>  		surface->unlock(Surface::Unlock_NoChange);
>  	}
> -
>  	// End write
>  	png_write_end(png_ptr, info_ptr);
>  	png_destroy_write_struct(&png_ptr, &info_ptr);
> 
> === modified file 'src/graphic/image_io.h'
> --- src/graphic/image_io.h	2014-11-24 07:10:03 +0000
> +++ src/graphic/image_io.h	2014-11-30 10:00:23 +0000
> @@ -29,6 +29,7 @@
>  class StreamWrite;
>  class Surface;
>  struct SDL_Surface;
> +enum class COLOR_TYPE {RGB, RGBA};
>  
>  class ImageNotFound : public WException {
>  public:
> @@ -50,6 +51,6 @@
>  SDL_Surface* load_image_as_sdl_surface(const std::string& fn, FileSystem* fs = nullptr);
>  
>  /// Saves the 'surface' to 'sw' as a PNG.
> -bool save_surface_to_png(Surface* surface, StreamWrite* sw);
> +bool save_surface_to_png(Surface* surface, StreamWrite* sw, COLOR_TYPE color_type);

Put the enum class directly before the function declaration, after it's comment. It is only used here, so it belongs here as part of the documentation.

>  
>  #endif  // end of include guard: WL_GRAPHIC_IMAGE_IO_H
> 
> === modified file 'src/map_io/map_saver.cc'
> --- src/map_io/map_saver.cc	2014-11-24 07:10:03 +0000
> +++ src/map_io/map_saver.cc	2014-11-30 10:00:23 +0000
> @@ -220,7 +220,7 @@
>  		std::unique_ptr<Texture> minimap(
>  		   draw_minimap(m_egbase, nullptr, Point(0, 0), MiniMapLayer::Terrain));
>  		FileWrite fw;
> -		save_surface_to_png(minimap.get(), &fw);
> +		save_surface_to_png(minimap.get(), &fw, COLOR_TYPE::RGBA);
>  		fw.write(m_fs, "minimap.png");
>  	}
>  }
> 
> === modified file 'src/wui/interactive_base.cc'
> --- src/wui/interactive_base.cc	2014-11-27 11:15:34 +0000
> +++ src/wui/interactive_base.cc	2014-11-30 10:00:23 +0000
> @@ -446,7 +446,8 @@
>  			((fps_format %
>  			  (1000.0 / m_frametime) % (1000.0 / (m_avg_usframetime / 1000)))
>  			 .str(), UI_FONT_SIZE_SMALL);
> -		dst.blit(Point(5, (is_game) ? 25 : 5), UI::g_fh1->render(fps_text), BlendMode::UseAlpha, UI::Align_Left);
> +		dst.blit(Point(5, (is_game) ? 25 : 5), UI::g_fh1->render(fps_text),
> +				BlendMode::UseAlpha, UI::Align_Left);
>  	}
>  }
>  
> 


-- 
https://code.launchpad.net/~widelands-dev/widelands/fix_screenshots/+merge/243225
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/fix_screenshots.