Linear and Radial gradients support

[tordex]

I'm near to finish implementing linear and radial gradients. You can find current implementation in this branch https://github.com/litehtml/litehtml/tree/gradient. Example of of gradients rendering:

Important

The interface of document_container was changed. I've removed draw_background super-method and added 4 new methods for each fill/draw type:

virtual void draw_image(litehtml::uint_ptr hdc, const background_layer& layer, const std::string& url, const std::string& base_url);
void draw_solid_fill(litehtml::uint_ptr hdc, const background_layer& layer, const web_color& color);
void draw_linear_gradient(litehtml::uint_ptr hdc, const background_layer& layer, const background_layer::linear_gradient& gradient);
void draw_radial_gradient(litehtml::uint_ptr hdc, const background_layer& layer, const background_layer::radial_gradient& gradient);

What do you think about this? Comments are appreciated!

[stasoid]

This interface is confusing to me:

• Why background_image and background color are not part of background_layer? (This is especially weird considering that conceptually, background image/color is the primary constituent of a background layer).
• Why draw_image and draw_solid_fill accept background_layer? (I mean I know why, but it is confusing as an API)

I would much prefer to have one method, document_container::draw_background_layer(uint_ptr hdc, const background_layer& layer), with background_layer having background_image, background color, and is_final, and let the container switch on layer type internally. I see you don't like it, you call you it super-method, but I don't see any problem with it. Maybe passing multiple layers was overkill, but I think one layer is fine.

container could still have a clean implementation - separate private functions for different types of background_layer, and then draw_background_layer would just call them. I don't see the need to lift this distinction to API level.

I like how background_layer concept encapsulates different types of images, and I think this is a nice object to pass through API boundary.

On the other hand, using draw_background_layer to draw regular images is also a bit confusing, but I still like this option more.

[stasoid]

After some more thinking I realized that I actually like your idea to have more basic drawing functions in the interface. My problem is the mentioning of the background in those functions. This is what I would like to have:

void draw_image          (uint_ptr hdc, const string& url, pixel_rect rect);
void draw_solid_rect     (uint_ptr hdc, web_color color,   pixel_rect rect);
void draw_linear_gradient(uint_ptr hdc, const linear_gradient& gradient, pixel_rect rect);
void draw_radial_gradient(uint_ptr hdc, const radial_gradient& gradient, pixel_rect rect);

That is, those functions don't have to be concerned about gradients, they should not know what they are used for. They just do their little thing - fill rectangle with something. draw_image takes a single url, already combined with baseurl by litehtml. All the clipping is also done by litehtml. If background has repeating image litehtml calls draw_image multiple times.

If we add draw_circle/draw_solid_circle, we can get rid of draw_list_marker, another too-high-level function.

[tordex]

All the clipping is also done by litehtml.

I like this idea.

draw_image takes a single url, already combined with baseurl by litehtml.

With image is not so easy. Background images have some important options: background-repeat, background-origin. It is possible to calculate repeats/origins into single call of draw_image from litehtml. But I guess it can be done more efficient on the container side. So we have to pass these options to the draw_image. This is valid for gradients also, but with gradients we have gradient parameter.

[stasoid]

I guess if litehtml takes care of clipping only repeat parameter is needed. - Nope, there is background-repeat: space. I guess you're right, if we care about performance we need to pass those pesky background parameters.

[stasoid]

I wonder how slower drawing with cairo_container would be if to draw each individual image separately instead of using CAIRO_EXTEND_REPEAT. Maybe it wouldn't be too bad, litehtml performance issues are usually in the layout.

[stasoid]

I tested cairo_container performance on this file:

<!doctype html>
<style>
html {
	background-image: url("tile16.png");
	background-repeat: repeat repeat;
}
</style>

Where tile16.png is 16x16px. Canvas size is 1600x850px. I measured litebrowser WM_PAINT handler performance on litehtml gradient branch vs just calling draw_pixbuf in a loop in container_cairo::draw_image:

		int size = 16;
		for (int y = 0; y < 850; y += size)
			for (int x = 0; x < 1600; x += size)
				draw_pixbuf(cr, bgbmp, x, y, size, size);

