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 656268 - Move video preview
Move video preview
Status: RESOLVED FIXED
Product: empathy
Classification: Core
Component: VoIP
3.1.x
Other Linux
: Normal normal
: ---
Assigned To: empathy-maint
Depends on:
Blocks: 629902
 
 
Reported: 2011-08-10 10:52 UTC by Emilio Pozuelo Monfort
Modified: 2011-08-15 12:38 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
empathy-call: don't run g_option_context_parse (1.39 KB, patch)
2011-08-10 10:57 UTC, Emilio Pozuelo Monfort
none Details | Review
CallWindow: add top and right margin to the preview container (1.30 KB, patch)
2011-08-10 10:57 UTC, Emilio Pozuelo Monfort
none Details | Review
CallWindow: allow to move the video preview (5.39 KB, patch)
2011-08-10 10:58 UTC, Emilio Pozuelo Monfort
none Details | Review
CallWindow: show drop zones when dragging the preview (4.72 KB, patch)
2011-08-10 10:58 UTC, Emilio Pozuelo Monfort
none Details | Review
Factor out empathy_call_window_get_preview_position() (1.68 KB, patch)
2011-08-10 10:58 UTC, Emilio Pozuelo Monfort
none Details | Review
CallWindow: highlight drop zones when hovered (4.44 KB, patch)
2011-08-10 10:58 UTC, Emilio Pozuelo Monfort
none Details | Review
Move empathy_call_window_get_preview_position around (4.12 KB, patch)
2011-08-10 10:58 UTC, Emilio Pozuelo Monfort
none Details | Review
Factor out empathy_call_window_get_preview_rectangle (2.30 KB, patch)
2011-08-10 10:58 UTC, Emilio Pozuelo Monfort
none Details | Review
Factor out empathy_call_window_darken_preview_rectangle (2.40 KB, patch)
2011-08-10 10:58 UTC, Emilio Pozuelo Monfort
none Details | Review
Highlight the preview when hovered (2.62 KB, patch)
2011-08-10 10:58 UTC, Emilio Pozuelo Monfort
none Details | Review
Don't darken the preview when dragging it (2.66 KB, patch)
2011-08-10 10:58 UTC, Emilio Pozuelo Monfort
none Details | Review
Darken the rectangles when starting a drag operation (959 bytes, patch)
2011-08-10 10:58 UTC, Emilio Pozuelo Monfort
none Details | Review
Use self->priv->preview_pos to determine the preview position (1.03 KB, patch)
2011-08-10 10:58 UTC, Emilio Pozuelo Monfort
none Details | Review

Description Emilio Pozuelo Monfort 2011-08-10 10:52:32 UTC
We should be able to move the video preview by dragging it.
Comment 2 Emilio Pozuelo Monfort 2011-08-10 10:57:55 UTC
Created attachment 193528 [details] [review]
empathy-call: don't run g_option_context_parse

It's not that important and it's breaking clutter events (!).
Comment 3 Emilio Pozuelo Monfort 2011-08-10 10:57:59 UTC
Created attachment 193529 [details] [review]
CallWindow: add top and right margin to the preview container

So that when it's placed on another corner, it still has some
margin.
Comment 4 Emilio Pozuelo Monfort 2011-08-10 10:58:04 UTC
Created attachment 193530 [details] [review]
CallWindow: allow to move the video preview
Comment 5 Emilio Pozuelo Monfort 2011-08-10 10:58:10 UTC
Created attachment 193531 [details] [review]
CallWindow: show drop zones when dragging the preview
Comment 6 Emilio Pozuelo Monfort 2011-08-10 10:58:17 UTC
Created attachment 193532 [details] [review]
Factor out empathy_call_window_get_preview_position()
Comment 7 Emilio Pozuelo Monfort 2011-08-10 10:58:21 UTC
Created attachment 193533 [details] [review]
CallWindow: highlight drop zones when hovered
Comment 8 Emilio Pozuelo Monfort 2011-08-10 10:58:26 UTC
Created attachment 193534 [details] [review]
Move empathy_call_window_get_preview_position around
Comment 9 Emilio Pozuelo Monfort 2011-08-10 10:58:30 UTC
Created attachment 193535 [details] [review]
Factor out empathy_call_window_get_preview_rectangle
Comment 10 Emilio Pozuelo Monfort 2011-08-10 10:58:35 UTC
Created attachment 193536 [details] [review]
Factor out empathy_call_window_darken_preview_rectangle
Comment 11 Emilio Pozuelo Monfort 2011-08-10 10:58:40 UTC
Created attachment 193537 [details] [review]
Highlight the preview when hovered
Comment 12 Emilio Pozuelo Monfort 2011-08-10 10:58:45 UTC
Created attachment 193538 [details] [review]
Don't darken the preview when dragging it
Comment 13 Emilio Pozuelo Monfort 2011-08-10 10:58:49 UTC
Created attachment 193539 [details] [review]
Darken the rectangles when starting a drag operation

