WIP: SVG images support for TDEHTML #156

Overview

The aim of this PR is to add support for showing SVG documents embedded in HTML documents via the <img> tag, for applications which use TDEHTML to be able to display SVG icons normally (e.g. Konqueror's launch page about:konqueror, kweatherreport, etc.).

This PR does not add support for:

the <svg> element
tiled SVG backgrounds

(might be added later if need arises for these)

Commits description

The first commit (dbd8fb94cc) is KSVGIconEngine-related. It allows passing 0 values for width and height to use defaults. This is useful for cases when these attributes are not specified in any way, so using the image's defaults is needed (pixmap(...)) in contrast to when width and height are specified (scaled_pixmap(...)).

How to test this commit:
```
$ ksvgtopng 0 0 <some svg file> <name of PNG output file>
```
Expected outcome if it works: Output image has the SVG documents's default width and height (which can be checked e.g. via Inkscape).

Expected outcome if it doesn't work:
```
art_render_new: x0 >= x1 (x0 = 0, x1 = 0)
Segmemtation fault
```
The second commit (85cac902ac) adds SVG rendering support into TDEHTML. The process is the following and it respects, more or less, the standard process followed by TDEHTML for images:
1. A special rendering function is made available in misc/svgrender.cpp
2. In CachedImage class (misc/loader.cpp) the image is loaded, parsed (once it is fully loaded) and rendered with its default size. Some attributes (isVectorImage(), svgExtension(), svg()) are stored for later use in rendering/render_image.cpp.
3. If the image needs scaling, instead of resizing it like a raster image, the image is re-rendered with the new width and height in rendering/render_image.cpp.
How to test this commit:
1. Download and extract tdehtml_svg_test.tar.gz (contains an HTML document, the BasKet icon as SVG and as PNG).
2. Open the HTML document in Konqueror.
Expected outcome: see attached screenshot.

Variant 1: check if icons in about:konqueror are rendered correctly (test with a SVG icon theme, e.g. Masalla).
Variant 2: check if icons in kweatherreport are rendered correctly, either by clicking on the KWeather Kicker applet or manually launching it: kweatherreport [city.airport code] (test with a SVG icon theme)

NOTE: Squash before merging

# Overview The aim of this PR is to **add support for showing SVG documents embedded in HTML documents via the `<img>` tag**, for applications which use TDEHTML to be able to display SVG icons normally (e.g. Konqueror's launch page `about:konqueror`, kweatherreport, etc.). This PR does not add support for: * the `<svg>` element * tiled SVG backgrounds (might be added later if need arises for these) ## Commits description * The first commit (dbd8fb94cc) is KSVGIconEngine-related. It allows passing 0 values for width and height to use defaults. This is useful for cases when these attributes are not specified in any way, so using the image's defaults is needed (`pixmap(...)`) in contrast to when width and height are specified (`scaled_pixmap(...)`). How to test this commit: ``` $ ksvgtopng 0 0 <some svg file> <name of PNG output file> ``` Expected outcome **if it works**: Output image has the SVG documents's default width and height (which can be checked e.g. via Inkscape). Expected outcome **if it doesn't work**: ``` art_render_new: x0 >= x1 (x0 = 0, x1 = 0) Segmemtation fault ``` * The second commit (85cac902ac) adds SVG rendering support into TDEHTML. The process is the following and it respects, more or less, the standard process followed by TDEHTML for images: 1. A special rendering function is made available in `misc/svgrender.cpp` 2. In `CachedImage` class (`misc/loader.cpp`) the image is loaded, parsed (once it is fully loaded) and rendered with its default size. Some attributes (`isVectorImage()`, `svgExtension()`, `svg()`) are stored for later use in `rendering/render_image.cpp`. 3. If the image needs scaling, instead of resizing it like a raster image, the image is re-rendered with the new width and height in `rendering/render_image.cpp`. How to test this commit: 1. Download and extract `tdehtml_svg_test.tar.gz` (contains an HTML document, the BasKet icon as SVG and as PNG). 2. Open the HTML document in Konqueror. Expected outcome: see attached screenshot. **Variant 1**: check if icons in `about:konqueror` are rendered correctly (test with a SVG icon theme, e.g. *Masalla*). **Variant 2**: check if icons in kweatherreport are rendered correctly, either by clicking on the KWeather Kicker applet or manually launching it: `kweatherreport [city.airport code]` (test with a SVG icon theme) ---- **NOTE: Squash before merging**

test.png

78 KiB

tdehtml_svg_test.tar.gz

19 KiB

Hi Philippe,
is the segmentation fault a result of the changes or was it there even before? The commit seems simple enough, so I would be surprise if it causes a SEGV

Hi Philippe,
is the segmentation fault a result of the changes or was it there even before? The commit seems simple enough, so I would be surprise if it causes a SEGV

The segfault caused is an assertion by libart when either requested width or height are <= 0 (which would be invalid).

The changes make KSVGIconEngine assume 0 width and height mean image default width and height (before this there was no way to use default values for width and height from the image itself). Thus the default values are used and the segfault is avoided. This means it will still crash if negative width or height are requested, but this is an improbable case.

Ok thanks for the explanation. That make sense.

Status update: I'm nearly there. Little remains to be fixed before I push the remaining changes to this PR.

Status update: I'm nearly there. Little remains to be fixed before I push the remaining changes to this PR.

That sounds good! We'll look forward.

In any case, independently of it, we can probably integrate the tdemarkdown under the TDE umbrella.

In any case, independently of it, we can probably integrate the tdemarkdown under the TDE umbrella.

Yes, it is a good idea. Would tdemarkdown remain an independent repository or make it a part of tdelibs? or maybe tdaddons? or other ideas?

Yes, it is a good idea. Would tdemarkdown remain an independent repository or make it a part of tdelibs? or maybe tdaddons? or other ideas?

tdemarkdown is lightweight enough to make it part of either tdelibs or tdeaddons without much added size penalty. That said, the point of making it was more or less to be able to display the README.md files in a quick and simple way.

tdemarkdown is lightweight enough to make it part of either tdelibs or tdeaddons without much added size penalty.

Exactly. tdelibs seems the best choice to me and that would offer the part to any application that would like to use it.
@SlavekB what do you think?

tdemarkdown is lightweight enough to make it part of either tdelibs or tdeaddons without much added size penalty.

Exactly. tdelibs seems the best choice to me and that would offer the part to any application that would like to use it.
@SlavekB what do you think?

I considered pros and cons for tdelibs, tdebase and tdeaddons. Because it seems good for markdown to be in basic libraries, tdeaddons seems less suitable. Because tdebase contains primarily basic applications, while tdemarkdown is a component, then as the most suitable seems tdelibs.

It would be good to rename the component to libtdemarkdownopart to remains the opportunity for future creation of the libtdemarkdown shared library. Like libtdehtml + libtdehtmlpart.

Finally it's ready for testing (see first comment -second commit- on how to test).

I added a small commit about which I assume it will then be squashed to the previous one.

@SlavekB Is the PR okay otherwise?

@SlavekB Is the PR okay otherwise?

I builded tdelibs with these patches to later afternoon I could do a test on my computer.

I did the test and I have several observations:

I tested the opening of git repository tdesudo in Konqueror. In the place where there is a graph of language translation status, instead of an unavailable image icon, there is now to see the picture of the right dimensions, but with no content – just white.
For some pages (for example, our news server iDnes), where some images in svg were previously missing (probably, I did not even notice, because CSS rendering is not okay), there is now a crash due to the problem of processing such images. It seems that there is a size 0×0. In the console I see:

art_render_new: x0 >= x1 (x0 = 0, x1 = 0)

In the case of a not loaded images (html mail with alert to load external images), there is a crash in KMail.

Problems 2 and 3 are resolved in the added commits. In the case of a problem 2, there is solved only a consequence – I did not find the beginning of the problem – that is, there was no attempt to render the image when the size is 0×0.

Thank you for testing, I will repeat these tests for myself and see how to solve remaining points.

I did not find the beginning of the problem – that is, there was no attempt to render the image when the size is 0×0.

I dont know if it is relevant, but the second commit (TDEHTML-relevant) passes size 0x0 to KSVGIconEngine (to avoid changing order of parameters in constructor function) as a specjal case to be substituted with the SVG image's default size, which is implemented with the first commit. So this error would probably occur if the second commit was applied but not the first one.

Now there is a fatal problem that needs a different solution. See comment.

I'm thinking of some API changes to KSVGIconEngine to allow using TQDomDocument or TQString as SVG data and also change order of parameters of existing load(width, height, path) to load(path, width, height) so that width and height parameters can be omitted.

I tested the opening of git repository tdesudo in Konqueror. In the place where there is a graph of language translation status, instead of an unavailable image icon, there is now to see the picture of the right dimensions, but with no content – just white.

I tried to do so and I could see this too. Then I tried downloading the SVG picture and running ksvgtopng on it. This resulted in a transparent image. This probably means that KSVGIconEngine can't render it for some reason. I think KSVGIconEngine was not actually meant for stuff more complex than icons and the problem might be some unsupported stuff (I can think of links).

In contrast, Konqueror's KSVG plugin displays it just fine. Maybe we should actually consider using KSVG instead of KSVGIconEngine? Then we won't even need modifying the API of KSVGIconEngine and let it handle just the icons. But KSVG is in tdegraphics, not tdelibs, so how to integrate it with TDEHTML then?

I think I'll try implementing dynamic loading and embedding of the KSVG part from TDEHTML (via TDETrader and KService). This way the part will get automatically loaded if ksvg from tdegraphics is installed and rendering will be better.

I'll probably try this in a separate branch, which I'll create a PR for when/if I have an acceptable result. KSVGIconEngine is a dead end IMO, it is useful only for what it was initially created, simple icons.

Yes, the KSVGIconEngine seems to require many fixes and possibly copy some parts of the code from KSVG in tdegraphics. It looks like futile effort. It will be great if there will be possible to use of KSVG part instead of KSVGIconEngine.

Yes, the KSVGIconEngine seems to require many fixes and possibly copy some parts of the code from KSVG in tdegraphics. It looks like futile effort. It will be great if there will be possible to use of KSVG part instead of KSVGIconEngine.

I agree, it would be a better option.

 	d->width = width; // this sets default for no width -> 100% case
 	if(rootElement.hasAttribute("width"))
 	{
 		d->width = d->helper->toPixel(rootElement.attribute("width"), true);

WIP: SVG images support for TDEHTML #156

Overview

Commits description

Reviewers