The International Simutrans Forum

Development => Patches & Projects => Considered Patches => Topic started by: HaydenRead on December 22, 2015, 08:20:18 PM

Title: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: HaydenRead on December 22, 2015, 08:20:18 PM
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:

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 (http://forum.simutrans.com/index.php?topic=15049.msg148976#msg148976)
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: captain crunch on December 22, 2015, 09:02:55 PM
Will this add a second BMP reader?
Could the BMP reader in simworld.cc profit from this or vice versa?
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: HaydenRead on December 22, 2015, 09:23:56 PM
Quote from: captain crunch on December 22, 2015, 09:02:55 PM
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:
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.
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: prissi on December 22, 2015, 09:34:04 PM
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.
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: HaydenRead on December 22, 2015, 10:03:44 PM
Quote from: prissi on December 22, 2015, 09:34:04 PM
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.
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: TurfIt on December 22, 2015, 10:08:45 PM
libpng is already a dependency for MakeObj.
  trunk/utils/dr_rdpng.cc
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: HaydenRead on December 22, 2015, 10:54:23 PM
Quote from: TurfIt on December 22, 2015, 10:08:45 PM
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.
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: Ters on December 23, 2015, 10:02:02 AM
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.
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: HaydenRead on December 23, 2015, 08:22:01 PM
Quote from: Ters on December 23, 2015, 10:02:02 AM
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:
Quote from: HaydenRead on December 22, 2015, 08:20:18 PM
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).
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: Ters on December 23, 2015, 11:09:31 PM
Quote from: HaydenRead on December 23, 2015, 08:22:01 PM
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".

Quote from: HaydenRead on December 23, 2015, 08:22:01 PM
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.
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: HaydenRead on December 24, 2015, 12:07:46 AM
Quote from: Ters on December 23, 2015, 11:09:31 PM
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.

Quote from: Ters on December 23, 2015, 11:09:31 PM
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?
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: Leartin on December 24, 2015, 02:26:48 PM
Quote from: HaydenRead on December 24, 2015, 12:07:46 AM
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.
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: HaydenRead on December 25, 2015, 07:14:53 AM
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.
These have been fixed in the New 'fixed' patch below.
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: An_dz on December 26, 2015, 12:22:38 AM
Quote from: HaydenRead on December 25, 2015, 07:14:53 AM
makes the images inline
That's the expected from the standard. ;) Good job.
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: HaydenRead on January 07, 2016, 02:51:29 AM
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
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.
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: prissi on April 20, 2016, 09:45:49 PM
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?
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: HaydenRead on May 01, 2016, 03:32:00 AM
Hi,
I haven't done any further work on this Patch, since the above update RC2 update.
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: HaydenRead on June 20, 2016, 02:09:38 AM
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?
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: prissi on June 20, 2016, 08:36:22 PM
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.
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: HaydenRead on July 08, 2016, 01:50:29 AM
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.
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: Yona-TYT on July 16, 2016, 10:58:59 PM
I wonder why this is not incorporated? ???
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: Ters on July 16, 2016, 11:11:24 PM
Quote from: Yona-TYT on July 16, 2016, 10:58:59 PM
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.
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: Isaac Eiland-Hall on July 17, 2016, 08:16:14 AM
Quote from: Ters on July 16, 2016, 11:11:24 PM
Or the weather outside might be more tempting than sitting in front of a computer, or so I hear.

LIES
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: Dwachs on July 17, 2016, 03:40:41 PM
@HaydenRead: could you please upload the patch somewhere else? Attachments seem to be broken for now.
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: HaydenRead on July 17, 2016, 06:27:43 PM
Quote from: Dwachs on July 17, 2016, 03:40:41 PM
@HaydenRead: could you please upload the patch somewhere else? Attachments seem to be broken for now.
imgsrc.patch on Dropbox (https://www.dropbox.com/s/qnw8mrpqrnrst3f/imgsrc.patch?dl=0)
I will remove from dropbox once the patch is incorporated into Simutrans.
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: Isaac Eiland-Hall on July 17, 2016, 07:02:31 PM
Quote from: Dwachs on July 17, 2016, 03:40:41 PM
@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.
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: Yona-TYT on September 10, 2016, 10:48:28 AM
Any news on this? I would like to use this to explain things a little better.
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: HaydenRead on September 10, 2016, 07:44:24 PM
Was waiting either for someone else to submit to SVN or for my SVN access to be granted, which is still pending.
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: prissi on September 10, 2016, 08:51:31 PM
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.
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: Dwachs on October 09, 2016, 04:11:46 PM
@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?
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: HaydenRead on October 11, 2016, 01:23:51 AM
Quote from: Dwachs on October 09, 2016, 04:11:46 PM
@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?
Quote from: prissi on September 10, 2016, 08:51:31 PM
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?
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: DrSuperGood on October 11, 2016, 01:58:12 AM
Quote
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.
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: Dwachs on October 11, 2016, 06:33:15 AM
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?
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: TurfIt on October 13, 2016, 12:03:26 AM
Simutrans coding style (http://forum.simutrans.com/index.php?topic=5287.msg115795#msg115795) didn't really get far on the last attempt...
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: DrSuperGood on October 13, 2016, 12:31:30 AM
Quote
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.
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: Ters on October 13, 2016, 04:27:41 AM
I just try to follow the style of the surrounding code. That might mean I have to use several styles for one patch.
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: prissi on March 29, 2017, 03:32:24 AM
I updated this patch to the current version, but I noted that the actual imgloader is not ebdian save and crashes with my test bitmaps (which were loaded perfectly fine before). Also the std stream libary may use exceptions, which are not ok with our current threading default libs. Hence here is an updated version, but it requires more code to make it actually working. Probably using the old code for the BMP header processing. Anyway, I stopped on that, but here is a patch that actually applies.

But all the renaming efforts in the past would make probably any older patch useless; insofar loosing al those previous attachments solved this task also somehow ...
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: Yona-TYT on March 30, 2017, 06:50:41 PM
Quote from: prissi on March 29, 2017, 03:32:24 AM
I updated this patch to the current version, but I noted that the actual imgloader is not ebdian save and crashes with my test bitmaps (which were loaded perfectly fine before). Also the std stream libary may use exceptions, which are not ok with our current threading default libs. Hence here is an updated version, but it requires more code to make it actually working. Probably using the old code for the BMP header processing. Anyway, I stopped on that, but here is a patch that actually applies.

But all the renaming efforts in the past would make probably any older patch useless; insofar loosing al those previous attachments solved this task also somehow ...
I am very happy to know that the patch is still alive, I hope to soon use this in the tutorial scenario.
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: Yona-TYT on April 15, 2017, 01:37:36 AM
@prissi
I'm about to release the first stable version of the tutorial, I really need this patch to explain things well.
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: Dwachs on April 15, 2017, 07:42:36 AM
The patch is not finished yet.
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: prissi on April 15, 2017, 01:10:48 PM
It crashes with previously working heightmaps, is not edndian save (Arm, PowerPC) and thus not ready for inclusion. The recent renaming may mean also some further work might be needed.
Title: Re: Patch to allow <img src= > Tag in any location using Flowtext.
Post by: Yona-TYT on December 16, 2017, 11:25:31 PM
It saddens me to know that this patch was never finished. :-[


It takes a lot to explain some things in the tutorial.