Improved support for reading and writing image files

[ceeac]

This patch improves support for reading and writing image files. The patch comprises the following additions and changes:

• libpng is now required for building Simutrans (not only for makeobj)
• Reading heightmaps from PNGs is now possible
• Makeobj now supports reading grayscale PNGs, not only colored ones
• Makeobj now supports reading BMP and PPM files (if anybody wants that...)
• Screenshots are now saved as PNGs on all platforms. Platform specific screenshot code has been removed.

[prissi]

I will probably have a look tomorrow. (I am not sure how one would define transparent colors with 8 bit grayscale makeobj images, but I think that is just a side product of the new loader.)

[prissi]

Thank you for your work.

Two principal things

I would prefer to have all these file in IO and not add another folder, please. The extra folders just make the include  path longer.(I think I would like to move the files from io/rdwr also to io at some point.) Well, with a patch file, this is just text replacing, so quickly done.

And please, do not indent #if etc. unless they are nested These are at left to make immediately clear these are not part of the code.

Now to the actual code:

On the concept:
I would suggest using two different classify routines, classify_img and classify_file. Images should never be handled by the normal routine and vice versa. That will avoid errors by design. Also rescanning a full savegame folder (can happen occasionally, if one uses different simutrans versions) takes even more time like this.

The comment in line 241 suggests that the last png file is cached. However, your code assumes the last file is cached, whatever format. Maybe overlooked the comment?

Wikipedia gives P3 as magic for PPM files. I think there are several versions around. Maybe go by extension and starting with P instead to classify? (It seems extension is ignored, which is ok for bmp file, which coudl end in rle and dib, but for PNG/png and ppm there should not be other files like this.)

In makeobj, image_writer_t will quite with a fatal error upon wrong size, which make debugging of broken images a little more difficult. Maybe an error message and fail to open would cause the upper level error printing, which indicates which obj to blame.

Assigning a raw_image_t to an existing raw_image_t could cause the memory to be lost. I would rather get an explicit copy command, or just forbid assignments and one has to use pointers. Also copying the memory seems wasteful, and this will happens likely by accident easily with C++. (But maybe I got this wrong, so please correct me, should I be wrong.)

[ceeac]

(I am not sure how one would define transparent colors with 8 bit grayscale makeobj images, but I think that is just a side product of the new loader.)

PNG supports 16 bpp gray + alpha images (8 bits gray + 8 bits alpha) but I have not implemented support for transparent grayscale PNGs, only ones without an alpha channel.

I would prefer to have all these file in IO and not add another folder, please. The extra folders just make the include  path longer.

i did it this way so that the files don't have to be moved again when the folder gets bigger. I have not changed it back just yet, but I can change it back before committing the patch.

And please, do not indent #if etc.

I would suggest using two different classify routines, classify_img and classify_file.

Maybe an error message and fail to open would cause the upper level error printing

Changed.

However, your code assumes the last file is cached, whatever format.

This is intentional - image_writer_t::block_load accepts any file name and any supported format, it is just that image_writer_t::write_obj assumes the image has a png extension.

Maybe go by extension and starting with P instead to classify?

The ppm reader only supports P6 type files where data is stored raw - P3 type ppm files are stored as ASCII numbers. So IMO it does not make sense to change it for now.

Assigning a raw_image_t to an existing raw_image_t could cause the memory to be lost.

No - the new image is passed by value which causes it to be copied, the temporary copy gets swapped with the original image, which causes the original data to be deleted when the temporary copy gets deleted on scope exit. Copy-and-swap.

I would rather get an explicit copy command, or just forbid assignments

Hmm, this would be consistent with the classes in tpl/ ... Changed.

[Ters]

Since typical assignment uses the copy operator, I would expect a copy, not a swap. Swapping is however acceptable for the move operator.

[prissi]

Here is the patch without the img folder and some changes to actually compile the MSVC project. (And the svn:eol-style attribute)

[Yona-TYT]

Can this patch be used to implement image tags in scripts?. https://forum.simutrans.com/index.php/topic,15049.0.html

[prissi]

No, because this requires other code. It could make it easier.

[ceeac]

Here is the patch without the img folder and some changes to actually compile the MSVC project. (And the svn:eol-style attribute)

Thanks! I did some more testing and submitted your version and a small fix for compiling on Linux in r9620.

[Andarix]

this doesn't compile for me under git action

r9629

Linux makeobj

