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 568913 - support instantiable non-GObject fundamental types
support instantiable non-GObject fundamental types
Status: RESOLVED FIXED
Product: gobject-introspection
Classification: Platform
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gobject-introspection Maintainer(s)
gobject-introspection Maintainer(s)
: 572420 594236 (view as bug list)
Depends on:
Blocks: 550616 559704 621716
 
 
Reported: 2009-01-23 20:40 UTC by Colin Walters
Modified: 2015-02-07 16:46 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add support for non-GObject fundamental objects (32.43 KB, patch)
2010-06-12 21:12 UTC, Johan (not receiving bugmail) Dahlin
reviewed Details | Review
Add support for non-GObject fundamental objects (46.73 KB, patch)
2010-06-16 01:53 UTC, Johan (not receiving bugmail) Dahlin
accepted-commit_now Details | Review
Add support for non-GObject fundamental objects (46.17 KB, patch)
2010-07-09 13:19 UTC, Johan (not receiving bugmail) Dahlin
committed Details | Review

Description Colin Walters 2009-01-23 20:40:23 UTC
ClutterUnit is a registered fundamental integral type that is best represented in bindings as a float.  Vala apparently has support for invoking the GValue transformation functions; so if we had an annotation to say e.g.:  (expose float) or something, then bindings could do the same.
Comment 1 Emmanuele Bassi (:ebassi) 2009-01-23 20:44:27 UTC
vala has this vapi annotation for the type:

  [CCode (cname = "ClutterFixed",
          cheader_filename = "clutter/clutter.h",
          type_id = "CLUTTER_TYPE_FIXED",
          get_value_function = "clutter_value_get_fixed",
          set_value_function = "clutter_value_set_fixed",
          default_value = "0",
          type_signature = "i",
          marshaller_type_name = "INT")]

and this for the equivalency between a type and the C type (in this case, gint32 has the same rank):

  [IntegerType (rank = 6)]
Comment 2 Johan (not receiving bugmail) Dahlin 2009-01-23 21:54:35 UTC
Appears we need the following:

 get-value-function
 set-value-function
 default-value (why?)
 type-signature
 
