The pattern in ImageCairo::drawImage() doesn't use tileRect(). Thats why border-images are drawn wrong in some cases. See: http://www.css3.info/preview/border-image/
Created attachment 21816 [details]
Added support for tileRect() for a correct drawing of border-images. I hope it is effiecent enough.
Created attachment 21818 [details]
for more safety
Created attachment 21823 [details]
cairo_t* cr_surface must be defined before the if clause, to make cairo_destroy(cr_surface). Even if we don't need cr_surface :-(
I changed rectangle and fill with cairo_paint, it's more efficient.
Comment on attachment 21823 [details]
This call to cairo_destroy can pass an uninitialized value for cr_surface which I doubt is safe:
Why does the call to cairo_destroy need to be outside the body of the if block where cr_surface is used? The naming of cr_surface seems like poor choice too: both surface and cr_surface are cairo types, and I can't see any particular significance to the cr_ prefix.
(In reply to comment #4)
> (From update of attachment 21823 [details] )
> This call to cairo_destroy can pass an uninitialized value for cr_surface which
> I doubt is safe:
> + cairo_destroy(cr_surface);
> Why does the call to cairo_destroy need to be outside the body of the if block
> where cr_surface is used?
cairo_destroy decreases the reference count. And every acces to image (where the new surface is stored at the end) caused crashes of webkit.
> The naming of cr_surface seems like poor choice too
rigth. How about:
surface -> clippedImage
cr_surface -> clippedImageContext
Created attachment 22854 [details]
Renamed the functions and destroy surface after using it.
Comment on attachment 22854 [details]
You can force a FloatSize to an IntSize using IntSize(floatSize);
nativeImageForCurrentFrame is supposed to return a pointer to a cached frame image, so you shouldn't be releasing it. Does cairo intentionally have a different memory management model for this? Maybe Images aren't ref-counted in Cairo?
Created attachment 23291 [details]
cairo_surface's are ref-counted in cairo. Thats why I put auto_ptr<ImageBuffer> above the if-clause. Otherwise the image is cleaned-up to early and I get a ref-count error (ref-count lower than zero).
But I measured the call and it takes only 0.000047s extra time as worst-case (normaly 0.000009s).
I don't know (or understand) what you mean with nativeImageForCurrentFrame().
I need a cairo_suface_t* out of the imageBuffer and nativeImageForCurrentFrame() can give it.
I haven't r? the patch, to discuss the changes again.
(In reply to comment #8)
> Created an attachment (id=23291) 
> Cairo border-image
> cairo_surface's are ref-counted in cairo. Thats why I put auto_ptr<ImageBuffer>
> above the if-clause. Otherwise the image is cleaned-up to early and I get a
> ref-count error (ref-count lower than zero).
> But I measured the call and it takes only 0.000047s extra time as worst-case
> (normaly 0.000009s).
> I don't know (or understand) what you mean with nativeImageForCurrentFrame().
> I need a cairo_suface_t* out of the imageBuffer and
> nativeImageForCurrentFrame() can give it.
> I haven't r? the patch, to discuss the changes again.
Maybe you could do something like this (completely untested) instead of using ImageBuffer?
if (tileRect.size() == size())
pattern = cairo_pattern_create_for_surface(image);
//IntRect imageSize = enclosingIntRect(tileRect);
cairo_set_source_surface(cr, image, -tileRect.x(), -tileRect.y());
pattern = cairo_pop_group(cr);
(In reply to comment #9)
Thanks for your reply. It is a great idea. I tested it with many modifications and spend much time on it, but didn't get it to work.
The problem is, the geometry of the context isn't reset. That means, the dimension of the context is different on every time.
The second problem is: You don't only move the the image by -tileRect.x(), -tileRect.y(), you need a part of it, e.g. if you have a 90x90 surface, you need 5 parts. One would be (30, 0, 30, 30)/(x,y,width,height). I wasn't able to realise it with push_to_group. One thing why i didn't manage it was that a rect with the coordinate 0,0 wasn't placed at the top,left of the context, it was somewhere out of the view area.
I have to admit, that this behavior makes it difficult to understand the sense of push_to_group.
I need a context with a height and width of the surface. Otherwise I'll get problems with the pattern. It will give more than just this part of a surface.
If it is possible to get the width and height of the current context and the coordinates of the first pixel in the top/left of the context, you can scale and transform the created image (pop_to_group).
But I don't believe that it will be more affective than just creating a new surface with the height and width given by tileRect and draw the part of the image into it's context.
If you have more suggestions, please post it. It would be great to do it without an new surface or ImageBuffer and an acceptable complexity.
Comment on attachment 23291 [details]
This would be super-easy to approve if Gtk supported pixel tests. ;)
@eseidel: don't upset the gtk gods.
I talked about this problem in #cairo on irc and it was assured me, that this is already the fastest way.
cairo_pop_to_group does not work here. We still create a surface, but with a undefined size. That means we have no win, it is neither a speed-up, nor more resource affective.
Comment on attachment 23291 [details]
Please add a note in the changelog pointing to a LayoutTest which will test this once Gtk has pixel tests. If there is no such test, please add one.
Otherwise this looks sane enough.
landed in r44081.