Topic: Patch to allow <img src= > Tag in any location using Flowtext.

[HaydenRead]

Hi All,
I have created a patch to allow the use of the <img src= > Tag in any location that uses flowtext.
It currently only supports BMP files
BMP Compression Support: None, RLE8, BITFIELDS
BMP Bits Per Pixel Support: 1, 4, 8, 16, 24, 32 (converts to the 16 bit format used by Simutrans on loading).

It keeps a list of loaded images in a hashtable, so that if the same image is requested again, it just passes the image_id back, without reloading the image.

It searches for the image in the following places, in the listed order:

• Currently loaded Scenario Directory
• Pak File Directory
• Application Directory

This patch adds 2 new files gui_flowtext_image.cc & gui_flowtext_image.h
The gui_flowtext_image.cc file will need to be added to whatever compile method is used (Makefile, VS Project, etc).

My Primary reason for creating this patch was to allow images to be included in a Tutorial type scenario, to help explain concepts (as they say, a picture is worth a thousand words).

Latest version of patch can be found below

[captain crunch]

Will this add a second BMP reader?
Could the BMP reader in simworld.cc profit from this or vice versa?

[HaydenRead]

Will this add a second BMP reader?
Could the BMP reader in simworld.cc profit from this or vice versa?

Thanks for your comment, I have taken a look at simworld.cc
This does add a second BMP reader.
The issues with the BMP reader in simworld are:

• It is only designed to only read in as a Heightmap, there is no option to read an image in using it.
• It only supports 8bit and 24bit formats (16 bit images are best for display, as the display uses 16 bit format)
• It only supports None, or RLE8 compression formats (GIMP saves using BITFIELDS, MS Paint None, other applications I don't know what formats they normally use)

While the reader in simworld could not be used to read in an image, (and even if it could, would have reduced functionality), it would be possible to adjust this BMP reader to read the height map image in for simworld.

As an additional note, I am looking at the possibility of reading in other file formats as well, but thought I should get feed back on what I have already first.

[prissi]

Alternatively, on windows there is the png reader/writer for screenshots, and Linux uses a lot of libpng. I am not sure BMP is a good format for tutorial images.

In any case I would suggest to consolidate the BMP reader functions in simworld and this patch, so have only one.

[HaydenRead]

Alternatively, on windows there is the png reader/writer for screenshots, and Linux uses a lot of libpng. I am not sure BMP is a good format for tutorial images.

In any case I would suggest to consolidate the BMP reader functions in simworld and this patch, so have only one.

I used BMP's because it was easy to obtain the required code - I will work on moving code around more, and changing it so that simworld and this image reader use the same code.

The code has calls to gdi functions to write out a png screenshot on Windows, but nothing for reading in a png file (or at least not that I can find), and also I want to make sure the code is compatible across platforms, so I need to make sure the code will work on Windows, Linux, and Mac.

I will look to see if there is any easy cross platform way to read in png files as well.

[TurfIt]

libpng is already a dependency for MakeObj.
  trunk/utils/dr_rdpng.cc

[HaydenRead]

libpng is already a dependency for MakeObj.
  trunk/utils/dr_rdpng.cc

Thanks, have taken a look at dr_rdpng.cc (I presume from what I can read in the code it outputs a 24 bit image, which would then need to be converted down to 16 bit for display).

Current Plans
I will look at moving the bmp reading code into the utils folder to match the png reading code
I will look at changing my existing code to check for bmp or png file extension, and use the appropriate code to read in the file
I will look at changing simworld to use the same bmp code for loading in bmp height maps (and possibly look at allowing png height maps to be loaded in).
Update
Attached patch file creates the following new files, as well as patching other files
utils/bmp.h
utils/bmp.cc
gui/components/gui_flowtext_image.h
gui/components/gui_flowtext_image.cc
It adds a dependency to libpng into Simutrans, and requires utils/dr_rdpng.cc to be added to the build.
It allows PNG and BMP images to be used in <img src=> tags.
It replaces the BMP heightmap loading routines from simworld.cc with the same BMP loading method used for loading the images, which allows a wider variety of BMP images to be used as height maps.

The BMP specific code has been moved from gui_flowtext_image to bmp (from previous patch) to allow it to be used for simworld.cc
The bmp code has been refactored to more closely match existing code in Simutrans.
Update
Patch would crash if img tag was used when scenario was not loaded, this has been fixed, and a new version of the patch uploaded.

[Ters]

I think BMP is not worth bothering with. Even Microsoft is no longer using it prominently. I don't think I've seen a BMP in an img-tag since the 20th century. BMP is also not as straight forward a format as one might believe. It supports a variety of color formats and compressions, including JPEG and PNG. Unless we support all, some users will eventually try using something that isn't and get stumped. Simutrans has poor error reporting mechanisms beyond the log file, and I'm not sure documentation writers know how to activate and find the log file.

[HaydenRead]

I think BMP is not worth bothering with. Even Microsoft is no longer using it prominently. I don't think I've seen a BMP in an img-tag since the 20th century. BMP is also not as straight forward a format as one might believe. It supports a variety of color formats and compressions, including JPEG and PNG. Unless we support all, some users will eventually try using something that isn't and get stumped. Simutrans has poor error reporting mechanisms beyond the log file, and I'm not sure documentation writers know how to activate and find the log file.

I understand in regards to BMP, which is why I noted:

BMP Compression Support: None, RLE8, BITFIELDS
BMP Bits Per Pixel Support: 1, 4, 8, 16, 24, 32 (converts to the 16 bit format used by Simutrans on loading).

My thoughts (if this code does get incorporated into the game) is to state that the <img src= > tag has support for PNG and limited support for BMP (noting the above limitations).

An even more limited BMP reader was already in the game for reading in height maps (only read None, RLE8 compression, and 8, 24 bit color formats) (the included BMP reader replaces that, and allows the loading of the BMP images for the img tag).

It would be nice if .jpg, .gif, .tif, .j2k and a host of other file formats could also be implemented, but PNG and limited BMP support seem to be the low hanging fruit for implementation.

I started with BMP mainly for testing purposes, to check that I could get images displaying, and I don't see any point in completely removing the BMP code now that it is already coded, although I have implemented PNG through the libpng inclusion, and if it was easy to implement other formats without too much bloat, I would implement them as well (I haven't actually looked at other formats yet).

