After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 747446 - Fix HiDpi rendering
Fix HiDpi rendering
Status: RESOLVED FIXED
Product: gnome-photos
Classification: Applications
Component: general
3.16.x
Other All
: Normal normal
: ---
Assigned To: GNOME photos maintainer(s)
GNOME photos maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2015-04-07 12:59 UTC by Andreas Nilsson
Modified: 2015-11-03 18:14 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
screenshot (812.89 KB, image/png)
2015-04-07 12:59 UTC, Andreas Nilsson
  Details
gegl-gtk-view, gegl-gtk-view-helper: Fix HiDpi rendering (6.46 KB, patch)
2015-11-02 14:04 UTC, Debarshi Ray
committed Details | Review

Description Andreas Nilsson 2015-04-07 12:59:19 UTC
Created attachment 301065 [details]
screenshot

I get this square artifact on all my photos, where the spinner have been
Comment 1 Debarshi Ray 2015-04-09 09:22:33 UTC
Yes, I have seen it too. Something wrong with the drawing in our preview widget.
Comment 2 Debarshi Ray 2015-06-22 10:41:38 UTC
Is this something that you see more commonly on a HiDpi display? At least that seems to be the case for me.

Anyway, I have a few fixes to gegl-gtk that should make the rendering more robust and prevent these problems. I will attach some patches shortly.
Comment 3 Andreas Nilsson 2015-06-22 11:01:03 UTC
Yes, mostly on the laptop with the HiDPI screen.
Maybe I've seen it on the other laptop too, but I'm not sure.
Comment 4 Debarshi Ray 2015-11-02 14:04:34 UTC
Created attachment 314640 [details] [review]
gegl-gtk-view, gegl-gtk-view-helper: Fix HiDpi rendering

This turned out to be easier than I thought.

We weren't taking the scale factor into account when rendering on HiDpi, so you would often see that the image is hazier than it looks on Eye of GNOME. Also HiDpi screenshots would be rendered at a larger size on a HiDpi display (which, by the way, is also a problem in eog). Things should now look sharper and HiDpi screenshots should look like the HiDpi widgets.

Anyway, fixing the rendering on HiDpi seems to have magically fixed the square artifacts too. I guess we were introducing some rounding error when rendering to a 1x1 cairo surface at a lower zoom and then upscaling to a 2x2 window.
Comment 5 Debarshi Ray 2015-11-03 11:08:29 UTC
Comment on attachment 314640 [details] [review]
gegl-gtk-view, gegl-gtk-view-helper: Fix HiDpi rendering

Pushed to master.
Comment 6 Debarshi Ray 2015-11-03 11:19:55 UTC
I tested it out on a LoDpi system today (the patch was written on HiDpi), and it worked as expected.

I am still not sure how this could have fixed the square artifacts, but it definitely improves things on HiDpi displays - sharper previews, and HiDpi screenshots are shown at the widget sizes.

However, my initial plan with respect to the square artifacts [*] still stands.

Currently, we (ie. our internal copy of gegl-gtk) always do a gegl_node_blit for the clip rectangle set on the Cairo context that is passed to the draw vfunc. I think, the safer (and faster?) thing to do would be to blit the entire GeglNode to a Cairo image surface, whenever the graph or the size of the widget changes, and re-use it on successive draws relying on the Cairo clip rectangle to take care of the clipping.

That should avoid all the rectangle computation, which is probably where the bug that causes the artifacts is. As a nice side-effect, we would be blitting the node much less.

[*] I have seen them when running the gegl-gtk tests/examples on HiDpi.
Comment 7 Debarshi Ray 2015-11-03 18:08:31 UTC
(In reply to Debarshi Ray from comment #6)

To summarize today's discussion in #gegl on GIMPNet ...

> However, my initial plan with respect to the square artifacts [*] still
> stands.
> 
> Currently, we (ie. our internal copy of gegl-gtk) always do a gegl_node_blit
> for the clip rectangle set on the Cairo context that is passed to the draw
> vfunc. I think, the safer (and faster?) thing to do would be to blit the
> entire GeglNode to a Cairo image surface, whenever the graph or the size of
> the widget changes, and re-use it on successive draws relying on the Cairo
> clip rectangle to take care of the clipping.
> 
> That should avoid all the rectangle computation, which is probably where the
> bug that causes the artifacts is. As a nice side-effect, we would be
> blitting the node much less.
> 
> [*] I have seen them when running the gegl-gtk tests/examples on HiDpi.

Obviously this comes at a constant memory cost, and it won't work with large images at high zoom levels. However, that is not an issue with gnome-photos because we don't let the user zoom to arbitrarily high zoom levels and we probably don't need to because we are not gimp. So, worst comes to worse and we can't figure out where the co-ordinate calculation is going wrong, we can still do it.

Regarding performance ...

Lately pippin has been trying to speed up gegl_node_blit even more, so we need to profile how an even faster [*] gegl_node_blit compares with caching the Cairo surface.

[*] Note that blitting on gegl-0.3 is generally faster than in 0.2, and using "cairo-ARGB32" as the Babl format has sped it up even more. So, performance-wise, things are already much better than they used to be.
Comment 8 Debarshi Ray 2015-11-03 18:09:01 UTC
From #gnome-hackers on GIMPNet:

17:49 <andreasn> rishi: finally built the right version. Now on 3.19.1 and the
      artifact is gone!
17:49 <andreasn> \o/
17:49 <andreasn> thanks for the fix!
Comment 9 Debarshi Ray 2015-11-03 18:14:03 UTC
Also pushed to gnome-3-18.