This is an automated email from the git hooks/post-receive script.
git pushed a commit to branch edje-vector-intergration
in repository efl.
View the commit online.
commit 43dd2187bcb603cfe30301bd63ae3eca373050ab
Author: [email protected] <[email protected]>
AuthorDate: Thu Apr 30 21:30:37 2026 -0600
edje: address full-branch review findings for vector state animation
Consolidates three critical blockers and six should-fix items from the
full-branch code review of the 41-commit vector state animation feature
(Phases 1–8.4). All changes are single-commit to support atomicity.
## Blocker 1: Vector part description memory leak on collection unload
The EDJE_PART_TYPE_VECTOR case was missing from both cleanup hooks in
edje_load.c, leaving vg.overrides lists and their nested allocations
(stroke_dash arrays, gradient stops) on the heap for every part
description in every loaded theme. The leak persisted across .edj file
loads and reloads because Edje collections are cached in-memory.
Added _edje_collection_free_part_description_clean and its sibling
_edje_collection_free_part_description_free handlers, mirroring the
external-params cleanup pattern: when free_strings is true (stringshare
mode), use _edje_vg_override_free; when false (Eet dict pointers), free
sub-arrays directly and the struct (Eet dict strings must not be freed).
This is the production memory leak referenced in the PR body.
## Blocker 2: Compiler silently writes corrupted .edj on tree-id patch failure
In edje_cc_out.c, the vg desc->tree_id patch loop was issuing a warning
and continuing when a tree was not found in the index map. This was
silently leaving tree_id=-1 on disk, producing a theme that renders no
vector output. Reproduction: any vector.use: reference pointing to a
non-existent or incorrectly-indexed tree would produce a corrupted .edj.
Promoted the warning to error_and_abort with the actual tree name (not
%p) and a message explaining the internal consistency failure. Actionable
for theme developers: they can now see which tree failed to index.
## Blocker 3: edje_edit tree_id fixup loop does not page in unloaded collections
When edje_edit_vg_tree_del removes a tree from vd->trees, the second-pass
loop that decrements tree_id in all remaining parts was only visiting
collections already loaded (ce2->ref). Groups not currently in memory
(e.g., unloaded via Edje_Cache) retained stale tree_id references past
the removed slot, silently pointing to wrong trees.
Added _edje_file_coll_open + _edje_cache_coll_unref pattern in the
second-pass loop to match the first-pass (in-use group) scan. This
ensures all collection groups in the file, loaded or not, have their
tree_id adjusted consistently. Comment clarifies the "mirror the
first-pass pattern" intent for future maintainers.
## Should-fix 3: Transition-end stale cache slot leak
When a transition completes (ep->param2 becomes NULL) but the previous
transition left an Efl_VG object in rpv->cache_param2, the object was
never released. This created a per-completed-transition, per-part leak
lasting for the object lifetime.
Added a slot-clear call when !ep->param2 && rpv->cache_param2.efl_vg
to release stale B-side cache entries. Scales from one leak per transition
to no leaks.
## Should-fix 4: Color class resolution did not properly clear the sentinel
_resolve_binding was using eina_stringshare_replace(..., "") to mark
resolved bindings, leaving an empty stringshare sentinel. This was
fragile: future passes that call for_each_class_name would skip the
binding (checking *cb->color_class), but direct dereference code (like
new diagnostic or iteration utilities) could break if they expected
NULL to mean "no class."
Changed to eina_stringshare_del(cb->color_class) + NULL assignment.
NULL is now the definitive "resolved or not found" sentinel. Updated
the doc comment in edje_vg_tree.h to reflect this change.
## Should-fix 5: Cascading VEC014 diagnostics on same node
When VEC014 (transform-form conflict) was demoted from hard error to
warning, the conflict path did not return early. Subsequent statements
on the same node would re-evaluate and re-emit the diagnostic, resulting
in spammy output and potentially incorrect error counts in logs.
Added return; after edje_diag_emit in the conflict path so the bitset
tracks the error and later statements don't re-emit.
## Should-fix 6: Early-abort trees leak their root subtrees
When _edje_cc_vg_cleanup runs, it expects all trees to have had
shallow-copy (root nullified). On early abort paths (e.g., VEC001
duplicate-name error), the shallow-copy never ran and root was live.
This left unfreed root Edle_Vg_Node subtrees.
Added _edje_vg_tree_free_fields(t, EINA_TRUE) before free(t) to
unconditionally free roots. Comment documents the two paths (normal
shallow-copy vs. early abort).
## Should-fix 7: Example build list alphabetical ordering
The edje-vector-states example was listed before edje-swallow, breaking
alphabetical order in meson.build. Moved it after edje-swallow2 to
restore ordering and make future additions easier to spot.
Deliberately NOT addressed:
- S1 (forward-decl support for vector.use:) requires Eet on-disk format
change; defer to follow-up.
- S2 (free() call placement) was false positive in the original review;
code was already correct.
- S8 (uniform EAPI decoration) follows project convention; no change.
All 91 edje_suite tests pass. Reviewer-of-fix nits (B2 message polish,
stale doc comment) also addressed in this commit.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
---
src/bin/edje/edje_cc_handlers.c | 9 ++++++++-
src/bin/edje/edje_cc_out.c | 7 +++++--
src/examples/edje/meson.build | 2 +-
src/lib/edje/edje_calc.c | 5 +++++
src/lib/edje/edje_edit.c | 27 ++++++++++++++++++++++++---
src/lib/edje/edje_load.c | 33 +++++++++++++++++++++++++++++++++
src/lib/edje/edje_vg_tree.c | 7 ++++---
src/lib/edje/edje_vg_tree.h | 11 +++++------
8 files changed, 85 insertions(+), 16 deletions(-)
diff --git a/src/bin/edje/edje_cc_handlers.c b/src/bin/edje/edje_cc_handlers.c
index 91379ed2e8..f75d1d6aa2 100644
--- a/src/bin/edje/edje_cc_handlers.c
+++ b/src/bin/edje/edje_cc_handlers.c
@@ -229,6 +229,9 @@ _vg_xform_track(int new_bit)
"transform.matrix and decomposed transform "
"(translate/rotate/scale) are mutually exclusive on "
"the same node within one description");
+ /* Do not update the bitset after a conflict: subsequent statements
+ on the same node must not trigger additional cascading diagnostics. */
+ return;
}
uintptr_t updated = prev | (uintptr_t)new_bit;
@@ -270,7 +273,11 @@ _edje_cc_vg_cleanup(void)
Edje_Vg_Tree *t;
EINA_LIST_FOREACH(all_vg_trees, l, t)
{
- /* root was nullified by shallow-copy; free only the wrapper. */
+ /* root was nullified by shallow-copy in the normal path.
+ On early abort (e.g. VEC001 duplicate-name error), the
+ shallow-copy never ran and root is still live — so always
+ call _edje_vg_tree_free_fields before freeing the wrapper. */
+ _edje_vg_tree_free_fields(t, EINA_TRUE);
free(t);
}
eina_list_free(all_vg_trees);
diff --git a/src/bin/edje/edje_cc_out.c b/src/bin/edje/edje_cc_out.c
index 37c6ac204a..31134180aa 100644
--- a/src/bin/edje/edje_cc_out.c
+++ b/src/bin/edje/edje_cc_out.c
@@ -1623,8 +1623,11 @@ data_write_vectors_trees(Eet_File *ef, int *tree_num)
uintptr_t idx_plus_1 = (uintptr_t)eina_hash_find(idx_map, &t);
if (idx_plus_1 == 0)
{
- WRN("vg desc->tree_id patch: tree not found in index map");
- continue;
+ error_and_abort(ef,
+ "vg desc->tree_id patch: tree '%s' (tree_id=%d) not found "
+ "in index map — internal consistency error; this theme may "
+ "have been produced by an incompatible tool",
+ t->name ? t->name : "<unnamed>", desc->vg.tree_id);
}
desc->vg.tree_id = (int)(idx_plus_1 - 1);
}
diff --git a/src/examples/edje/meson.build b/src/examples/edje/meson.build
index 58e8f8fa87..07340ca50a 100644
--- a/src/examples/edje/meson.build
+++ b/src/examples/edje/meson.build
@@ -118,9 +118,9 @@ edje_examples = [
'edje-multisense',
'edje-perspective',
'edje-signals-messages',
- 'edje-vector-states',
'edje-swallow',
'edje-swallow2',
+ 'edje-vector-states',
'edje-table',
'edje-text',
'edje-textblock-hyphenation',
diff --git a/src/lib/edje/edje_calc.c b/src/lib/edje/edje_calc.c
index 5630a5775e..2b78067557 100644
--- a/src/lib/edje/edje_calc.c
+++ b/src/lib/edje/edje_calc.c
@@ -3322,6 +3322,11 @@ _edje_vector_recalc_apply(Edje *ed, Edje_Real_Part *ep, Edje_Calc_Params *p3 EIN
return;
}
+ /* No transition: release the B-side cache slot if it holds a stale
+ entry from a previous transition on the same part. */
+ if (!ep->param2 && rpv->cache_param2.efl_vg)
+ _edje_vg_cache_slot_clear(&rpv->cache_param2);
+
/* Transition active? (param2 set and pos meaningfully non-zero) */
if (ep->param2 && NEQ(pos, ZERO))
{
diff --git a/src/lib/edje/edje_edit.c b/src/lib/edje/edje_edit.c
index 48f0ccb846..49ff67b1b2 100644
--- a/src/lib/edje/edje_edit.c
+++ b/src/lib/edje/edje_edit.c
@@ -17424,7 +17424,10 @@ edje_edit_vg_tree_del(Evas_Object *obj, const char *name)
memmove(&vd->trees[tree_idx], &vd->trees[tree_idx + 1],
sizeof(Edje_Vg_Tree) * (vd->trees_count - (unsigned int)tree_idx - 1));
- /* Fix up tree_id references that pointed past the removed slot. */
+ /* Fix up tree_id references that pointed past the removed slot.
+ * Mirror the first-pass pattern exactly: page in unloaded collections
+ * via _edje_file_coll_open so groups not currently in use still get
+ * their tree_id adjusted. */
if (ed->file->collection)
{
Eina_Iterator *it2 =
@@ -17433,8 +17436,24 @@ edje_edit_vg_tree_del(Evas_Object *obj, const char *name)
EINA_ITERATOR_FOREACH(it2, ce2)
{
- Edje_Part_Collection *col = ce2->ref;
- if (!col) continue;
+ Edje_Part_Collection *col;
+ Eina_Bool transient2 = EINA_FALSE;
+
+ if (ce2->ref)
+ {
+ col = ce2->ref;
+ }
+ else
+ {
+ col = _edje_file_coll_open(ed->file, ce2->entry);
+ transient2 = EINA_TRUE;
+ }
+
+ if (!col)
+ {
+ continue;
+ }
+
for (pi = 0; pi < col->parts_count; ++pi)
{
Edje_Part *ep = col->parts[pi];
@@ -17452,6 +17471,8 @@ edje_edit_vg_tree_del(Evas_Object *obj, const char *name)
vdesc->vg.tree_id--;
}
}
+
+ if (transient2) _edje_cache_coll_unref(ed->file, col);
}
eina_iterator_free(it2);
}
diff --git a/src/lib/edje/edje_load.c b/src/lib/edje/edje_load.c
index 043413b562..849c9c1b4d 100644
--- a/src/lib/edje/edje_load.c
+++ b/src/lib/edje/edje_load.c
@@ -2606,6 +2606,38 @@ _edje_collection_free_part_description_clean(int type, Edje_Part_Description_Com
eina_stringshare_del(text->text.font.str);
}
break;
+
+ case EDJE_PART_TYPE_VECTOR:
+ {
+ Edje_Part_Description_Vector *svg;
+ Edje_Vg_Override *ovr;
+
+ svg = (Edje_Part_Description_Vector *)desc;
+ if (free_strings)
+ {
+ /* Strings in overrides are stringshares — use full free. */
+ EINA_LIST_FREE(svg->vg.overrides, ovr)
+ _edje_vg_override_free(ovr);
+ }
+ else
+ {
+ /* Strings are Eet dictionary pointers (mmap-backed); do NOT
+ free them. Sub-arrays (stroke_dash, gradient stops) are
+ still heap-allocated by Eet even in mmap mode and must be
+ freed. Mirror the pattern of _edje_external_params_free:
+ free arrays and the struct, skip string fields. */
+ EINA_LIST_FREE(svg->vg.overrides, ovr)
+ {
+ if (ovr->expected_type == EDJE_VG_NODE_SHAPE)
+ free(ovr->payload.shape.stroke_dash);
+ else if (ovr->expected_type == EDJE_VG_NODE_GRADIENT_LINEAR ||
+ ovr->expected_type == EDJE_VG_NODE_GRADIENT_RADIAL)
+ free(ovr->payload.gradient.stops);
+ free(ovr);
+ }
+ }
+ }
+ break;
}
}
@@ -2636,6 +2668,7 @@ case EDJE_PART_TYPE_##Type: eina_mempool_free(ce->mp->mp.Type, Desc); \
FREE_POOL(EXTERNAL, ce, desc);
FREE_POOL(SNAPSHOT, ce, desc);
FREE_POOL(SPACER, ce, desc);
+ FREE_POOL(VECTOR, ce, desc);
}
}
diff --git a/src/lib/edje/edje_vg_tree.c b/src/lib/edje/edje_vg_tree.c
index 15ed5e9d0c..dd1b8bf5d1 100644
--- a/src/lib/edje/edje_vg_tree.c
+++ b/src/lib/edje/edje_vg_tree.c
@@ -1852,7 +1852,7 @@ _edje_vg_cache_get_or_build(Edje_Vg_Cache_Slot *slot,
static void
_resolve_binding(Edje_Vg_Color_Binding *cb, const Edje *ed)
{
- if (!cb || !cb->color_class || !*cb->color_class) return;
+ if (!cb || !cb->color_class) return;
Edje_Color_Class *cc = _edje_color_class_recursive_find(ed, cb->color_class);
if (cc)
@@ -1863,7 +1863,8 @@ _resolve_binding(Edje_Vg_Color_Binding *cb, const Edje *ed)
cb->a = (cb->a * cc->a) / 255;
}
/* Hit or miss: clear so subsequent passes see resolved RGBA. */
- eina_stringshare_replace(&cb->color_class, "");
+ eina_stringshare_del(cb->color_class);
+ cb->color_class = NULL;
}
static void
@@ -1911,7 +1912,7 @@ static void
_emit_binding_class(const Edje_Vg_Color_Binding *b,
Edje_Vg_Class_Cb cb, void *data)
{
- if (b->color_class && *b->color_class)
+ if (b->color_class)
cb(b->color_class, data);
}
diff --git a/src/lib/edje/edje_vg_tree.h b/src/lib/edje/edje_vg_tree.h
index 7e532284e6..4bc5bdaf1b 100644
--- a/src/lib/edje/edje_vg_tree.h
+++ b/src/lib/edje/edje_vg_tree.h
@@ -188,15 +188,14 @@ EAPI void _edje_vg_cache_slot_clear(Edje_Vg_Cache_Slot *slot);
/* --- Phase 6.2: class color resolution ----------------------------------- */
/* Walk the working tree; for every Edje_Vg_Color_Binding whose
- * color_class is non-NULL/non-empty, look up the class via
+ * color_class is non-NULL, look up the class via
* _edje_color_class_recursive_find(ed, name). On hit, multiply the
* binding's raw RGBA componentwise (resolved.r = base.r * C.r / 255).
- * Then clear color_class to "" (empty stringshare) so subsequent passes
- * (lerp / materialize) see only resolved RGBA.
+ * Then release color_class (eina_stringshare_del + NULL) so subsequent
+ * passes (lerp / materialize) see only resolved RGBA.
*
- * Empty string is the "no class" sentinel — left untouched. Class
- * not found: clear the binding's class so subsequent recalcs don't keep
- * looking it up. No-op when working or ed is NULL. */
+ * Class not found: also release the binding's class so subsequent
+ * recalcs don't keep looking it up. No-op when working or ed is NULL. */
EAPI void _edje_vg_tree_resolve_colors(Edje_Vg_Tree *working, const Edje *ed);
/* --- Phase 6.1: class-name walk helpers ---------------------------------- */
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