[Ters]

I understand in regards to BMP, which is why I noted:My thoughts (if this code does get incorporated into the game) is to state that the <img src= > tag has support for PNG and limited support for BMP (noting the above limitations).

I'd just drop mentioning the support for BMP, even if it is included. That keeps it simple. Writing functionality is one thing, but we also have to support it, that is keep it working and help people using it, for "all eternity".

It would be nice if .jpg, .gif, .tif, .j2k and a host of other file formats could also be implemented, but PNG and limited BMP support seem to be the low hanging fruit for implementation.

For consumers, there are only two image file formats relevant today: PNG and JPEG. JPEG might not be suitable for the kind of graphic Simutrans has. GIF only has support for animations going for it (which also requires support from img-tags), but only does up to 256 colors. TIFF is a monster, and support is rare outside image processing tools. TGA used to be a popular, easy to use file format, but support is equally rare.

[HaydenRead]

I'd just drop mentioning the support for BMP, even if it is included. That keeps it simple. Writing functionality is one thing, but we also have to support it, that is keep it working and help people using it, for "all eternity".

Ok, so leave the bmp support in but don't list it as supported, I can understand that (height maps from what I can see only list ppm support but also support limited bmp at the moment), so it would be the same for images.

For consumers, there are only two image file formats relevant today: PNG and JPEG. JPEG might not be suitable for the kind of graphic Simutrans has. GIF only has support for animations going for it (which also requires support from img-tags), but only does up to 256 colors. TIFF is a monster, and support is rare outside image processing tools. TGA used to be a popular, easy to use file format, but support is equally rare.

I do think sticking just with PNG may be easiest.

