Hey, On Tue, Mar 08, 2016 at 07:34:49PM -0500, Matthias Clasen wrote:
Shouldn't the view redraw as needed, when its model changes ?
Ideally, yes. TLDR: Doing the right thing would require more intrusive changes, and I am not comfortable making them at the eleventh hour. In comparison, this is a quick and dirty way to achieve the same thing. Longer explanation follows ... The PhotosImageView widget originated as a fork of the toy gegl-gtk widget. Due to not having any real life users, gegl-gtk is not perfect, and even though we have evolved the code a bit, there are still some hacks to plaster over the cracks. PhotosImageView draws the result of a graph (source at one end and then some operations). The result of the graph can change if: (a) the connectivity of the nodes change (b) the properties of the nodes change (c) we process the graph after (a) or (b) has happened to generate new pixels The widget can implicitly handle (a) or (b). eg., it can reposition and resize the image if you apply a crop. However, it needs some help with (c). GEGL processes the graph in smaller bits. If you have 1000x1000 image, it might process it in 100x100 chunks. This is nice because we can use an idle handler and turn the processing into an asynchronous operation to avoid freezing the UI. (Even if this is purely CPU intensive and no I/O is done, it can occupy the CPU for a second or two.) PhotosImageDraw gets notified when each chunk is finished, but it doesn't know when all of them are processed. Queueing a redraw after each chunk is problematic because it leads to a checkerboard effect because you can see the chunks being updated. The application hacked around this by queueing a redraw from the photos_base_item_process_async callback. We only needed to do it from the container holding PhotosImageView, so it was quick and easy. Except, now, we have to do it from a different part of the code - the notification. So it was quick (and safe) to just turn the redraw into a GAction that can be activated from elsewhere and is implemented by the widget's container. The correct thing to do would be to teach PhotosImageView to keep track of the chunks. I tried to do it [1] during my attempt to animate changes to the graph - cross fade between filters, rubber band effect when cropping. That exposed some bugs in GEGL's processing - it emitted too many (and sometimes with bogus values) notifications. I fixed some. Some I punted, because I was not sufficiently happy with the larger animation code anyway. So, something that can be targetted for GNOME 3.22, but not going to be safe for 3.20. Thanks for taking a look at the code! Cheers, Rishi [1] https://git.gnome.org/browse/gnome-photos/log/?h=wip/rishi/imageview
Attachment:
pgpKpA7e67_EQ.pgp
Description: PGP signature