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.
_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to : [email protected]
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