cairo_fill with CAIRO_EXTEND_REPEAT: (4+9+3+3+3+4+3+3+3+3)/10=4ms
draw_pixbuf in a loop: (19+11+12+24+28+12+11+12+12+18)/10=16ms

So yeah, not bad at all. On a real html file this would be dwarfed by style computation and layout.
So I still think having the simple void draw_image(uint_ptr hdc, const string& url, pixel_rect rect); is a good idea. (Background handling logic would be in litehtml and it would call draw_image in a loop).

For me API purity is more important than 16ms. Maybe it's different for you.
I spent so much time on this that I forgot why I care so much. Maybe passing background_layer is not that bad after all. But I still think calling this background_layer is confusing because the main part of background_layer is the image. But calling it something else, like image_placement, would be even more confusing... I don't know.

[kobalicek]

If you use a high performance library to render litehtml content like Blend2D it really matters how litehtml presents the draw options.

I think that what would make most sense is somehow abstracting the background which is used, because theoretically you can use gradient content such as text, but the current interface is just for backgrounds.

For example something like this would be much better from API perspective:

void draw_background(litehtml::uint_ptr hdc, const background_layer& layer, const style& style);

The function would have to examine the style basically, but that would be okay as implementations that don't support gradients would just do solid fills I guess.

BTW there is also conic gradient in CSS - it would be worth having all 3 if you are adding the support. Cairo doesn't support these gradients, but other libraries such as Blend2D and Skia do.

BTW I have a Blend2D container for litehtml and it's really great!

[tordex]

@stasoid thank you for testing. As for images there are two ways HTML/CSS processes the images:

1. <img> tag
2. background-image (background) CSS property

For <img> tag we have the position and the url parameters to draw the image. For the background we have some additional parameters. That's why draw image interface can be confused. Generally <img> tag is simplified background-image property. That's why I didn't create separate function to draw image from <img> tag.

BTW, gradients are images too in terms of CSS. So there are two type of backgrounds from CSS view point: solid color and images.

For example something like this would be much better from API perspective:

void draw_background(litehtml::uint_ptr hdc, const background_layer& layer, const style& style);

The function would have to examine the style basically, but that would be okay as implementations that don't support gradients would just do solid fills I guess.

@kobalicek, Generally this is how draw_background is declared right now. What I don't like this this implementation:

1. The style struct/class must have union with all supported background styles.
2. Additional logic in container implementation to select the valid style. But yes, it can be not important.

Looking at structures I've created in the gradient branch I see:

1. Most members of the background_paint now in the background_layer
2. Background specific classes are not so complicated.

So maybe this implementation of background interface is not so bad:

void draw_background(litehtml::uint_ptr hdc, const background_layer& layer);

Just unite all style specific (image, color, gradients) into the background_layer class.
Other advantage of single method: if something will be changed in structures (add new gradient for example), you have to just rebuild app. With different functions you have to implement "empty" function.

PS. It is difficult decision, thank you for helping in selecting the best implementation.

[kobalicek]

I'm just elaborating as I think having a single function to draw a background is better than 4 :)

When it comes to gradients there is one annoying thing. Usually most libraries implement gradient rendering by using a color LUT, which is generated from stops. Some libraries would cache this LUT in a gradient/brush object, some other libraries may have external cache or no cache at all. This means that in order to have a really efficient rendering of gradients that are used as a backgrounds of small shapes/spans, it would be great to have a "handle" or some representation of the gradient so it doesn't have to be recreated every time you hit the draw_background() call. Not sure it's part of the current API, I haven't checked it in detail.

[stasoid]

I am writing a standard-compliant CSS parser now. I am mentioning it here because I will rewrite everything in terms of CSS tokens, including gradients. It sucks that a good part of your work on gradients will be almost immediately replaced.

[tordex]

It is interesting

[memecode]

I'm happy with this branch. I've converted my view class over to use the new API and also added round corner support to get this working: