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 458460 - Presentation mode should use effects from the file
Presentation mode should use effects from the file
Status: RESOLVED FIXED
Product: evince
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Evince Maintainers
Evince Maintainers
: 568626 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2007-07-19 23:59 UTC by Bastien Nocera
Modified: 2009-11-25 12:08 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[PATCH] 2007-12-20 Carlos Garcia Campos <carlosgc@gnome.org> (18.68 KB, patch)
2007-12-31 18:28 UTC, Carlos Garnacho
rejected Details | Review
[PATCH] Implement EvDocumentTransition::get_effect() in the pdf backend. (1.77 KB, patch)
2007-12-31 18:34 UTC, Carlos Garnacho
needs-work Details | Review
[PATCH] Add EvTimeline, base object for animations. (14.12 KB, patch)
2007-12-31 18:34 UTC, Carlos Garnacho
committed Details | Review
[PATCH] Add EvTransitionAnimation, which will contain the implementations for page transition animations. (12.82 KB, patch)
2007-12-31 18:34 UTC, Carlos Garnacho
needs-work Details | Review
[PATCH] Use EvTransitionAnimation in EvView. cancel animations when the next page is requested and the animation is already running, and when leaving the presentation mode. (5.22 KB, patch)
2007-12-31 18:34 UTC, Carlos Garnacho
needs-work Details | Review
[PATCH] Implement "split" animation. (2.40 KB, patch)
2007-12-31 18:34 UTC, Carlos Garnacho
committed Details | Review
[PATCH] Implement "blinds" effect. (1.87 KB, patch)
2007-12-31 18:34 UTC, Carlos Garnacho
committed Details | Review
[PATCH] Implement "box" effect. (2.01 KB, patch)
2007-12-31 18:34 UTC, Carlos Garnacho
committed Details | Review
[PATCH] Implement "wipe" effect. (2.00 KB, patch)
2007-12-31 18:34 UTC, Carlos Garnacho
committed Details | Review
[PATCH] Implement "dissolve" effect. (1.34 KB, patch)
2007-12-31 18:34 UTC, Carlos Garnacho
committed Details | Review
[PATCH] Implement "push" effect. (1.80 KB, patch)
2007-12-31 18:34 UTC, Carlos Garnacho
committed Details | Review
[PATCH] Implement "cover" effect. (1.63 KB, patch)
2007-12-31 18:34 UTC, Carlos Garnacho
committed Details | Review
[PATCH] Implement "uncover" effect. (1.61 KB, patch)
2007-12-31 18:34 UTC, Carlos Garnacho
committed Details | Review
[PATCH] Implement "fade" effect. (1.29 KB, patch)
2007-12-31 18:34 UTC, Carlos Garnacho
committed Details | Review
[PATCH] Add EvTransitionEffect object and ev_document_transition_get_effect() to EvDocumentTransitionIface. (14.99 KB, patch)
2007-12-31 18:35 UTC, Carlos Garnacho
needs-work Details | Review
replacement for patch in comment #5 (2.29 KB, patch)
2008-01-04 00:59 UTC, Carlos Garnacho
committed Details | Review
replacement for patch in comment #7 (13.54 KB, patch)
2008-01-04 01:08 UTC, Carlos Garnacho
committed Details | Review
replacement for patch in comment #8 (6.52 KB, patch)
2008-01-04 01:15 UTC, Carlos Garnacho
committed Details | Review
Replacement for patch in comment #18 (15.76 KB, patch)
2008-01-04 01:19 UTC, Carlos Garnacho
committed Details | Review

Description Bastien Nocera 2007-07-19 23:59:18 UTC
Evince could use the effects mentioned in the PDF files, when presentations are exported to PDF.

It could also be used regardless if the user chooses a couple of effects:
- first "slide" effect
- last "slide" effect
- effects for all the other slides

