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 773642 - From gtkmm3 to gtkmm4
From gtkmm3 to gtkmm4
Status: RESOLVED OBSOLETE
Product: gtkmm
Classification: Bindings
Component: general
3.91.x
Other All
: Normal enhancement
: ---
Assigned To: gtkmm-forge
gtkmm-forge
Depends on:
Blocks:
 
 
Reported: 2016-10-28 17:06 UTC by Kjell Ahlstedt
Modified: 2018-05-22 12:15 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch: Gtk::Widget: Temporary workaround for bug 774778 (get_render_mode()) (1.67 KB, patch)
2016-11-24 08:25 UTC, Kjell Ahlstedt
committed Details | Review
patch: Gtk::Object::_release_c_instance(): Don't release if in a container (4.10 KB, patch)
2018-02-12 09:06 UTC, Kjell Ahlstedt
committed Details | Review

Description Kjell Ahlstedt 2016-10-28 17:06:10 UTC
This bug report is intended to be a place for discussing the upgrade from
gtkmm 3 to gtkmm 4. Gtk+ has announced a new versioning scheme:
https://blog.gtk.org/2016/09/01/versioning-and-long-term-stability-promise-in-gtk

To be compatible with future gtk+ versions and branches, gtkmm will have to
stick to the same, or a very similar, versioning scheme and branching scheme.
This has been discussed on the gtkmm-list. A bug report is a good place to
continue the discussion.
Comment 1 Kjell Ahlstedt 2016-10-28 17:09:12 UTC
Two issues to begin with.

1. Many functions have been deprecated in the gtk-3-22 branch. Usually that's
not allowed in a stable branch, is it? Still, I suppose we should deprecate the
corresponding parts in the gtkmm-3-22 branch. Right now it's not possible to
build the gtkmm-3-22 branch (or gtkmm's master branch, which is now identical to
gtkmm-3-22) against gtk+'s gtk-3-22 branch (gtk+-3).

2. Gtk+ has 3 entries in jhbuild's moduleset file
https://git.gnome.org/browse/jhbuild/tree/modulesets/gnome-suites-core-deps-3.24.modules
  gtk+, <branch/>, i.e. master branch, checkoutdir gtk+
  gtk+-3, <branch checkoutdir="gtk+-3" module="gtk+" revision="gtk-3-22"/>
  gtk+-2, <branch checkoutdir="gtk+-2" module="gtk+" revision="gtk-2-24"/>

We should have similar entries for gtkmm (except for the obsolete gtkmm2):
  gtkmm, <branch/>, i.e. master branch, checkoutdir gtkmm
  gtkmm-3, <branch checkoutdir="gtkmm-3" module="gtkmm" revision="gtkmm-3-22"/>

gtkmm-3 should depend on gtk+-3, gtkmm should depend on gtk+.

Who may modify the moduleset files? Can e.g. a gtkmm developer modify the
entries for gtkmm, or does it require a jhbuild bug and a jhbuild developer
accepting a suggested modification?
Comment 2 Kjell Ahlstedt 2016-10-31 15:26:24 UTC
In the gtkmm-3-22 branch I have deprecated API which has been deprecated in
gtk+'s gtk-3-22 branch. Hope that's alright. If not, it can be reverted.
https://git.gnome.org/browse/gtkmm/commit/?h=gtkmm-3-22&id=5247fc640a63920cabda1056b01530d30de04ce7
Comment 3 Kjell Ahlstedt 2016-11-03 15:51:50 UTC
I have pushed some commits to the master branch. Those commits remove a lot of
old API, which has been removed in gtk+4. The code at the head of gtkmm's master
branch right now can be built against gtk+'s master branch from 2016-10-28. The
latest gtk+ code contains a bug which makes it impossible to use it for building
gtkmm. See bug 773903. Hopefully it will be fixed soon. Then I will continue
fixing gtkmm4. It remains to fix 'make check', and to try to keep up with the
ongoing changes in gtk+4.
Comment 4 Kjell Ahlstedt 2016-11-09 17:32:32 UTC
Now gtkmm's master branch is compatible with gtk+'s master branch.
Even 'make check' works.

And I see that gnome-suites-core-deps-3.24.modules has been updated like I
suggested in comment 1, item 2.
https://git.gnome.org/browse/jhbuild/commit/?id=cedd05f772c778061c44507d759c1828c2988571
https://git.gnome.org/browse/jhbuild/commit/?id=5784ace909b84879dfffacf11bdb7cd2c63bebfa

