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/imageviewAttachment:
pgpiycbPllVSK.pgp
Description: PGP signature