It should probably use clutter as a compile-time dependency for that to get acceleration for free.
Comment 1 Carlos Garcia Campos 2007-07-20 07:28:54 UTC
Yes, it's in our roadmap for 2.20 indeed. Do we really need acceleration? is not cairo enough? 

poppler-glib has already support for page transition effects:

http://webcvs.freedesktop.org/poppler/poppler/glib/poppler-page.cc?r1=1.56&r2=1.57

And fixing this would fix #320352 too. Patches are welcome :-)
Comment 2 Alberto Ruiz 2007-07-30 13:11:44 UTC
I've added support for clutter's opt toy for pdf using poppler. Check: http://bugzilla.openedhand.com/show_bug.cgi?id=410

It still needs transition detection from the pdf file, that should be pretty straight forward, also, asynchronous loading of the pdf pixbuf to be able to cache and save video memory.
Comment 3 Carlos Garnacho 2007-12-31 18:26:23 UTC
Hi! I'm going to attach a bunch of patches to implement transition animations in evince, at the moment, it's only missing support for the "fly" and "glitter" effects, and all the implemented animations are done using cairo functions, which is quite straightforward, if some animation isn't fast enough, then other ways or techniques can be used there.
Comment 4 Carlos Garnacho 2007-12-31 18:28:51 UTC
Created attachment 101907 [details] [review]
[PATCH] 2007-12-20  Carlos Garcia Campos  <carlosgc@gnome.org>

	* configure.ac:
	* backend/ps/Makefile.am:
	* backend/ps/ev-spectre.[ch]:
	Use libspectre, if available, for the ps backend. Fixes bugs
	#317106, #499787, #501235, #421879, #445797, #443859 and #486547.


git-svn-id: svn+ssh://svn.gnome.org/svn/evince/trunk@2774 e12069bd-dc25-0410-a696-d39a8afcd844
---
 ChangeLog               |   10 +-
 backend/ps/Makefile.am  |   11 +-
 backend/ps/ev-spectre.c |  457 +++++++++++++++++++++++++++++++++++++++++++++++
 backend/ps/ev-spectre.h |   46 +++++
 configure.ac            |   60 ++++---
 5 files changed, 558 insertions(+), 26 deletions(-)
Comment 5 Carlos Garnacho 2007-12-31 18:34:06 UTC
Created attachment 101908 [details] [review]
[PATCH] Implement EvDocumentTransition::get_effect() in the pdf backend.


* backend/pdf/ev-poppler.cc: Implement get_effect(), creating a EvTransitionEffect from the returned PopplerPageTransition.
---
 backend/pdf/ev-poppler.cc |   37 +++++++++++++++++++++++++++++++++++++
 1 files changed, 37 insertions(+), 0 deletions(-)
Comment 6 Carlos Garnacho 2007-12-31 18:34:09 UTC
Created attachment 101909 [details] [review]
[PATCH] Add EvTimeline, base object for animations.

 shell/Makefile.am   |    2 +
 shell/ev-timeline.c |  446 +++++++++++++++++++++++++++++++++++++++++++++++++++
 shell/ev-timeline.h |   86 ++++++++++
 3 files changed, 534 insertions(+), 0 deletions(-)
Comment 7 Carlos Garnacho 2007-12-31 18:34:13 UTC
Created attachment 101910 [details] [review]
[PATCH] Add EvTransitionAnimation, which will contain the implementations for page transition animations.


* shell/ev-transition-animations.[ch]: Added.
* shell/Makefile.am: Add these files.
---
 shell/Makefile.am               |    2 +
 shell/ev-transition-animation.c |  325 +++++++++++++++++++++++++++++++++++++++
 shell/ev-transition-animation.h |   66 ++++++++
 3 files changed, 393 insertions(+), 0 deletions(-)
Comment 8 Carlos Garnacho 2007-12-31 18:34:17 UTC
Created attachment 101911 [details] [review]
[PATCH] Use EvTransitionAnimation in EvView. cancel animations when the next page is requested and the animation is already running, and when leaving the presentation mode.

 shell/ev-view-private.h |    3 +
 shell/ev-view.c         |  120 +++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 108 insertions(+), 15 deletions(-)