One issue I have been thinking on is the search order, as noted it currently goes 'Scenario directory', 'pak directory', 'application directory'. As this allows a scenario to override any Pak or Application images, and Pak to override Application images. (Not that Pak's or the Application currently have any images to load).
Does this seem the correct order to search, or would it be better to have it go the other way round 'Application', 'Pak', 'Scenario'.

I just tested, and it works for 'help files' as well as 'scenario text' (as it should do, because it should work for all flowtext).

Another query, currently it places a new line before and after the image. Would it be better not to place the new line, and let the user determine the positioning by placing their own new lines?

[Leartin]

One issue I have been thinking on is the search order, as noted it currently goes 'Scenario directory', 'pak directory', 'application directory'. As this allows a scenario to override any Pak or Application images, and Pak to override Application images. (Not that Pak's or the Application currently have any images to load).
Does this seem the correct order to search, or would it be better to have it go the other way round 'Application', 'Pak', 'Scenario'.

If you use images in the help file, while they should be delivered with the application, each pakset would need their own images so they fit graphically. Therefore, pak-images need to overwrite application images.
I don't see any place where scenario images and pak images would overlap other than by mistake. So the current order seems useful.

[HaydenRead]

New version of the patch, makes the images inline (so that images don't automatically go on new line).
There were numerous issues with the file.

• Passing pointer to a pointer instead of pointer resulting in garbage image for PNG
• Incorrect color conversion for PNG
• Incorrect caching of images
• PNG images appearing upside down
• Where multiple images were being displayed, the first image was displayed in all locations

These have been fixed in the New 'fixed' patch below.

[An_dz]

makes the images inline

That's the expected from the standard. Good job.

[HaydenRead]

Ok current state:
Re-coding the patch from Scratch.

Looking to place all image loading / saving functionality into the one class.

This includes the existing PPM & BMP loading functionality from loading height maps, although the BMP loading will be expanded to support more types of BMP's, and the PPM is changing from just P6 files to P2, P3, P5 & P6 files (will look at changing heightmap code to better handle P2/P5 Greyscale images). (Ignoring P1 files as they are only B&W images).

This will mean that PNG's will also be able to be used for height maps once the patch is finished and tested.
Attached 'Release Candidate' of the patch, hopefully I have removed all the bugs.

Notes

• Per the attached image, text will scroll down the right hand side of the image, but not the left hand side. Text to the left is top aligned (as there is no way (that I can see) to retroactively change it to bottom aligned as most browsers do)
• The patch support PNG, BMP (limited to the functionality previously discussed), PPM (P3&P6) & PGM (P2&P5) files - these files will also work for loading height maps (P6/BMP were previously used for height maps
• The patch includes the changes Allow load height map with high mountains
• The patch includes loading pure greyscale heightmaps (currently 150 translates to 0, 255 translates to 32... not sure if it should be moved to 128 translates to 0... 150 was approximately 0 using the existing functionality for translating colors).
• Images loaded in FlowText are cached - images loaded as Height Maps are not cached.

Unless there are obvious bugs, or anyone raises any concerns, I do not intend to further change this patch.
Updated to RC2 patch:
Subsequent Flowtexts with images used the same images as the first Flowtext displayed - RC2 fixes this.

[prissi]

After finally having the alpha transparency incorporated, I would like to look into this. Has there any new work done on that in the meantime?

[HaydenRead]

Hi,
I haven't done any further work on this Patch, since the above update RC2 update.

[HaydenRead]

Would there be interest in me recoding this patch to take into account the changes that have since occured within the Simutrans application?

In particular the changed heightmap code, and any changes around alpha transparency?

[prissi]

Yes, I am very busy, so I did not do this myself. But just comitting something wihtout big changes is easily done even in my few spare minutes.

[HaydenRead]

Ok, here is an updated patch that should work against r7847

Information for the patch...
It wipes out large portions of height_map_loader.cc as it now handles the image loading for it... This means that the following files are now supported for height maps, PxM files (PBM, PGM, PPM) although PBM images won't give a good heightmap as they are black and white images, PNG files, and better BMP support (still not complete BMP support, but better than it was).

Introduces an img_loader class that handles loading images, and can provide the images to various systems in requested formats. As above supports PxM, BMP (limited), and PNG files. Also supports saving to PxM, BMP, and PNG, although there is nothing currently that uses these functions, although the screenshot functionality could be changed to use it...

Introduces an gui_flowtext_image_t class for managing images in flow text, and alters the gui_flowtext_t class to facilitate displaying images through the <img src=""> tag...

Introduces a function to get the scenario path from scenarios (so scenarios can use images in their flowtext).

It is set to attempt to read the image from any active scenario first, then the pakset, then the application.

Have successfully compiled and tested on Windows against both MinGW SDL2 build, and MVSC GDI build.

[Yona-TYT]

I wonder why this is not incorporated?

[Ters]

I wonder why this is not incorporated?

It has only been little over a week. Prissi wrote that he was busy. Dwachs is apparently doing script related things. I don't remember seing any other committers recently. It is also summer. Some people might be travelling. Or the weather outside might be more tempting than sitting in front of a computer, or so I hear.

[Isaac.Eiland-Hall]

Or the weather outside might be more tempting than sitting in front of a computer, or so I hear.

LIES

[Dwachs]

@HaydenRead: could you please upload the patch somewhere else? Attachments seem to be broken for now.

[HaydenRead]

@HaydenRead: could you please upload the patch somewhere else? Attachments seem to be broken for now.

imgsrc.patch on Dropbox
I will remove from dropbox once the patch is incorporated into Simutrans.

[Isaac.Eiland-Hall]

@HaydenRead: could you please upload the patch somewhere else? Attachments seem to be broken for now.

My sincere apologies. Answers are slow (and so far useless) from SMF on trying to fix. :(

Anyone who needs, please let me know and I'll set up a subdomain to upload any files via SFTP/FTP that would be publicly accessible.

[Yona-TYT]

Any news on this? I would like to use this to explain things a little better.

[HaydenRead]

Was waiting either for someone else to submit to SVN or for my SVN access to be granted, which is still pending.

[prissi]

Ok, I was 4 weeks on holiday and had just a jub interview in Japan.

I had a short look: First using stdstream is not adviced, as it cannot fail with exceptions (because of the multithreading or SDL2 do not like exceptions much). Also it will fail on any non-intel system (ARM, Power Mac, Amiga). But then, some of the old code had the sample problem ...

I can fix this but not today.

[Dwachs]

@HaydenRead could you please adjust the coding style such that it matches the existing code (e.g. tabs for indentation).

The struct flowtext_t::image_t just wraps an image-id - use image_id instead?

[HaydenRead]

@HaydenRead could you please adjust the coding style such that it matches the existing code (e.g. tabs for indentation).

The struct flowtext_t::image_t just wraps an image-id - use image_id instead?

Ok, I was 4 weeks on holiday and had just a jub interview in Japan.

I had a short look: First using stdstream is not adviced, as it cannot fail with exceptions (because of the multithreading or SDL2 do not like exceptions much). Also it will fail on any non-intel system (ARM, Power Mac, Amiga). But then, some of the old code had the sample problem ...

I can fix this but not today.

Hi Dwachs/Prissi
I will take a look at the code, and work on the advised changes. then submit the update here for further review.
Just as a side note, is there a location that details 'standard coding style' for Simutrans?

[DrSuperGood]

Just as a side note, is there a location that details 'standard coding style' for Simutrans?

...\simutrans\simutrans\trunk\documentation

Although it is not that complete and at times the style of Simutrans seems confused.

[Dwachs]

The style is not completely consistent, and most likely does not obey the rules in coding_styles.txt. Just see that the code you are writing looks similarly to the rest. We are not that religious about following the coding style correct to the dot.

The most important points imho: use tabs for indentation, avoid trailing whitespaces and blank lines, always wrap the body of loops between braces, the first brace of the implementation of a function should be on a new line.

Maybe we should update the coding style.txt?

[TurfIt]

Simutrans coding style didn't really get far on the last attempt...

[DrSuperGood]

Simutrans coding style didn't really get far on the last attempt...

And both attachments are missing lol.

I think as long as the code does not stick out too much is of a decent quality and is clear it is fine. At least until someone writes a proper style guide.