What about the vala bindings for GstFraction/GstIntRange etc?
Comment 3 Emmanuele Bassi (:ebassi) 2009-01-24 13:43:12 UTC
(In reply to comment #2)
> Appears we need the following:
> 
>  get-value-function
>  set-value-function
>  default-value (why?)

I guess for initialization on the stack; it can probably be safely ignored.

>  type-signature
> 
> What about the vala bindings for GstFraction/GstIntRange etc?

there are none; gstreamer-0.10.vapi only exposes ParamSpecFraction.
Comment 4 Johan (not receiving bugmail) Dahlin 2009-02-19 14:32:55 UTC
*** Bug 572420 has been marked as a duplicate of this bug. ***
Comment 5 Johan (not receiving bugmail) Dahlin 2010-06-12 15:33:55 UTC
Okay, I've been looking into this as a way to add support for GstMiniObject, especially in the gjs/pygi context.

Basically we need to extend the <class> tag in GIR and GIObjectInfo in libgirepository to support other fundamental types apart from G_TYPE_OBJECT.

Using GstMiniObject as an example. The headers would need to be annotated like this:

/**
 * GstMiniObject:
 *
 * SetValueFunction: gst_value_set_mini_object
 * GetValueFunction: gst_value_get_mini_object
 */

It would be represented in the GIR as:

  <class name="MiniObject"
         c:type="GstMiniObject"
         abstract="1"
         glib:is-fundamental-type="1"
         glib:type-signature="i"
         glib:type-name="GstMiniObject"
         glib:get-type="gst_mini_object_get_type"
         glib:set-value-function="gst_value_set_mini_object"
         glib:get-value-function="gst_value_get_mini_object"> .. </class>
  
These new apis would be added to GIObjectInfo:

gboolean      g_object_info_is_fundamental_type(GIObjectInfo *info);
GValueSetFunc g_object_info_get_set_value_function(GIObjectInfo *info);
GValueGetFunc g_object_info_get_get_value_function(GIObjectInfo *info);
const char *  g_object_info_get_type_signature(GIObjectInfo *info);

The GValue<->Wrapper routines in language bindings would need to be modified to receive either a set/get function or the GIObjectInfo directly.
Comment 6 Johan (not receiving bugmail) Dahlin 2010-06-12 21:12:35 UTC
Created attachment 163485 [details] [review]
Add support for non-GObject fundamental objects

This patch adds support for instantiable fundamental object types,
which are not GObject based. This is mostly interesting for being
able to support GstMiniObject's which are extensivly used in GStreamer.
Includes a big test case to the Everything module (inspired by
GstMiniObject) which should be used by language bindings who wishes to
test this functionallity.

This patch increases the size of the typelib and breaks compatibility
with older typelibs.
Comment 7 Colin Walters 2010-06-14 18:32:53 UTC
Review of attachment 163485 [details] [review]:

In the big picture, wasn't a lot of the motivation behind recent GObject optimizations is to help GstMiniObject die?  Has anyone asked the GStreamer people to reevaluate performance there?

Actually, can GstMiniObject just be treated as a boxed from a binding perspective?  All we care about is _ref/_unref, and boxeds give us that.  Basically in the scanner just lie and do if (ns = "Gst" and name = "MiniObject) return new Boxed(...)

Otherwise, looks fairly mechanical and I don't have objections.

::: gir/everything.c
@@ +1920,3 @@
+  mo_class->finalize (fundamental_object);
+
+  if (G_LIKELY (g_atomic_int_dec_and_test (&fundamental_object->refcount))) {

Not that this really matters, but this isn't a valid use of G_LIKELY/G_UNLIKELY; as I understand it, missing them can be really quite expensive, and it's totally valid for a refcount to go to 0, in fact quite often.

They were added for implementing g_assert type stuff basically.
Comment 8 Johan (not receiving bugmail) Dahlin 2010-06-15 12:20:46 UTC
(In reply to comment #7)
> Review of attachment 163485 [details] [review]:
> 
> In the big picture, wasn't a lot of the motivation behind recent GObject
> optimizations is to help GstMiniObject die?  Has anyone asked the GStreamer
> people to reevaluate performance there?

GObject will always be larger and slower, unless we plan to break the ABI.
GStreamer uses GstMiniObject in their APIs and its unlikely that they want to break it,
so we'll just have to live with that.

> 
> Actually, can GstMiniObject just be treated as a boxed from a binding
> perspective?  All we care about is _ref/_unref, and boxeds give us that. 
> Basically in the scanner just lie and do if (ns = "Gst" and name = "MiniObject)
> return new Boxed(...)
> 

Well, boxed gives us free/copy which is not quite the same as ref counting.
Fundamentals also support derivation which Boxed doesn't. So it sits in
the middle between GObject and GBoxed.

> Otherwise, looks fairly mechanical and I don't have objections.
> 
> ::: gir/everything.c
> @@ +1920,3 @@
> +  mo_class->finalize (fundamental_object);
> +
> +  if (G_LIKELY (g_atomic_int_dec_and_test (&fundamental_object->refcount))) {
> 
> Not that this really matters, but this isn't a valid use of
> G_LIKELY/G_UNLIKELY; as I understand it, missing them can be really quite
> expensive, and it's totally valid for a refcount to go to 0, in fact quite
> often.
> 
> They were added for implementing g_assert type stuff basically.

This is just copied over from GStreamer, TestFundamentalObject == GstMiniObject but
with unnecessary api stripped. Better to keep the implementation close to that. You should submit
a GStreamer bug if you think strongly about it though.

There are a few small things missing in this patch, but I don't want to updated until I can validate it,
which currently means that I need to finish a proof of concept in gjs to support it.
Comment 9 Colin Walters 2010-06-15 12:49:24 UTC
(In reply to comment #8)
> (In reply to comment #7)
> > Review of attachment 163485 [details] [review] [details]:
> > 
> > In the big picture, wasn't a lot of the motivation behind recent GObject
> > optimizations is to help GstMiniObject die?  Has anyone asked the GStreamer
> > people to reevaluate performance there?
> 
> GObject will always be larger and slower, unless we plan to break the ABI.
> GStreamer uses GstMiniObject in their APIs and its unlikely that they want to
> break it,
> so we'll just have to live with that.

Ok, well I just asked on #gstreamer.  No one has benchmarks, but the real problem is there's no plan for 1.0 in the near future, so they can't break ABI.

So we'll need this patch or something like it.
 
> Well, boxed gives us free/copy which is not quite the same as ref counting.

Boxed is sort of a generalization of refcounting where you just set copy=ref, free=unref.  (It took me a long time to get this).

> Fundamentals also support derivation which Boxed doesn't. 

Good point, yes.
Comment 10 Colin Walters 2010-06-15 13:16:22 UTC
Also I'd like to batch up API/ABI breaks. 

Let's make a 0.8 branch and commit breaks (such as this typelib one) to master, hopefully really getting to 1.0 in time for GNOME 3?
Comment 11 Johan (not receiving bugmail) Dahlin 2010-06-16 01:53:00 UTC
Created attachment 163758 [details] [review]
Add support for non-GObject fundamental objects

This patch adds support for instantiable fundamental object types,
which are not GObject based. This is mostly interesting for being
able to support GstMiniObject's which are extensivly used in GStreamer.
Includes a big test case to the Everything module (inspired by
GstMiniObject) which should be used by language bindings who wishes to
test this functionallity.

This patch increases the size of the typelib and breaks compatibility
with older typelibs.
Comment 12 Colin Walters 2010-07-09 13:06:17 UTC
Review of attachment 163758 [details] [review]:

Looks fine
Comment 13 Johan (not receiving bugmail) Dahlin 2010-07-09 13:19:28 UTC
The following fix has been pushed:
1e9822c Add support for non-GObject fundamental objects
Comment 14 Johan (not receiving bugmail) Dahlin 2010-07-09 13:19:34 UTC
Created attachment 165555 [details] [review]
Add support for non-GObject fundamental objects

This patch adds support for instantiable fundamental object types,
which are not GObject based. This is mostly interesting for being
able to support GstMiniObject's which are extensivly used in GStreamer.
Includes a big test case to the Everything module (inspired by
GstMiniObject) which should be used by language bindings who wishes to
test this functionallity.

This patch increases the size of the typelib and breaks compatibility
with older typelibs.
Comment 15 Johan (not receiving bugmail) Dahlin 2010-09-06 04:22:09 UTC
*** Bug 594236 has been marked as a duplicate of this bug. ***
Comment 16 André Klapper 2015-02-07 16:46:56 UTC
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]