Comment 9 Carlos Garnacho 2007-12-31 18:34:20 UTC
Created attachment 101912 [details] [review]
[PATCH] Implement "split" animation.

 shell/ev-transition-animation.c |   68 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 68 insertions(+), 0 deletions(-)
Comment 10 Carlos Garnacho 2007-12-31 18:34:24 UTC
Created attachment 101913 [details] [review]
[PATCH] Implement "blinds" effect.

 shell/ev-transition-animation.c |   47 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 47 insertions(+), 0 deletions(-)
Comment 11 Carlos Garnacho 2007-12-31 18:34:27 UTC
Created attachment 101914 [details] [review]
[PATCH] Implement "box" effect.

 shell/ev-transition-animation.c |   47 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 47 insertions(+), 0 deletions(-)
Comment 12 Carlos Garnacho 2007-12-31 18:34:31 UTC
Created attachment 101915 [details] [review]
[PATCH] Implement "wipe" effect.

 shell/ev-transition-animation.c |   57 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 57 insertions(+), 0 deletions(-)
Comment 13 Carlos Garnacho 2007-12-31 18:34:34 UTC
Created attachment 101916 [details] [review]
[PATCH] Implement "dissolve" effect.

 shell/ev-transition-animation.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)
Comment 14 Carlos Garnacho 2007-12-31 18:34:37 UTC
Created attachment 101917 [details] [review]
[PATCH] Implement "push" effect.

 shell/ev-transition-animation.c |   33 +++++++++++++++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)
Comment 15 Carlos Garnacho 2007-12-31 18:34:41 UTC
Created attachment 101918 [details] [review]
[PATCH] Implement "cover" effect.

 shell/ev-transition-animation.c |   33 +++++++++++++++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)
Comment 16 Carlos Garnacho 2007-12-31 18:34:45 UTC
Created attachment 101919 [details] [review]
[PATCH] Implement "uncover" effect.

 shell/ev-transition-animation.c |   33 +++++++++++++++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)
Comment 17 Carlos Garnacho 2007-12-31 18:34:48 UTC
Created attachment 101920 [details] [review]
[PATCH] Implement "fade" effect.

 shell/ev-transition-animation.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)
Comment 18 Carlos Garnacho 2007-12-31 18:35:15 UTC
Created attachment 101921 [details] [review]
[PATCH] Add EvTransitionEffect object and ev_document_transition_get_effect() to EvDocumentTransitionIface.


* libdocument/ev-transition-effect.[ch]: Added, at the moment it mostly maps PopplerPageTransition to a GObject.
* libdocument/Makefile.am: added these files.
* libdocument/ev-document-transition.[ch]: Add get_effect() vmethod to the interface, so a EvDocumentTransitionIface implementation can return a EvTransitionEffect describing the effect.
---
 libdocument/Makefile.am              |    4 +-
 libdocument/ev-document-transition.c |   10 ++
 libdocument/ev-document-transition.h |   16 ++-
 libdocument/ev-transition-effect.c   |  285 ++++++++++++++++++++++++++++++++++
 libdocument/ev-transition-effect.h   |   89 +++++++++++
 5 files changed, 397 insertions(+), 7 deletions(-)
Comment 19 Carlos Garnacho 2007-12-31 18:37:13 UTC
attachment 101907 [details] [review] was obviously bogus, and patch in comment #18 is the first patch to apply, sorry about that...
Comment 20 Carlos Garcia Campos 2007-12-31 18:38:12 UTC
Comment on attachment 101908 [details] [review]
[PATCH] Implement EvDocumentTransition::get_effect() in the pdf backend.

Hey Carlos!, great to see progress on this :-)) Some minor comments below. 

