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 694388 - gd-stack: Allow turning off transitions
gd-stack: Allow turning off transitions
Status: RESOLVED FIXED
Product: libgd
Classification: Platform
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: libgd maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2013-02-21 19:58 UTC by Jasper St. Pierre (not reading bugmail)
Modified: 2013-02-26 10:48 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gd-stack: Rename duration to transition-duration (4.65 KB, patch)
2013-02-21 19:58 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review
gd-stack: Allow turning off transitions (5.87 KB, patch)
2013-02-21 19:58 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review

Description Jasper St. Pierre (not reading bugmail) 2013-02-21 19:58:16 UTC
This is listed in the TODO, so I figured I might do it here.
Comment 1 Jasper St. Pierre (not reading bugmail) 2013-02-21 19:58:18 UTC
Created attachment 237102 [details] [review]
gd-stack: Rename duration to transition-duration

This makes it clear what the transition is for.
Comment 2 Jasper St. Pierre (not reading bugmail) 2013-02-21 19:58:20 UTC
Created attachment 237103 [details] [review]
gd-stack: Allow turning off transitions
Comment 3 Cosimo Cecchi 2013-02-21 23:38:24 UTC
Review of attachment 237102 [details] [review]:

Looks good to me, and it's better to do this now than later that more apps are using it.
I'd like the similar property in GdRevealer to be changed too though, can you make a patch for that?

::: libgd/gd-stack.c
@@ +718,2 @@
 void
+gd_stack_set_transition_duration (GdStack *stack,

Please keep indentation here.
Comment 4 Cosimo Cecchi 2013-02-21 23:38:31 UTC
Review of attachment 237102 [details] [review]:

Looks good to me, and it's better to do this now than later that more apps are using it.
I'd like the similar property in GdRevealer to be changed too though, can you make a patch for that?

::: libgd/gd-stack.c
@@ +718,2 @@
 void
+gd_stack_set_transition_duration (GdStack *stack,

Please keep indentation here.
Comment 5 Cosimo Cecchi 2013-02-21 23:42:32 UTC
Review of attachment 237103 [details] [review]:

Thanks, I like this idea in general.

::: libgd/gd-stack.c
@@ +280,3 @@
+  g_object_class_install_property (object_class,
+                                   PROP_TRANSITION_TYPE,
+                                   g_param_spec_int ("transition-type",

Should use glib-mkenums and have this be an enum property instead.

::: libgd/gd-stack.h
@@ +43,3 @@
+  GD_STACK_TRANSITION_CROSSFADE,
+  GD_STACK_TRANSITION_MAX,
+} GdStackTransition;

I like GdStackTransitionType better for this. Also, do we really need the MAX value?
Comment 6 Alexander Larsson 2013-02-22 07:28:00 UTC
Review of attachment 237103 [details] [review]:

Yeah, this was what i expected to do too. I need it so i can allow left/right slide transitons.

::: libgd/gd-stack.c
@@ +183,3 @@
       break;
+    case PROP_TRANSITION_TYPE:
+      g_value_set_int (value, gd_stack_get_transition_type (stack));

g_value_set_enum s more pedantically right.
Comment 7 Jasper St. Pierre (not reading bugmail) 2013-02-25 18:15:35 UTC
(In reply to comment #5)
> Review of attachment 237103 [details] [review]:
> 
> Thanks, I like this idea in general.
> 
> ::: libgd/gd-stack.c
> @@ +280,3 @@
> +  g_object_class_install_property (object_class,
> +                                   PROP_TRANSITION_TYPE,
> +                                   g_param_spec_int ("transition-type",
> 
> Should use glib-mkenums and have this be an enum property instead.

I intentionally didn't do that because you chose the same thing (int pspec) with GdMainView::view-type. If you want to fix that and introduce glib-mkenums infrastructure, go for it I guess.
Comment 8 Alexander Larsson 2013-02-26 10:46:46 UTC
Attachment 237103 [details] pushed as 0d817b0 - gd-stack: Allow turning off transitions
Comment 9 Alexander Larsson 2013-02-26 10:48:33 UTC
Pushed this with some fixups, including transition-duration change for GdRevealer.

The one thing i'm unsure of is the default value for transition-type. Now it is NONE, but maybe crossfade is a better choice?