Currently, the memory management in ClutterScript is overly complicated.
The basic design tenet should be:
- ClutterScript owns a reference on every object it creates
This allows the Script instance to reliably handle the lifetime of the
instances from creation to disposal.
In case of unmerge, the Script instance should destroy any Actor
instance, except for the Stage, and release the reference it owns. The
Stage is special because it's really owned by Clutter itself, and it
should be destroyed explicitly.
When disposing the Script itself, it should just release the reference;
any parented actor, or any InitiallyUnowned instance, will then be
managed by the parent object, as they should, while every GObject
instance will go away, as documented.
This commit is based on a patch by:
Henrik Hedberg <hhedberg@innologies.fi>
http://bugzilla.clutter-project.org/show_bug.cgi?id=2316
Comprehensively add (out) annotations to functions parameters
returning int/float/double.
Not handled here: structure out returns like ClutterColor or
ClutterPerspective or GValue that should get (out caller-allocates).
Not handled here: Cogl
http://bugzilla.clutter-project.org/show_bug.cgi?id=2302
Up until now, the "behaviours" member of an actor definition was parsed
by the ClutterScript parser itself - even though it's not strictly
necessary.
In an effort to minimize the ad hoc code in the Script parser, we should
let ClutterActor handle all the special cases that involve
actor-specific members.
The get_id_from_node() internal function should be exposed inside
Clutter (as a private function) because it can be useful to other
custom parsing code. The code is pretty trivial, but it would be
pointless to re-implement it.
Layout properties work similarly to child properties, with the added
headache that they require the 3-tuple:
( layout manager, container, actor )
to be valid in order to be inspected, parsed and applied. This means
using the newly added back-pointer from the container to the layout
manager and then rejigging a bit how the ScriptParser handles the
unresolved properties.
Similarly to the child properties, which use the "child::" prefix, the
layout manager properties use the "layout::" prefix and are defined with
the child of a container holding a layout manager.
While this is totally fine (0 in the pointer context will be converted
in the right internal NULL representation, which could be a value with
some bits to 1), I believe it's clearer to use NULL in the pointer
context.
It seems that, in most case, it's more an overlook than a deliberate
choice to use FALSE/0 as NULL, eg. copying a _COGL_GET_CONTEXT (ctx, 0)
or a g_return_val_if_fail (cond, 0) from a function returning a
gboolean.
Since using addresses that might change is something that finally
the FSF acknowledge as a plausible scenario (after changing address
twice), the license blurb in the source files should use the URI
for getting the license in case the library did not come with it.
Not that URIs cannot possibly change, but at least it's easier to
set up a redirection at the same place.
As a side note: this commit closes the oldes bug in Clutter's bug
report tool.
http://bugzilla.openedhand.com/show_bug.cgi?id=521
The top-level types list was comically out of date, and it was only
determining whether the type we were constructing was initially unowned
or a full object. We can safely replace it with a simple type check.
Instead of taking a string and duplicating the "is it a string or an
integer" check in both Alpha and Animation, the function in
ClutterScript that resolves the animation mode values should take a
JsonNode and do all the checks it needs.
PropertyInfo should store a copy of the JsonNodes it references, so
that property_info_free() can safely dispose them, and we can reference
values across different UI definition data.
The implicit timeline parsing code is not copying the JsonNode; this
leads to a double free in some cases, which is masked by the GSlice
allocator and produces a heap corruption later on.
ClutterBehaviour should implement the Scriptable interface
and parse ClutterAlpha when implicitly defined, instead of
having this ad hoc code inside ClutterScriptParser itself.
After all, only ClutterBehaviour supports Alpha defined
implicitly.
The ClutterScriptParser should do most of the heavy-lifting for
parsing a JSON object member defining another JSON object into
a GObject property defined using a GParamSpecObject.
The ClutterScript parser needs to be extended to parse child properties
and apply them after an actor has been added to a container. In order to
distinguish child properties from regular GObject properties we can use
the "child::" prefix, e.g.:
{
"type" : "ClutterRectangle",
"id" : "child-01",
"child::has-focus" : true,
...
}
Parsing child properties can be deferred to the ClutterScriptable
interface, just like regular properties.
All the ClutterColor parsing rules should be coalesced inside
clutter_script_parse_color(): object, array and string notations
are the canonical ways of defining a ClutterColor inside a
ClutterScript definition. Having a single function in charge of
the parsing cleans up the code.
Currently, ClutterScriptParser will construct the object (using the
construct-only and construct parameters), apply the properties from
the ClutterScript definition, and eventuall will add children and
behaviours.
The construction phase should be more compartimentalized: the objects
should be constructed first and eventual children and behaviours
added. Then, once an object is requested or when the parsing process
has terminated, all the properties should be applied.
This change allows us to set up the actors before setting their
non-construct properties.
ClutterScript is currently a mix of parser-related code and
the ClutterScript object. All the parser-related code should
be moved inside a private class, ClutterScriptParser, inheriting
from JsonParser.
The clutter-script-parser.c does not have a copyright and license
notices; even though the LGPL is a per-project license and not a
per-file license, having those notices in every source file is a
good idea.
* clutter/clutter-script-parser.c:
* clutter/clutter-script-private.h: Clean up the code; add a
conversion function for reading a ClutterColor out of a
JSON object or array definition.
* clutter/clutter-script.c: Clean up the code; document properly
how we translate from type name to type function.
* clutter/clutter-script-parser.c:
(clutter_script_get_type_from_symbol),
(clutter_script_get_type_from_class): Use BIND_LAZY flag
when looking at the symbols, so we don't load them all up.
* clutter/clutter-script.c (resolve_alpha_func): Ditto as above.
Remove the layout containers: they will be moved to a
high-level library.
* clutter/clutter.h:
* clutter/Makefile.am: Remove layout and boxes from the
build.
* clutter/clutter-layout.[ch]: Remove the ClutterLayout
interface.
* clutter/clutter-box.[ch]:
* clutter/clutter-hbox.[ch]:
* clutter/clutter-vbox.[ch]: Remove ClutterBox and its
subclasses.
* clutter/clutter-label.c: Remove ClutterLayout implementation
* clutter/clutter-script-private.h:
* clutter/clutter-script-parser.c:
* clutter/clutter-script.c:
(clutter_script_parse_node): Remove special parsing for
ClutterMargin and ClutterPadding.
* clutter/clutter-types.h: Remove ClutterPadding and ClutterMargin.
* tests/Makefile.am:
* tests/test-boxes.c: Remove the boxes test case.
* clutter/Makefile.am:
* clutter/clutter.h:
* clutter/clutter-scriptable.[ch]: Add the ClutterScriptable
interface; by implementing this interface, a class can
override the UI definition parsing and transform complex data
types into GObject properties, or allow custom properties.
* clutter/clutter-script.c:
* clutter/clutter-script-parser.c:
* clutter/clutter-script-private.h: Rearrange the code and
use the ClutterScriptable interface to parse and build the
custom properties. This cleans up the code and also it makes
it more reliable (the complex type parsing is now done using
the target type and not just the name of the property).