>b0cb4bcf11a95cf0479401909c092d5b5eb6ff8f
>diff --git a/backend/pdf/ev-poppler.cc b/backend/pdf/ev-poppler.cc
>index 5b03980..174c9b5 100644
>--- a/backend/pdf/ev-poppler.cc
>+++ b/backend/pdf/ev-poppler.cc
>@@ -45,6 +45,7 @@
> #include "ev-document-transition.h"
> #include "ev-document-forms.h"
> #include "ev-selection.h"
>+#include "ev-transition-effect.h"
> #include "ev-attachment.h"
> #include "ev-image.h"
> 
>@@ -1967,10 +1968,46 @@ pdf_document_get_page_duration (EvDocumentTransition *trans,
> 	return duration;
> }
> 
>+static EvTransitionEffect *
>+pdf_document_get_effect (EvDocumentTransition *trans,
>+			 gint                  page)
>+{
>+	PdfDocument            *pdf_document;
>+	PopplerPage            *poppler_page;
>+	PopplerPageTransition  *page_transition;
>+	EvTransitionEffect     *effect;
>+
>+	pdf_document = PDF_DOCUMENT (trans);
>+	poppler_page = poppler_document_get_page (pdf_document->document, page);
>+
>+	if (!poppler_page)
>+		return NULL;
>+
>+	page_transition = poppler_page_get_transition (poppler_page);
>+
>+	if (!page_transition)

poppler_page should be unrefed here

>+		return NULL;
>+
>+	/* enums in PopplerPageTransition match the EvTransitionEffect ones */
>+	effect = ev_transition_effect_new ((EvTransitionEffectType) page_transition->type,
>+					   "alignment", page_transition->alignment,
>+					   "direction", page_transition->direction,
>+					   "duration", page_transition->duration,
>+					   "angle", page_transition->angle,
>+					   "scale", page_transition->scale,
>+					   "rectangular", page_transition->rectangular,
>+					   NULL);
>+
>+	poppler_page_transition_free (page_transition);

and here too

>+	return effect;
>+}
>+
> static void
> pdf_document_page_transition_iface_init (EvDocumentTransitionIface *iface)
> {
> 	iface->get_page_duration = pdf_document_get_page_duration;
>+	iface->get_effect = pdf_document_get_effect;
> }
> 
> /* Forms */
Comment 21 Carlos Garcia Campos 2008-01-02 17:01:15 UTC
Comment on attachment 101910 [details] [review]
[PATCH] Add EvTransitionAnimation, which will contain the implementations for page transition animations.

Comments below

