Created attachment 167790 [details] [review]
Factor out the code to store a pixbuf in EogScrollView

Created attachment 167791 [details] [review]
Store a cairo_surface_t in EogScrollView for the GdkPixbuf to be rendered

Using this later during the rendering should improve performance.

Created attachment 167792 [details] [review]
Replace GDK rendering in EogScrollView with cairo

This is still missing proper background drawing for images with alpha
channel. Also it becomes awfully slow when rendering an image to fit
completely in the screen and this is big enough (3888×2592).

Adding Benjamin to the CC, so that he can tell me that I'm doing it all wrong :-)

(In reply to comment #0)
> On a related note, I notice that a lot of the code that's currently used for
> svg rendering can be reused for this. I'm not gonna do it now, but I think that
> at some point we need to rework everything to avoid code duplication.

I was guessing something like that as well lately. Good to know! :)

(In reply to comment #3)
> This is still missing proper background drawing for images with alpha
> channel. Also it becomes awfully slow when rendering an image to fit
> completely in the screen and this is big enough (3888×2592).

Hmm, seems to be not filtering at all in this case here although the debug out says it does. Strange.

(In reply to comment #5)
> (In reply to comment #3)
> > This is still missing proper background drawing for images with alpha
> > channel. Also it becomes awfully slow when rendering an image to fit
> > completely in the screen and this is big enough (3888×2592).
> 
> Hmm, seems to be not filtering at all in this case here although the debug out
> says it does. Strange.

Correction: it does filter, but not with the same quality as before.

Created attachment 168005 [details] [review]
Use gdk_window_create_similar_surface() for EogScrollView

Created attachment 168006 [details] [review]
Fully rely on cairo to render the image in EogScrollView

This is an initial patch to showcase how EogScrollView performs when
we discard all the internal optimizations and rely on cairo to render
the image. In general, performance is quite good, with a few caveats:

- Rendering svg is broken (librsvg is not building because of the
  changes in API in GTK+ 3)
- There is flickering in the rendering of the background for images
  with an alpha channel
- Images bigger than 4096×4096 seems to be quite slow (at least with
  an nvidia card)

Comment on attachment 167792 [details] [review]
Replace GDK rendering in EogScrollView with cairo

This patch would be obsolete by the patches I have attached today.

Created attachment 168010 [details] [review]
Enable double-buffer in EogScrollView

Now that we rely on cairo, there is no reason to try to be über-smart.
Enabling double-buffer fixes the flickering and the slowness with huge
images.

Created attachment 168067 [details] [review]
Fix the SVG rendering

Take the transformation bits from the existing code and use them to
add proper svg rendering support.

So, that's it. I am pretty satisfied with the performance, at least with my nvidia card. I would love to get feedback from more people as for the performance, specially when it comes to big images and svg.

Felix, you commented something on irc, about svg being slow. For me, these patches work quite well. Can you give them a try?

(In reply to comment #12)
> So, that's it. I am pretty satisfied with the performance, at least with my
> nvidia card. I would love to get feedback from more people as for the
> performance, specially when it comes to big images and svg.
> 
> Felix, you commented something on irc, about svg being slow. For me, these
> patches work quite well. Can you give them a try?

I guess that was a mostly because I didn't do the clipping mask right yesterday (it was just a quick hack reenabling the old code) and thus the image was redrawn completely all the time. Also I was used to the "old" style of rendering SVGs which hides the small delay a little bit.

Re-enabling double buffering helps too.

Performance seems to be good as well (though my box is not really slow; I'll try and see what my laptop has to about this), GIF-Animations work filtered in fullscreen now. We should still keep the possibility to disable filtering, IIRC there were usecases where people wanted to view their images unfiltered.

Created attachment 168157 [details] [review]
Remove unneeded code

Actually, #ifdef it out and comment it. It's nice code anyway.

Created attachment 168158 [details] [review]
Replace GdkInterpType with cairo_filter_t in EogScrollView

Created attachment 168159 [details] [review]
Honour the antialiasing-in and antialiasing-out EogScrollView properties

(In reply to comment #13)

> Performance seems to be good as well (though my box is not really slow; I'll
> try and see what my laptop has to about this), GIF-Animations work filtered in
> fullscreen now. We should still keep the possibility to disable filtering, IIRC
> there were usecases where people wanted to view their images unfiltered.

I hadn't honoured the properties for this. Thanks for pointing it out. Now it's working fine with new the patches above.

A little thing I noticed:

When zooming the images tend to get a small black border.

(In reply to comment #18)
> When zooming the images tend to get a small black border.

Assuming you're talking about zooming out:
That can be fixed by using cairo_pattern_set_extend (cairo_get_source (cr), CAIRO_EXTEND_PAD)

(In reply to comment #19)
> Assuming you're talking about zooming out:
> That can be fixed by using cairo_pattern_set_extend (cairo_get_source (cr),
> CAIRO_EXTEND_PAD)

No, it affects both directions (the image just needs to be small enough to see it when zooming in).
Interestingly doesn't affect SVGs or GIF animations (alpha channel?).

cairo_pattern_set_extend didn't help, at least in the places I tried (e.g directly after setting the image as source surface).

I can't reproduce that; can you attach an image that exposes this?

Created attachment 168560 [details]
screenshot showing the borders

You can take attachment 158376 [details] from bug 615356. It gets some black borders on zooming (see attached screenshot)

Ok, now I see them :) I had the black background enabled, that's why they weren't noticeable. I'll have a look to the cause soon.

-       if (gdk_pixbuf_get_has_alpha (pixbuf)) {
-               format = CAIRO_FORMAT_ARGB32;
-               content = CAIRO_CONTENT_COLOR | CAIRO_CONTENT_ALPHA;
-       } else {
-               format = CAIRO_FORMAT_RGB24;
-               content = CAIRO_CONTENT_COLOR;
-       }
+       format = CAIRO_FORMAT_ARGB32;
+       content = CAIRO_CONTENT_COLOR | CAIRO_CONTENT_ALPHA;


This is basically the way to fix it.

Created attachment 168713 [details] [review]
Improve the rendering of scaled images

Not using CAIRO_CONTENT_COLOR added a black border to images, when
painted scaled. Also, a extend mode of CAIRO_EXTEND_PAD, clipped to
the image rectangle, prevents the image borders to be drawn blending
out with the background.

(In reply to comment #25)
> Not using CAIRO_CONTENT_COLOR ...

This should be CAIRO_CONTENT_ALPHA. Fixed the comment locally.

Felix, do you have any objection to committing this to master?

(In reply to comment #27)
> Felix, do you have any objection to committing this to master?

CLAMP (xofs, 0, xofs) == MAX (0, xofs) in attachment 168005 [details] [review]?


Go ahead. I was wondering already. :)
We should see to get a 2.91.0 test-release out soonish (let's see if r-t has some plans already) to get this (hopefully) tested more widely.

(In reply to comment #28)
> (In reply to comment #27)
> > Felix, do you have any objection to committing this to master?
> 
> CLAMP (xofs, 0, xofs) == MAX (0, xofs) in attachment 168005 [details] [review]?

Uh, actually attachment 168006 [details] [review] ;)

Ok, fixed the above and also did a few extra cleanups. Committed to master. Let's see what we get now..