2021-02-12T09:45:33.7218418Z ===> HOSTCXX io/raw_image_bmp.cc
2021-02-12T09:45:34.0143184Z ===> HOSTCXX io/raw_image_png.cc
2021-02-12T09:45:34.1756537Z io/raw_image_png.cc: In member function 'bool raw_image_t::write_png(const char*) const':
2021-02-12T09:45:34.1758407Z io/raw_image_png.cc:298:40: error: invalid conversion from 'const uint8* {aka const unsigned char*}' to 'png_bytep {aka unsigned char*}' [-fpermissive]
2021-02-12T09:45:34.1759429Z      png_write_row(png_ptr, access_pixel(0, y));
2021-02-12T09:45:34.1760010Z                                         ^
2021-02-12T09:45:34.1760674Z In file included from /usr/include/png.h:321:0,
2021-02-12T09:45:34.1761317Z                  from io/raw_image_png.cc:9:
2021-02-12T09:45:34.1762495Z /usr/include/png.h:1646:8: note:   initializing argument 2 of 'void png_write_row(png_structp, png_bytep)'
2021-02-12T09:45:34.1763733Z  extern PNG_EXPORT(void,png_write_row) PNGARG((png_structp png_ptr,
2021-02-12T09:45:34.1765437Z         ^
2021-02-12T09:45:34.1922511Z common.mk:50: recipe for target 'build/default/io/raw_image_png.o' failed
2021-02-12T09:45:34.1925128Z make: *** [build/default/io/raw_image_png.o] Error 1
2021-02-12T09:45:34.1933231Z ##[error]Process completed with exit code 2.
2021-02-12T09:45:34.1976007Z Cleaning up orphan processes

Linux simutrans SDL2

2021-02-12T09:45:49.3913145Z ===> HOSTCXX io/raw_image.cc
2021-02-12T09:45:49.5928168Z ===> HOSTCXX io/raw_image_bmp.cc
2021-02-12T09:45:49.9153800Z ===> HOSTCXX io/raw_image_png.cc
2021-02-12T09:45:50.2225078Z io/raw_image_png.cc: In member function 'bool raw_image_t::write_png(const char*) const':
2021-02-12T09:45:50.2226814Z io/raw_image_png.cc:298:40: error: invalid conversion from 'const uint8* {aka const unsigned char*}' to 'png_bytep {aka unsigned char*}' [-fpermissive]
2021-02-12T09:45:50.2227856Z      png_write_row(png_ptr, access_pixel(0, y));
2021-02-12T09:45:50.2228400Z                                         ^
2021-02-12T09:45:50.2229032Z In file included from /usr/include/png.h:321:0,
2021-02-12T09:45:50.2229656Z                  from io/raw_image_png.cc:9:
2021-02-12T09:45:50.2230767Z /usr/include/png.h:1646:8: note:   initializing argument 2 of 'void png_write_row(png_structp, png_bytep)'
2021-02-12T09:45:50.2232224Z  extern PNG_EXPORT(void,png_write_row) PNGARG((png_structp png_ptr,
2021-02-12T09:45:50.2232833Z         ^
2021-02-12T09:45:50.2394670Z common.mk:50: recipe for target 'build/default/io/raw_image_png.o' failed
2021-02-12T09:45:50.2397584Z make: *** [build/default/io/raw_image_png.o] Error 1
2021-02-12T09:45:50.2408386Z ##[error]Process completed with exit code 2.
2021-02-12T09:45:50.2460164Z Cleaning up orphan processes

Linux simutrans posix

2021-02-12T09:45:33.5163784Z ===> HOSTCXX io/raw_image.cc
2021-02-12T09:45:33.7218418Z ===> HOSTCXX io/raw_image_bmp.cc
2021-02-12T09:45:34.0143184Z ===> HOSTCXX io/raw_image_png.cc
2021-02-12T09:45:34.1756537Z io/raw_image_png.cc: In member function 'bool raw_image_t::write_png(const char*) const':
2021-02-12T09:45:34.1758407Z io/raw_image_png.cc:298:40: error: invalid conversion from 'const uint8* {aka const unsigned char*}' to 'png_bytep {aka unsigned char*}' [-fpermissive]
2021-02-12T09:45:34.1759429Z      png_write_row(png_ptr, access_pixel(0, y));
2021-02-12T09:45:34.1760010Z                                         ^
2021-02-12T09:45:34.1760674Z In file included from /usr/include/png.h:321:0,
2021-02-12T09:45:34.1761317Z                  from io/raw_image_png.cc:9:
2021-02-12T09:45:34.1762495Z /usr/include/png.h:1646:8: note:   initializing argument 2 of 'void png_write_row(png_structp, png_bytep)'
2021-02-12T09:45:34.1763733Z  extern PNG_EXPORT(void,png_write_row) PNGARG((png_structp png_ptr,
2021-02-12T09:45:34.1765437Z         ^
2021-02-12T09:45:34.1922511Z common.mk:50: recipe for target 'build/default/io/raw_image_png.o' failed
2021-02-12T09:45:34.1925128Z make: *** [build/default/io/raw_image_png.o] Error 1
2021-02-12T09:45:34.1933231Z ##[error]Process completed with exit code 2.
2021-02-12T09:45:34.1976007Z Cleaning up orphan processes

[Dwachs]

Seems it does not like the second parameter to be const. It compiles for me, maybe different version of png.

Replacing

Code Select Expand


png_write_row(png_ptr, access_pixel(0, y));

by

Code Select Expand


uint8 pxl = access_pixel(0, y);
png_write_row(png_ptr, pixel);

should do the trick

[ceeac]

maybe different version of png.

Yes, png_write_row takes a png_bytep instead of a png_const_bytep in older versions. The compile error should be fixed in r9630.

[Andarix]

simutrans compile

makeobj not compile

Linux

2021-02-12T12:37:39.3593335Z ===> CXX ../io/raw_image.cc
2021-02-12T12:37:39.4706871Z ../io/raw_image.cc: In destructor 'raw_image_t::~raw_image_t()':
2021-02-12T12:37:39.4709486Z ../io/raw_image.cc:53:12: error: 'free' was not declared in this scope
2021-02-12T12:37:39.4710998Z    free(data);
2021-02-12T12:37:39.4711947Z             ^
2021-02-12T12:37:39.4748123Z ../uncommon.mk:53: recipe for target '../build/default/io/raw_image-makeobj.o' failed
2021-02-12T12:37:39.4752836Z make: *** [../build/default/io/raw_image-makeobj.o] Error 1
2021-02-12T12:37:39.4759653Z ##[error]Process completed with exit code 2.
2021-02-12T12:37:39.4819044Z Cleaning up orphan processes

[prissi]

inclusing <stdlib.h> in the header should fix this. see r9632

[Andarix]

r9632

2021-02-13T23:48:01.4307872Z ===> CXX ../io/classify_file.cc
2021-02-13T23:48:01.5931953Z ===> CXX ../io/raw_image_bmp.cc
2021-02-13T23:48:01.7073524Z ../io/raw_image_bmp.cc: In member function 'bool raw_image_t::read_bmp(const char*)':
2021-02-13T23:48:01.7076070Z ../io/raw_image_bmp.cc:129:26: error: 'abs' was not declared in this scope
2021-02-13T23:48:01.7077388Z   this->width  = abs(width);
2021-02-13T23:48:01.7078032Z                           ^
2021-02-13T23:48:01.7163425Z ../uncommon.mk:53: recipe for target '../build/default/io/raw_image_bmp-makeobj.o' failed
2021-02-13T23:48:01.7164960Z make: *** [../build/default/io/raw_image_bmp-makeobj.o] Error 1
2021-02-13T23:48:01.7172870Z ##[error]Process completed with exit code 2.
2021-02-13T23:48:01.7218671Z Cleaning up orphan processes

MSVC

1>raw_image.cc
1>raw_image_bmp.cc
1>raw_image_png.cc
1>raw_image_ppm.cc
1>log.cc
1>searchfolder.cc
1>Kompilieren...
1>simstring.cc
1>raw_image.obj : error LNK2001: Nicht aufgelöstes externes Symbol ""public: __thiscall file_info_t::file_info_t(void)" (??0file_info_t@@QAE@XZ)".
1>raw_image.obj : error LNK2001: Nicht aufgelöstes externes Symbol ""enum file_classify_status_t __cdecl classify_image_file(char const *,struct file_info_t *)" (?classify_image_file@@YA?AW4file_classify_status_t@@PBDPAUfile_info_t@@@Z)".
1>raw_image_ppm.obj : error LNK2001: Nicht aufgelöstes externes Symbol ""char * __cdecl read_line(char *,int,struct _iobuf *)" (?read_line@@YAPADPADHPAU_iobuf@@@Z)".
1>D:\simutrans_entwicklung\svn\simutrans_nightly\makeobj\..\build\makeobj\Release\Makeobj.exe : fatal error LNK1120: 3 nicht aufgelöste Externe
1>Die Erstellung des Projekts "Makeobj.vcxproj" ist abgeschlossen -- FEHLER.

[prissi]

r9635 should fix this, thank you for testing

[Andarix]

r9635 Linux

2021-02-14T13:31:25.5640926Z ===> CXX ../io/classify_file.cc
2021-02-14T13:31:25.7540416Z ===> CXX ../io/raw_image_bmp.cc
2021-02-14T13:31:26.0312953Z ===> CXX ../io/raw_image_png.cc
2021-02-14T13:31:26.2939746Z ===> CXX ../io/raw_image_ppm.cc
2021-02-14T13:31:26.4167915Z ../io/raw_image_ppm.cc: In member function 'bool raw_image_t::read_ppm(const char*)':
2021-02-14T13:31:26.4169609Z ../io/raw_image_ppm.cc:47:26: error: 'atoi' was not declared in this scope
2021-02-14T13:31:26.4170315Z    param[index++] = atoi(c);
2021-02-14T13:31:26.4170711Z                           ^
2021-02-14T13:31:26.4242271Z make: *** [../build/default/io/raw_image_ppm-makeobj.o] Error 1
2021-02-14T13:31:26.4244310Z ../uncommon.mk:53: recipe for target '../build/default/io/raw_image_ppm-makeobj.o' failed
2021-02-14T13:31:26.4255828Z ##[error]Process completed with exit code 2.
2021-02-14T13:31:26.4317732Z Cleaning up orphan processes

[prissi]

Also adding stdlib fixes this. r9636

[Andarix]

linux makeobj works, thanks