Perhaps a similar change should be made for gtkmm-documentation in
gnome-world.modules? gtkmm-3-documentation and gtkmm-documentation?
Isn't it also time for a new release of gtkmm-documentation? A 3.22 release and
a gtkmm-3-22 branch? Then we could start making the master branch compatible
with gtkmm's master branch, when someone has time to do that.

An appropriate fix if gtkmm-documentation is changed like that:
  - Create branch gtkmm-3-22.
  - In index-in.docbook in branch gtkmm-3-22, change
      <!ENTITY url_examples_branchsuffix "master">
    to
      <!ENTITY url_examples_branchsuffix "gtkmm-3-22">
  - Make the new release from branch gtkmm-3-22 after the usual changes of
    configure.ac and NEWS.

So far I have mostly removed a lot of code and fixed some simple TODO comments
that waited for an ABI break. It remains to add code, and fix many more TODO
comments.
Comment 5 Murray Cumming 2016-11-10 11:16:42 UTC
Agreed. Many thanks.
Comment 6 Kjell Ahlstedt 2016-11-13 15:21:02 UTC
Who shall make the gtkmm-documentation 3.22.0 release? I can do it, if you like.

On second thought I don't think it's that important to have a
gtkmm-3-documentation entry in jhbuild's modules files.
Comment 7 Murray Cumming 2016-11-14 10:00:37 UTC
Please feel free to make a release. Don't let me get in the way, please. Thanks again.
Comment 8 Kjell Ahlstedt 2016-11-14 15:51:22 UTC
I have released gtkmm-documentation 3.22.

Download: https://download.gnome.org/sources/gtkmm-documentation/3.22/
Documentation: https://developer.gnome.org/gtkmm-tutorial/3.22/
Comment 9 Kjell Ahlstedt 2016-11-20 15:04:49 UTC
Very few of the gtkmm applications are drawn correctly right now. The gtk+
commit to blame is https://git.gnome.org/browse/gtk+/commit/?id=2cd9e5170e0da6b9c8d92fab21c7b8541edb2738
"widget: Add more sophisticated detection of rendering method"

That commit is not compatible with the way gtkmm overrides vfuncs and default
signal handlers. I don't yet know how to fix it. I doubt that it can be fixed
in gtkmm alone. We probably need a fix in gtk+.
Comment 10 Murray Cumming 2016-11-21 07:37:09 UTC
Could you file a bug for GTK+ about it, please?
Comment 11 Kjell Ahlstedt 2016-11-21 10:04:27 UTC
Gtk+ bug 774778
Comment 12 Kjell Ahlstedt 2016-11-24 08:25:05 UTC
Created attachment 340667 [details] [review]
patch: Gtk::Widget: Temporary workaround for bug 774778 (get_render_mode())

This patch can be used temporarily until we have a solution for the present
drawing problem. With the patch, all drawing is left to gtk+, meaning that
standard gtkmm widgets are drawn correctly, also DrawingArea, if the new draw
function is used instead of the old draw signal.

I don't recommend pushing this patch, but it can be applied locally.
It hides a real problem instead of solving it.
Comment 13 Murray Cumming 2016-11-25 09:40:49 UTC
Wouldn't it be best to push this to master to make gtkmm4 useful in general?
Comment 14 Kjell Ahlstedt 2016-11-25 16:00:52 UTC
Do you think so? The drawback is that it hides the problem. Custom widgets won't
be drawn (or is it rendered?). signal_draw() works only for widgets that derive
from a gtk+ class that uses the draw signal in the gtk+ code, and those widgets
are quickly becoming rare, it seems. Drawing is now done with GtkSnapshot and a
bunch of Gsk classes. I haven't looked into these new classes. I guess there are
many new classes that should be wrapped in gtkmm.

