[Bug 752441] gtk: Create a base class to remove code duplication

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=752441

--- Comment #11 from Nicolas Dufresne (stormer) <nicolas.dufresne at collabora.co.uk> ---
(In reply to Xavier Claessens from comment #9)
> Review of attachment 307587 [details] [review]:
> 
> ::: ext/gtk/gtkgstbasewidget.c
> @@ +280,3 @@
> +
> +  if (widget->reset)
> +    widget->reset (widget);
> 
> You don't seems to ever set a value to widget->reset()

It's NULL as per GObject initialization. It is only set for GL.

> 
> @@ +290,3 @@
> +  if (!widget->resize_id) {
> +    widget->resize_id = g_idle_add_full (G_PRIORITY_DEFAULT,
> +        (GSourceFunc) _queue_resize, widget, NULL);
> 
> You don't need _full if you set DEFAULT priority and no destroynotify.

IDLE_DEFAULT (200) is not DEFAULT (0).

(In reply to Xavier Claessens from comment #10)
> Review of attachment 307590 [details] [review]:
> 
> ::: ext/gtk/gstgtkbasesink.c
> @@ +150,3 @@
> +{
> +  if (gtk_sink->widget != NULL)
> +    return gtk_sink->widget;
> 
> I would add here an assert that we are in the main thread. Something like:
> g_assert (g_main_context_is_owner(NULL));
> But the default main context could not yet be acquired, if we are called
> from main() before we first call g_main_loop_run(), I think.
> 
> Maybe rather:
> g_assert (g_main_context_acquire(NULL));
> But then you have to release it somewhere...

I don't like this, as this would executing code in a debugging statement.

> 
> Maybe just add a comment saying it must happen in GTK thread. And at least
> tell it in the property doc.

I'll do.

> 
> @@ +249,3 @@
> +    window = gtk_window_new (GTK_WINDOW_TOPLEVEL);
> +    gtk_window_set_default_size (GTK_WINDOW (window), 640, 480);
> +    gtk_window_set_title (GTK_WINDOW (window), "Gtk+ Cairo renderer");
> 
> The title must come from subclass.

Yes, thanks.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.