So that if we have just dragged it, the rectangle where
it was before isn't highlighted.
Comment 14 Emilio Pozuelo Monfort 2011-08-10 10:58:54 UTC
Created attachment 193540 [details] [review]
Use self->priv->preview_pos to determine the preview position
Comment 15 Danielle Madeley 2011-08-12 00:01:18 UTC
Review of attachment 193528 [details] [review]:

I'm pretty skeptical of this. Are you calling clutter_init() somewhere?
Comment 16 Danielle Madeley 2011-08-12 00:02:35 UTC
Review of attachment 193529 [details] [review]:

::: src/empathy-call-window.c
@@ +540,3 @@
   priv->video_preview = clutter_box_new (layout);
   clutter_actor_set_size (priv->video_preview,
+      SELF_VIDEO_SECTION_WIDTH + 20, SELF_VIDEO_SECTION_HEIGTH + 20);

Magic numbers in code are bad. Move this value into a #define so we know what it is.
Comment 17 Danielle Madeley 2011-08-12 00:07:20 UTC
Review of attachment 193531 [details] [review]:

::: src/empathy-call-window.c
@@ +525,3 @@
 static void
+empathy_call_window_create_preview_rectangle (EmpathyCallWindow *self,
+    ClutterActor **rectangle,

This is really strange... why not return ClutterActor * ?

@@ +560,3 @@
+static void
+empathy_call_window_create_preview_rectangles (EmpathyCallWindow *self)
+      1);

Would it be worth packing these rectangles into a group. Then you only need to hold a pointer to the group, you only need to set the group visible, you only need to destroy the group, etc.
Comment 18 Emilio Pozuelo Monfort 2011-08-12 16:00:14 UTC
(In reply to comment #15)
> Review of attachment 193528 [details] [review]:
> 
> I'm pretty skeptical of this. Are you calling clutter_init() somewhere?

gtk_clutter_init() calls it (and its documentation says you shouldn't call clutter_init directly if you call gtk_clutter_init). I've found the real problem now though, so I'm not doing that hack anymore but calling gdk_disable_multidevice(), which is the right thing to do. See the new commit message.

(In reply to comment #16)
> Review of attachment 193529 [details] [review]:
> 
> ::: src/empathy-call-window.c
> @@ +540,3 @@
>    priv->video_preview = clutter_box_new (layout);
>    clutter_actor_set_size (priv->video_preview,
> +      SELF_VIDEO_SECTION_WIDTH + 20, SELF_VIDEO_SECTION_HEIGTH + 20);
> 
> Magic numbers in code are bad. Move this value into a #define so we know what
> it is.

Done.

(In reply to comment #17)
> Review of attachment 193531 [details] [review]:
> 
> ::: src/empathy-call-window.c
> @@ +525,3 @@
>  static void
> +empathy_call_window_create_preview_rectangle (EmpathyCallWindow *self,
> +    ClutterActor **rectangle,
> 
> This is really strange... why not return ClutterActor * ?

Huh, I wonder why I didn't do that! Changed (amended).

> 
> @@ +560,3 @@
> +static void
> +empathy_call_window_create_preview_rectangles (EmpathyCallWindow *self)
> +      1);
> 
> Would it be worth packing these rectangles into a group. Then you only need to
> hold a pointer to the group, you only need to set the group visible, you only
> need to destroy the group, etc.

We still need pointers to each rectangle as we want to highlight one of them when dragging the preview. I'm also not sure about that, as I think it would receive events if it was on top of other actors, and we don't want that for e.g. the preview button or the floating toolbar.

Branch updated.
Comment 19 Guillaume Desmottes 2011-08-15 11:14:40 UTC
++
Comment 20 Emilio Pozuelo Monfort 2011-08-15 12:38:34 UTC
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.