>+void
>+ev_transition_animation_paint (EvTransitionAnimation *animation,
>+			       cairo_t               *cr,
>+			       GdkRectangle           page_area)
>+{
>+	EvTransitionAnimationPriv *priv;
>+	EvTransitionEffectType type;
>+	gdouble progress;
>+
>+	g_return_if_fail (EV_IS_TRANSITION_ANIMATION (animation));
>+
>+	priv = EV_TRANSITION_ANIMATION_GET_PRIVATE (animation);
>+	g_object_get (priv->effect, "type", &type, NULL);
>+	progress = ev_timeline_get_progress (EV_TIMELINE (animation));
>+
>+	priv->origin_surface = ev_pixbuf_cache_get_surface (priv->pixbuf_cache, priv->page_from);
>+	priv->dest_surface = ev_pixbuf_cache_get_surface (priv->pixbuf_cache, priv->page_to);
>+
>+	if (!priv->origin_surface || !priv->dest_surface)
>+		return;

when current page is not still rendered we should paint the previous page until the current one is available. This avoids the famous flickering effect. (see bug #320352)

>+	switch (type) {
>+	case EV_TRANSITION_EFFECT_REPLACE:
>+		/* just paint the destination slide */
>+		paint_surface (cr, priv->dest_surface, 0, 0, 0, page_area);
>+		break;
>+	default:
>+		g_warning ("Transition animation not implemented");

add an option for every unsupported type specifying in the warning message which effect is and how to report a bug like we do for unsupported named actions:

g_warning ("Unimplemented named action: %s, please post a "                                                                                  
                           "bug report in Evince bugzilla "                                                                                                  
                           "(http://bugzilla.gnome.org) with a testcase.",                                                                                   
                           name);

In addition, for unsupported types, we could fallback to replace effect which is the default one.
Comment 22 Carlos Garcia Campos 2008-01-02 17:04:28 UTC
Comment on attachment 101911 [details] [review]
[PATCH] Use EvTransitionAnimation in EvView. cancel animations when the next page is requested and the animation is already running, and when leaving the presentation mode.


>+static void
>+ev_view_presentation_animation_start (EvView *view,
>+                                      int     new_page)
>+{
>+	EvTransitionEffect *effect = NULL;
>+
>+        if (view->presentation &&
>+	    EV_IS_DOCUMENT_TRANSITION (view->document))
>+		effect = ev_document_transition_get_effect (EV_DOCUMENT_TRANSITION (view->document),
>+							    view->current_page);

we already check whether view is in presentation mode when calling animation_start so we can avoid checking it here again.
Comment 23 Carlos Garcia Campos 2008-01-02 17:07:40 UTC
Comment on attachment 101921 [details] [review]
[PATCH] Add EvTransitionEffect object and ev_document_transition_get_effect() to EvDocumentTransitionIface.


>+GType
>+ev_transition_effect_type_get_type (void)
>+{
>+        static GType type = 0;
>+
>+        if (G_UNLIKELY (type == 0)) {
>+                static const GEnumValue values[] = {

no need for 'values' to be static. It's better to remove it so that we avoid relocations. The same for the other get_type() functions.
Comment 24 Carlos Garcia Campos 2008-01-02 17:10:24 UTC
The other patches look good to me. Marking them as reviewed. Thank you very much for your great work!
Comment 25 Carlos Garnacho 2008-01-04 00:59:42 UTC
Created attachment 102079 [details] [review]
replacement for patch in comment #5

Unrefs the poppler page as suggested in comment #20
Comment 26 Carlos Garnacho 2008-01-04 01:08:41 UTC
Created attachment 102081 [details] [review]
replacement for patch in comment #7

EvTransitionAnimation has suffered a few changes, it's now just aware of the two cairo surfaces and the animation, it has these 2 functions:

ev_transition_animation_set_origin_surface ()
ev_transition_animation_set_dest_surface ()

the animation will only start when the two surfaces are available, and the former is shown if the latter is missing.
Comment 27 Carlos Garnacho 2008-01-04 01:10:01 UTC
Bleh, forgot to mention it also implements what's suggested in comment #21
Comment 28 Carlos Garnacho 2008-01-04 01:15:38 UTC
Created attachment 102082 [details] [review]
replacement for patch in comment #8

This patch does as suggested in comment #22, also uses differently the EvTransitionAnimation, ensuring the animation starts when both surfaces are loaded. Oh, it also removes the workaround added for bug #320352 :)
Comment 29 Carlos Garnacho 2008-01-04 01:19:55 UTC
Created attachment 102083 [details] [review]
Replacement for patch in comment #18

Do not declare GEnumValues arrays to be static, as suggested in comment #23
Comment 30 Carlos Garcia Campos 2008-01-04 10:38:17 UTC
Great! thanks again, it looks awesome. Please commit it. Do not forget to close also bug #320352 and mark task as done in our RoadMap (http://live.gnome.org/Evince/Roadmap) :-)
Comment 31 Carlos Garnacho 2008-01-04 20:36:20 UTC
Thanks! I've just committed all the patches, please tell me if you find any problem in this code, and I'll also try to provide implementations for "fly" and "glitter" if get to see I how do they look, but now that's just an improvement :)
Comment 32 Carlos Garcia Campos 2009-11-25 12:08:25 UTC
*** Bug 568626 has been marked as a duplicate of this bug. ***