If you think it's better to push the patch than not do it, I can push it.
Or you can do it yourself. We'd better remember that we need a real solution
before gtkmm4's ABI is frozen.
Comment 15 Murray Cumming 2016-11-26 12:02:07 UTC
(In reply to Kjell Ahlstedt from comment #14)
> Do you think so? The drawback is that it hides the problem. Custom widgets
> won't
> be drawn (or is it rendered?).
[snip]

It won't hide that problem, right? Custom widgets will still be be broken.

I don't think breaking every other widget helps us. At this point nobody is using gtkmm 4 anyway, and this wouldn't encourage them to start using it.

> We'd better remember that we need a real solution before gtkmm4's ABI is frozen.

Yes, as long as we leave a TODO there we should catch it.

Thanks.
Comment 16 Kjell Ahlstedt 2016-11-27 12:55:41 UTC
Comment on attachment 340667 [details] [review]
patch: Gtk::Widget: Temporary workaround for bug 774778 (get_render_mode())

I have pushed the patch.
Comment 17 Kjell Ahlstedt 2016-11-29 16:09:41 UTC
Bug 775348 shows a solution for the drawing problem without modification in
gtk+. Unfortunately quite complicated.
Comment 18 Kjell Ahlstedt 2017-01-02 14:59:04 UTC
Snapshot questions:

I suppose that we shall wrap the new GtkSnapshot struct and all public Gsk*
classes. These classes use the Graphene library, which has not been used by
gtk+3. Is there a graphenemm C++ binding? Will there be one? Or shall we use
graphene's data types, such as graphene_rect_t and graphene_matrix_t, in gtkmm's
C++ API?

I'm uncertain how to wrap GtkSnapshot. It's an opaque structure without public
new, copy, free, ref, unref functions. The documentation says
 * The only way to obtain a #GtkSnapshot object is as an argument to
 * the #GtkWidget::snapshot vfunc.
My conclusion is that it must be wrapped as a _CLASS_GENERIC, containing a
pointer to a GtkSnapshot, which the wrapping Gtk::Snapshot object does not own.
Not very attractive. Is there a better way?
Comment 19 Kjell Ahlstedt 2017-01-03 16:05:59 UTC
Yes, there is a better way. Wrap GtkSnapshot like GThread is wrapped in
Glib::Threads::Thread. Let Gtk::Snapshot be a class with no data members.
Convert between Gtk::Snapshot* and GtkSnapshot* with reinterpret_cast.
But unlike Glib::Threads::Thread make it a _CLASS_GENERIC, making it possible
to use _WRAP_METHOD.
Comment 20 Kjell Ahlstedt 2017-12-01 12:23:37 UTC
The problem with snapshot_vfunc() and signal_draw() was solved several months
ago. Now another important difference between gtk+-3 and gtk+-4 has popped up.
See bug 786048 comments 4-5. It's no longer always possible to remove a widget
from its container widget with a call such as 
 gtk_container_remove (GTK_CONTAINER (gtk_widget_get_parent (child)), child);

That means it's no longer possible to call g_object_run_dispose() from
Gtk::Object::_release_c_instance() (called from Gtk::Object::~Object()) the way
we've done so far.

I tried to just remove the calls to g_object_run_dispose(), but it doesn't work.
Gtkmm's demo program then generates lots of warnings and critical messages.
You can't delete the C++ wrapper and keep the wrapped C object alive.

Benjamin Otte in bug 786048 comment 5 suggests that we shall treat widgets in
the same way as other GObjects, i.e. store them in a reference counting pointer.
That would require massive changes in gtkmm and every application that uses
gtkmm. Is it possible? I suppose there was a reason for once deciding not to
keep widgets in Glib::RefPtr. Does that reason still exist?
Comment 21 Kjell Ahlstedt 2017-12-05 09:15:47 UTC
It would probably be possible to keep our present code for widgets and
Gtk::Object, including destructors, if we add a new restriction:

  - A widget must not be deleted while it's contained in a container widget.

It would introduce a restriction on the order of widget declarations in
subclasses of Gtk::Window and Gtk::ApplicationWindow.

  class MyWindow : public Gtk::Window
  {
    // ....
  protected:
    Gtk::Label m_label_in_box1;
    Gtk::Box m_box1; // OK, container widget is destructed before its child.

    Gtk::Box m_box2;            // Wrong, child widget is destructed before
    Gtk::Label m_label_in_box2; // the box it's contained in.
  }
Comment 22 Kjell Ahlstedt 2018-02-12 09:06:19 UTC
Created attachment 368245 [details] [review]
patch: Gtk::Object::_release_c_instance(): Don't release if in a container

This simple patch may actually solve the problem. All demo programs run without
segfault and without messages that relate to destruction of C instances.

Shall I push the patch right away, or does anyone first want to test it more
thoroughly?
Comment 23 Kjell Ahlstedt 2018-03-01 08:44:11 UTC
Comment on attachment 368245 [details] [review]
patch: Gtk::Object::_release_c_instance(): Don't release if in a container

I've pushed the patch in comment 22.
Comment 24 GNOME Infrastructure Team 2018-05-22 12:15:39 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/gtkmm/issues/13.