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 736370 - Reload just the page when changing the annotation properties
Reload just the page when changing the annotation properties
Status: RESOLVED OBSOLETE
Product: evince
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Evince Maintainers
Evince Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-09-10 01:33 UTC by José Aliste
Modified: 2018-05-22 15:51 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
libview: make ev_view_reload_page public (1.86 KB, patch)
2014-09-10 01:41 UTC, José Aliste
none Details | Review
shell: Reload only the page of the annotation when changing its properties (890 bytes, patch)
2014-09-10 01:41 UTC, José Aliste
none Details | Review

Description José Aliste 2014-09-10 01:33:21 UTC
Currently when updating the properties of an annotation we invalidate the complete pixbuf cacache with a call to ev_view_reload(). This is super slow on large documents... I know in the past we rejected making ev_view_reload_page() public but for this, I don't see any other proper solution. So I am attaching two patches. One that makes ev_view_reload_page public and the other which uses this function to reload just the page of the annotation instead of everything.
Comment 1 José Aliste 2014-09-10 01:41:06 UTC
Created attachment 285783 [details] [review]
libview: make ev_view_reload_page public
Comment 2 José Aliste 2014-09-10 01:41:11 UTC
Created attachment 285784 [details] [review]
shell: Reload only the page of the annotation when changing its properties
Comment 3 Carlos Garcia Campos 2014-09-14 08:56:07 UTC
(In reply to comment #0)
> Currently when updating the properties of an annotation we invalidate the
> complete pixbuf cacache with a call to ev_view_reload(). This is super slow on
> large documents... I know in the past we rejected making ev_view_reload_page()
> public but for this, I don't see any other proper solution. So I am attaching
> two patches. One that makes ev_view_reload_page public and the other which uses
> this function to reload just the page of the annotation instead of everything.

ev_view_reload_page() has a region argument that it's easier to handle inside the view, since you need to convert coordinates and take scroll into account. How would you solve the FIXME: update annot region only from the window? So, since the annotations are created and removed by the view, we could also make the view update them, with something like ev_view_update_annotation() that receives the annotation and save mask and calls ev_document_annotations_save_annotation(). The view can check whether the annotation is visible or not, when not visible we invalidate the annots in the page cache, and when visible we update the page and schedule a redraw of the annotation area.
Comment 4 Carlos Garcia Campos 2014-09-14 08:57:15 UTC
Maybe ev_view_save_annotation() is a more consistent name, but you get the point.
Comment 5 José Aliste 2014-09-15 16:21:30 UTC
My impression is that the FIXME was assuming that the rendering is slow on the cairo part, while it's slow on the poppler core part. I see us having a problem with reloading only the region of the annotation when we really have partial rendering support...or tile rendering...So I'd revisit this at the moment. My impression is that it does not help a lot... 

That being said.  I understand and agree with your point about ev_view_reload. It's not simple to make it public API. While I understand the solution of adding a ev_view_save_annotation(), it looks a bit akward to me to call a a libview function to save the annotation...maybe a better option would be to emit a changed signal or somthing on the annotation or the page cache or whatever and then let the ev-view update the view itself?
Comment 6 Carlos Garcia Campos 2014-09-16 12:52:56 UTC
(In reply to comment #5)
> My impression is that the FIXME was assuming that the rendering is slow on the
> cairo part, while it's slow on the poppler core part.

No, repainting only the damage area is a just a micro optimization. I was not suggesting to do that to fix this problem, but just as a nice addition.

> I see us having a problem
> with reloading only the region of the annotation when we really have partial
> rendering support...or tile rendering...So I'd revisit this at the moment. My
> impression is that it does not help a lot... 
> 
> That being said.  I understand and agree with your point about ev_view_reload.
> It's not simple to make it public API. While I understand the solution of
> adding a ev_view_save_annotation(), it looks a bit akward to me to call a a
> libview function to save the annotation...maybe a better option would be to
> emit a changed signal or somthing on the annotation or the page cache or
> whatever and then let the ev-view update the view itself?

I don't see what's wrong with ev_view_save_annotation() considering that we already have being_add_annotation() and remove_annotation() in EvView.
Comment 7 GNOME Infrastructure Team 2018-05-22 15:51:54 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/evince/issues/509.