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 b231a5f2e7bdf523c45d926fdee67104b34312f9
Author: [email protected] <[email protected]>
AuthorDate: Fri May 1 09:42:29 2026 -0600
edje: fix stringshare ref bug, gradient override parser, and vector_states demo
## Stringshare ownership model fix
The edje_vg_tree.c dup helpers (_node_dup, _color_binding_dup, and
_edje_vg_override_dup) were calling eina_stringshare_ref on input strings.
The _ref call is only valid when the input is a real stringshare handle —
it checks a magic field, which fails at runtime (~75725f6c magic error)
when the input is a borrowed pointer from an Eet-dictionary mapping.
This was not caught by unit tests because the test fixture always loads
into heap-allocated stringshare-owned trees (parser produces them).
Production loaded .edj files decode into dictionary-backed trees, which
failed the magic check on every override-apply operation.
Replaced all 16 _ref calls with eina_stringshare_add, which is
dictionary-safe: it looks up the bytes and creates a fresh stringshare
regardless of source ownership. Surfaced during edje_player smoke testing
on every mouse,in event (override dup on interaction).
Added a String-ownership convention block to edje_vg_tree.h preamble
documenting the dual-ownership model (Eet-dictionary vs stringshare),
rules for new code, and the rationale (mmap efficiency). Updated
_edje_vg_tree_dup doc to explain the dictionary-safe guarantee.
## Parser override-mode bug fix
The _vg_open_override handler (edje_cc_handlers.c) zero-initialized the
Edje_Vg_Override struct via calloc but never set payload.type. When a
description-scope override block targeted a gradient node
(vector.gradient_linear { target: …; start: …; end: …; }), the
EXPECT_GRADIENT_LINEAR guards on st_vg_grad_start/end/radius read
current_vg_node->type (which was 0 = EDJE_VG_NODE_SHAPE) and aborted
with "linear-gradient-only property on wrong node".
No fixture covered this — the vector_states.edc example originally did
not override a gradient node. One-line fix sets ovr->payload.type = type
after calloc to match expected_type.
## vector_states.edc demo enhancement
Added a gradient_linear node body_grad as a sibling of the body shape,
with two color_class-bound stops (logo_fill / logo_outline). The body
shape now references it via fill.gradient: "body_grad" instead of using
a direct fill.color_class.
The hover state now has a second override block (vector.gradient_linear)
that sweeps the gradient direction from vertical (top→bottom) to
horizontal (left→right) AND swaps the leading stop's class from logo_fill
to logo_hover. Demonstrates four features at once: gradient nodes in the
toplevel tree, shape-to-gradient reference, gradient endpoint
interpolation across states, and class-bound gradient stops.
The demo originally did not cover gradient overrides, so the parser bug
above was not caught by visual testing until gradients were added.
All 91 edje_suite tests pass. edje_cc compiles the new gradient + override
syntax with zero warnings. edje_player smoke test no longer shows
stringshare magic errors on interactive events.
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
---
src/bin/edje/edje_cc_handlers.c | 6 +++++-
src/examples/edje/vector_states.edc | 40 +++++++++++++++++++++++++---------
src/lib/edje/edje_vg_tree.c | 38 ++++++++++++++++----------------
src/lib/edje/edje_vg_tree.h | 43 ++++++++++++++++++++++++++++++++-----
4 files changed, 93 insertions(+), 34 deletions(-)
diff --git a/src/bin/edje/edje_cc_handlers.c b/src/bin/edje/edje_cc_handlers.c
index f75d1d6aa2..bf6e64f7e2 100644
--- a/src/bin/edje/edje_cc_handlers.c
+++ b/src/bin/edje/edje_cc_handlers.c
@@ -2015,9 +2015,13 @@ _vg_open_override(Edje_Vg_Node_Type type)
ovr = calloc(1, sizeof(Edje_Vg_Override));
if (!ovr) error_and_abort(NULL, "OOM allocating override");
ovr->expected_type = type;
- /* payload zeroed; target_name still NULL.
+ /* payload mostly zeroed; target_name still NULL. Set payload.type so
+ * the EXPECT_GRADIENT_LINEAR / EXPECT_GRADIENT_RADIAL / EXPECT_SHAPE
+ * guards on property handlers (e.g. st_vg_grad_start) accept this
+ * override block — they read current_vg_node->type.
* Do NOT append to ed->vg.overrides yet — st_vg_override_target will
* decide whether to merge with an existing inherited entry or append. */
+ ovr->payload.type = type;
current_vg_override = ovr;
current_vg_node = &ovr->payload;
diff --git a/src/examples/edje/vector_states.edc b/src/examples/edje/vector_states.edc
index a6475b9bbb..06be72a23b 100644
--- a/src/examples/edje/vector_states.edc
+++ b/src/examples/edje/vector_states.edc
@@ -3,9 +3,11 @@
*
* Demonstrates:
* - A named vector tree in the toplevel vectors {} block
- * - A VECTOR part referencing it via vector.use:
- * - Two states (default, hover) with per-node shape override
- * - fill.color_class binding so the fill colour is themeable at runtime
+ * - A linear gradient node referenced from a shape's fill
+ * - A VECTOR part referencing the tree via vector.use:
+ * - Two states (default, hover) with per-node overrides on BOTH the
+ * shape (path + stroke width) AND the gradient (endpoint sweep)
+ * - color_class binding on gradient stops + stroke
* - STATE_SET programs wired to mouse,in / mouse,out for live transitions
*
* Compile:
@@ -24,11 +26,22 @@ collections {
vectors {
vector { name: "logo_glyph";
viewbox: 0 0 100 100;
+ /* The gradient node sits as a sibling of the shape inside the
+ * tree. Shapes reference it by name via fill.gradient:.
+ * Stops bind to color_class so the gradient retints together
+ * with the rest of the theme. */
+ gradient_linear { name: "body_grad";
+ start: 50 0;
+ end: 50 100;
+ spread: PAD;
+ stop { offset: 0.0; color_class: "logo_fill"; }
+ stop { offset: 1.0; color_class: "logo_outline"; }
+ }
shape { name: "body";
/* topology: M + 8×L + Z — identical command sequence in both
* states, so efl_gfx_path_interpolate() can morph the path. */
path: "M0,0 L0,0 L100,0 L100,0 L100,100 L100,100 L0,100 L0,100 L0,0";
- fill.color_class: "logo_fill";
+ fill.gradient: "body_grad";
stroke.color_class: "logo_outline";
stroke.width: 2;
}
@@ -45,17 +58,24 @@ collections {
}
description { state: "hover" 0.0;
inherit: "default" 0.0;
- /* Override the "body" shape:
- * - morph path from square to star (same topology: M + 8×L + Z)
- * - swap fill class to logo_hover (smooth crossfade via lerp)
- * - widen stroke from 2 to 4
- * stroke color_class stays logo_outline — no override needed. */
+ /* Two overrides in this state:
+ * 1. Morph the body path from square to star (same topology:
+ * M + 8×L + Z) and widen its stroke.
+ * 2. Sweep the gradient direction from vertical (top→bottom)
+ * to horizontal (left→right) AND swap the top stop to
+ * logo_hover so the warm colour leads the gradient. */
vector.shape {
target: "body";
path: "M0,0 L50,-80 L100,0 L180,50 L100,100 L50,180 L0,100 L-80,50 L0,0";
- fill.color_class: "logo_hover";
stroke.width: 4;
}
+ vector.gradient_linear {
+ target: "body_grad";
+ start: 0 50;
+ end: 100 50;
+ stop { offset: 0.0; color_class: "logo_hover"; }
+ stop { offset: 1.0; color_class: "logo_outline"; }
+ }
}
}
}
diff --git a/src/lib/edje/edje_vg_tree.c b/src/lib/edje/edje_vg_tree.c
index 902ae851c7..cff0088879 100644
--- a/src/lib/edje/edje_vg_tree.c
+++ b/src/lib/edje/edje_vg_tree.c
@@ -476,7 +476,9 @@ _edje_vg_tree_equal(const Edje_Vg_Tree *a, const Edje_Vg_Tree *b)
* Phase 4.1 — Deep-dup and override-apply helpers
* ========================================================================= */
-/* Duplicate a color binding — stringshare_ref the class string. */
+/* Duplicate a color binding — stringshare_add the class string so the
+ * dup owns its own ref regardless of whether src came from an Eet
+ * dictionary or a heap stringshare. */
static void
_color_binding_dup(Edje_Vg_Color_Binding *dst,
const Edje_Vg_Color_Binding *src)
@@ -486,7 +488,7 @@ _color_binding_dup(Edje_Vg_Color_Binding *dst,
dst->b = src->b;
dst->a = src->a;
dst->color_class = src->color_class
- ? eina_stringshare_ref(src->color_class) : NULL;
+ ? eina_stringshare_add(src->color_class) : NULL;
}
static Edje_Vg_Node *_node_dup(const Edje_Vg_Node *src);
@@ -502,7 +504,7 @@ _node_dup(const Edje_Vg_Node *src)
/* Common fields */
dst->type = src->type;
dst->visible = src->visible;
- dst->name = src->name ? eina_stringshare_ref(src->name) : NULL;
+ dst->name = src->name ? eina_stringshare_add(src->name) : NULL;
_color_binding_dup(&dst->color, &src->color);
dst->xform = src->xform; /* Edje_Vg_Transform has no heap pointers */
@@ -510,7 +512,7 @@ _node_dup(const Edje_Vg_Node *src)
{
case EDJE_VG_NODE_SHAPE:
dst->shape.path =
- src->shape.path ? eina_stringshare_ref(src->shape.path) : NULL;
+ src->shape.path ? eina_stringshare_add(src->shape.path) : NULL;
_color_binding_dup(&dst->shape.fill, &src->shape.fill);
_color_binding_dup(&dst->shape.stroke_color, &src->shape.stroke_color);
dst->shape.stroke_width = src->shape.stroke_width;
@@ -520,7 +522,7 @@ _node_dup(const Edje_Vg_Node *src)
dst->shape.stroke_dash_count = src->shape.stroke_dash_count;
dst->shape.gradient_ref =
src->shape.gradient_ref
- ? eina_stringshare_ref(src->shape.gradient_ref) : NULL;
+ ? eina_stringshare_add(src->shape.gradient_ref) : NULL;
if (src->shape.stroke_dash_count > 0 && src->shape.stroke_dash)
{
size_t sz =
@@ -566,7 +568,7 @@ _node_dup(const Edje_Vg_Node *src)
if (dst->gradient.stops)
{
memcpy(dst->gradient.stops, src->gradient.stops, sz);
- /* stringshare_ref each stop's color_class */
+ /* stringshare_add each stop's color_class — dictionary-safe */
unsigned int i;
for (i = 0; i < dst->gradient.stops_count; i++)
{
@@ -574,7 +576,7 @@ _node_dup(const Edje_Vg_Node *src)
dst->gradient.stops[i].color.color_class;
if (cc)
dst->gradient.stops[i].color.color_class =
- eina_stringshare_ref(cc);
+ eina_stringshare_add(cc);
}
}
else
@@ -597,7 +599,7 @@ _edje_vg_tree_dup(const Edje_Vg_Tree *t)
Edje_Vg_Tree *dst = calloc(1, sizeof(Edje_Vg_Tree));
if (!dst) return NULL;
- dst->name = eina_stringshare_ref(t->name);
+ dst->name = eina_stringshare_add(t->name);
dst->vbx = t->vbx;
dst->vby = t->vby;
dst->vbw = t->vbw;
@@ -744,7 +746,7 @@ _edje_vg_tree_apply_override(Edje_Vg_Tree *working,
node->gradient.stops[i].color.color_class;
if (cc)
node->gradient.stops[i].color.color_class =
- eina_stringshare_ref(cc);
+ eina_stringshare_add(cc);
}
}
}
@@ -772,7 +774,7 @@ _edje_vg_override_dup(const Edje_Vg_Override *src)
if (!dup) return NULL;
dup->target_name = src->target_name
- ? eina_stringshare_ref(src->target_name) : NULL;
+ ? eina_stringshare_add(src->target_name) : NULL;
dup->expected_type = src->expected_type;
dup->field_mask = src->field_mask;
@@ -789,7 +791,7 @@ _edje_vg_override_dup(const Edje_Vg_Override *src)
}
if (src->field_mask & EDJE_VG_OVR_COLOR_CLASS)
dup->payload.color.color_class = src->payload.color.color_class
- ? eina_stringshare_ref(src->payload.color.color_class)
+ ? eina_stringshare_add(src->payload.color.color_class)
: NULL;
if (src->field_mask & EDJE_VG_OVR_TRANSFORM)
@@ -800,7 +802,7 @@ _edje_vg_override_dup(const Edje_Vg_Override *src)
{
if (src->field_mask & EDJE_VG_OVR_PATH)
dup->payload.shape.path = src->payload.shape.path
- ? eina_stringshare_ref(src->payload.shape.path)
+ ? eina_stringshare_add(src->payload.shape.path)
: NULL;
if (src->field_mask & EDJE_VG_OVR_FILL_RAW)
@@ -811,14 +813,14 @@ _edje_vg_override_dup(const Edje_Vg_Override *src)
dup->payload.shape.fill.a = src->payload.shape.fill.a;
dup->payload.shape.gradient_ref =
src->payload.shape.gradient_ref
- ? eina_stringshare_ref(src->payload.shape.gradient_ref)
+ ? eina_stringshare_add(src->payload.shape.gradient_ref)
: NULL;
}
if (src->field_mask & EDJE_VG_OVR_FILL_CLASS)
dup->payload.shape.fill.color_class =
src->payload.shape.fill.color_class
- ? eina_stringshare_ref(src->payload.shape.fill.color_class)
+ ? eina_stringshare_add(src->payload.shape.fill.color_class)
: NULL;
if (src->field_mask & EDJE_VG_OVR_STROKE_RAW)
@@ -832,7 +834,7 @@ _edje_vg_override_dup(const Edje_Vg_Override *src)
if (src->field_mask & EDJE_VG_OVR_STROKE_CLASS)
dup->payload.shape.stroke_color.color_class =
src->payload.shape.stroke_color.color_class
- ? eina_stringshare_ref(src->payload.shape.stroke_color.color_class)
+ ? eina_stringshare_add(src->payload.shape.stroke_color.color_class)
: NULL;
if (src->field_mask & EDJE_VG_OVR_STROKE_WIDTH)
@@ -900,7 +902,7 @@ _edje_vg_override_dup(const Edje_Vg_Override *src)
dup->payload.gradient.stops[i].color.color_class;
if (cc)
dup->payload.gradient.stops[i].color.color_class =
- eina_stringshare_ref(cc);
+ eina_stringshare_add(cc);
}
}
}
@@ -1151,7 +1153,7 @@ _color_lerp_with_ref(Edje_Vg_Color_Binding *out_cb,
/* _edje_vg_color_lerp left a borrowed stepped pointer; acquire a ref. */
if (out_cb->color_class)
- out_cb->color_class = eina_stringshare_ref(out_cb->color_class);
+ out_cb->color_class = eina_stringshare_add(out_cb->color_class);
}
static void
@@ -1310,7 +1312,7 @@ _node_lerp_recursive(Edje_Vg_Node *out_node,
out_node->gradient.stops[i].color.color_class;
if (cc)
out_node->gradient.stops[i].color.color_class =
- eina_stringshare_ref(cc);
+ eina_stringshare_add(cc);
}
}
/* Spread: step — out holds B's value */
diff --git a/src/lib/edje/edje_vg_tree.h b/src/lib/edje/edje_vg_tree.h
index 4bc5bdaf1b..055842e93e 100644
--- a/src/lib/edje/edje_vg_tree.h
+++ b/src/lib/edje/edje_vg_tree.h
@@ -9,6 +9,37 @@
* Only the bare-minimum API needed by the Task 1.3 round-trip test is
* exposed here. The full helper surface (lookup, dup, override-apply,
* colour resolve, materialise) belongs to Phase 2.
+ *
+ * String-ownership convention (Eet-dictionary vs stringshare):
+ *
+ * The string fields on Edje_Vg_Tree, Edje_Vg_Node, Edje_Vg_Override and
+ * Edje_Vg_Color_Binding are typed `const char *` because they carry
+ * either of two ownership models depending on how the containing tree
+ * was produced:
+ *
+ * 1. Loaded from an .edj file (Eet decode) — strings are borrowed
+ * pointers into the file's mmap'd dictionary section. The struct
+ * does NOT own them; freeing them would corrupt the dictionary.
+ * These trees are freed with `free_strings = EINA_FALSE` (which
+ * skips the per-field eina_stringshare_del calls).
+ *
+ * 2. Produced by the parser, the dup helpers, the edit API, or any
+ * other in-memory manipulation — strings are stringshare-owned
+ * (always created via eina_stringshare_add, never raw strdup).
+ * These trees are freed with `free_strings = EINA_TRUE`.
+ *
+ * Rules of thumb for new code:
+ * * In dup paths, always use `eina_stringshare_add(src->field)` —
+ * NEVER `eina_stringshare_ref`. The `_ref` call requires the input
+ * to be a real stringshare handle; a dictionary-backed pointer
+ * will fail the magic check at runtime.
+ * * In free paths, the per-field `eina_stringshare_del` calls must
+ * be gated on `free_strings`. The struct field type cannot tell
+ * you which ownership applies — only the caller knows.
+ * * In code that always operates on a stringshare-owned tree (lerp,
+ * resolve, materialise, edit-API setters), call eina_stringshare_*
+ * directly without any flag — the invariant is that working trees
+ * are dups, hence stringshare-owned.
*/
/* --- Test fixture -------------------------------------------------------- */
@@ -77,11 +108,13 @@ EAPI Efl_VG *_edje_vg_tree_to_efl_vg(const Edje_Vg_Tree *t);
/* --- Phase 4.1 helpers --------------------------------------------------- */
-/* Deep-clone the in-memory tree. Strings are stringshare-shared with the
- * source via eina_stringshare_ref where the source is already stringshared.
- * Container children are recursively duplicated; gradient stops and
- * stroke_dash arrays are memcpy'd into fresh allocations. The result is
- * fully independent of the source — mutating one does not affect the other. */
+/* Deep-clone the in-memory tree. Every string is copied into a fresh
+ * eina_stringshare_add'd handle so the dup is safe to free with
+ * free_strings=EINA_TRUE regardless of whether src came from an Eet
+ * dictionary mapping (.edj load) or from heap stringshares (parser /
+ * Edje_Edit). Container children are recursively duplicated; gradient
+ * stops and stroke_dash arrays are memcpy'd into fresh allocations.
+ * The result is fully independent of the source. */
EAPI Edje_Vg_Tree *_edje_vg_tree_dup(const Edje_Vg_Tree *t);
/* Apply a single override onto a working tree in place. The override's
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
