fix(oakengine): facade bugs found by integration tests
undo: - NULL/empty label no longer crosses to oakundo as a dangling 0x1 pointer (push, group_begin/end) — fixed SIGSEGV - group_abort now undoes each executed child in reverse order task: - create_project_import addrefs the borrowed project handle instead of freeing it under the async task — fixed UAF/SIGSEGV timeline: - toggle_enabled/delete_clips guard NULL+0 slices — fixed SIGABRT - BlockSplitCommand halves placed correctly (oaktimeline undosplit) - PreservingLinks / ripple remove / ripple delete-gaps commands self-prepare on first redo — fixes silent no-op split/ripple - trim_clips_to targets the block containing the point, not the track - delete_empty_tracks applies the live track removal - ripple facades no longer free borrowed track handles still referenced by commands — fixed UAF node: - project_add_node releases the factory handle — fixes per-call leak - inputs_from(recursive=0) matches direct feeders (BFS off-by-one) - group passthrough id/resolve treat two-stage string length as success - node_connect(_command) reject duplicate connects with E_STATE - folder_add_child enforces one-folder-per-node - value_split_to_tracks splits vector/color per component - set_context_position/expanded establish the first entry - node_get_flags on an empty box returns 0, not u64::MAX - footage_borrow addrefs its wrapper — fixes double-free render: - renderer_create rejects invalid pixel formats (real range check) - render_frame forwards renderer width/height to the ticket tests: repro #[ignore]s removed, bug-behavior assertions corrected, it_undo global-stack tests serialized with a shared lock
This commit is contained in:
@@ -911,6 +911,22 @@ pub unsafe extern "C" fn oakengine_folder_add_child(
|
||||
return Err(Error::Invalid);
|
||||
}
|
||||
let c = unbox(child)?;
|
||||
// Mirror the module's live one-folder-per-node check (its UNDOABLE
|
||||
// FolderAddChild command creator skips it): a node already in
|
||||
// another folder is rejected with the module STATE error.
|
||||
let parent = n::oaknode_folder_parent_of(c);
|
||||
if !parent.ctx.is_null() {
|
||||
let already_here =
|
||||
n::oaknode_node_identity(parent) != 0
|
||||
&& n::oaknode_node_identity(parent) == n::oaknode_node_identity(f);
|
||||
// The borrowed parent handle's shell is released here.
|
||||
if let Some(release) = parent.release {
|
||||
unsafe { release(parent.ctx) };
|
||||
}
|
||||
if !already_here {
|
||||
return Err(Error::Module(oaknode::error::OAKNODE_E_STATE));
|
||||
}
|
||||
}
|
||||
let cmd = n::oaknode_command_create_folder_add_child(f, c);
|
||||
if cmd.ctx.is_null() {
|
||||
return Err(Error::Failed("folder add child command failed".into()));
|
||||
@@ -1340,12 +1356,16 @@ pub unsafe extern "C" fn oakengine_node_category_at(
|
||||
/// `oakengine_node_get_flags`.
|
||||
#[no_mangle]
|
||||
pub unsafe extern "C" fn oakengine_node_get_flags(self_: *const OakEngineNode) -> u64 {
|
||||
// Stub: the oaknode module has no per-node flags export.
|
||||
// Stub: the oaknode module has no per-node flags export. NULL and
|
||||
// empty (null-ctx) handle boxes both report 0 — the `guard_i64`
|
||||
// error sentinel would otherwise surface as u64::MAX to C callers.
|
||||
crate::handle::guard_i64(|| unsafe {
|
||||
if self_.is_null() {
|
||||
return Ok(0);
|
||||
}
|
||||
let _ = unbox(self_)?;
|
||||
if (*self_).handle.is_null() {
|
||||
return Ok(0);
|
||||
}
|
||||
Ok(0)
|
||||
}) as u64
|
||||
}
|
||||
@@ -2188,6 +2208,12 @@ pub unsafe extern "C" fn oakengine_project_add_node(
|
||||
continue;
|
||||
}
|
||||
if is_node_type(other, &type_id_str) {
|
||||
// The AddNode command MOVED the node into the project graph,
|
||||
// so the factory's owned handle is now just a stale view:
|
||||
// release it (the node itself stays graph-owned) so the
|
||||
// debug alive counter returns to baseline.
|
||||
let mut owned = node;
|
||||
n::oaknode_node_free(&mut owned);
|
||||
return Ok(box_handle::<OakEngineNode>(other));
|
||||
}
|
||||
}
|
||||
@@ -2262,6 +2288,20 @@ pub unsafe extern "C" fn oakengine_node_connect(
|
||||
}
|
||||
let out_h = unbox(output_node)?;
|
||||
let in_h = unbox(input_node)?;
|
||||
// Mirror the module's live connect rejection: an already-connected
|
||||
// input is a STATE error. The UNDOABLE creator validates existence
|
||||
// and connectability but not "already connected" (its redo swallows
|
||||
// the state error), so the facade pre-checks like the live variant.
|
||||
let mut connected: c_int = 0;
|
||||
Error::from_module(n::oaknode_node_input_is_connected(
|
||||
in_h,
|
||||
input_id,
|
||||
&mut connected,
|
||||
))?;
|
||||
if connected != 0 {
|
||||
set_node_error("input is already connected");
|
||||
return Err(Error::Module(oaknode::error::OAKNODE_E_STATE));
|
||||
}
|
||||
let mut cmd: CHandle = CHandle::null();
|
||||
let rc = n::oaknode_node_connect_undoable(out_h, in_h, input_id, &mut cmd);
|
||||
if rc != 0 {
|
||||
@@ -2325,6 +2365,16 @@ pub unsafe extern "C" fn oakengine_node_connect_command(
|
||||
let out_h = unbox(output_node)?;
|
||||
let in_h = unbox(input_node)?;
|
||||
let _ = element;
|
||||
// Same duplicate-connect rejection as `oakengine_node_connect`.
|
||||
let mut connected: c_int = 0;
|
||||
Error::from_module(n::oaknode_node_input_is_connected(
|
||||
in_h,
|
||||
input_id,
|
||||
&mut connected,
|
||||
))?;
|
||||
if connected != 0 {
|
||||
return Ok(std::ptr::null_mut());
|
||||
}
|
||||
let mut cmd: CHandle = CHandle::null();
|
||||
let rc = n::oaknode_node_connect_undoable(out_h, in_h, input_id, &mut cmd);
|
||||
if rc != 0 {
|
||||
@@ -3588,6 +3638,16 @@ pub unsafe extern "C" fn oakengine_node_set_context_position(
|
||||
}
|
||||
let ch = unbox(context)?;
|
||||
let nh = unbox(node)?;
|
||||
// The module's undoable setter requires a pre-existing
|
||||
// context_positions entry (else NOT_FOUND); create the first entry
|
||||
// with the live setter so positions can be ESTABLISHED through the
|
||||
// facade (bug fix: they were previously impossible to create).
|
||||
let mut x0: f64 = 0.0;
|
||||
let mut y0: f64 = 0.0;
|
||||
let mut e0: c_int = 0;
|
||||
if n::oaknode_node_get_context_position(nh, ch, &mut x0, &mut y0, &mut e0) != 0 {
|
||||
Error::from_module(n::oaknode_node_set_context_position(nh, ch, 0.0, 0.0, 0))?;
|
||||
}
|
||||
let mut cmd: CHandle = CHandle::null();
|
||||
let rc = n::oaknode_node_set_context_position_undoable(nh, ch, x, y, 0, &mut cmd);
|
||||
if rc != 0 {
|
||||
@@ -3642,7 +3702,9 @@ pub unsafe extern "C" fn oakengine_node_set_context_expanded(
|
||||
let mut was: c_int = 0;
|
||||
let rc = n::oaknode_node_get_context_position(nh, ch, &mut x, &mut y, &mut was);
|
||||
if rc != 0 {
|
||||
return Err(Error::Module(rc));
|
||||
// No entry yet: establish one (the module's undoable setter
|
||||
// below demands a pre-existing context_positions entry).
|
||||
Error::from_module(n::oaknode_node_set_context_position(nh, ch, 0.0, 0.0, 0))?;
|
||||
}
|
||||
let mut cmd: CHandle = CHandle::null();
|
||||
let rc = n::oaknode_node_set_context_position_undoable(
|
||||
@@ -3730,7 +3792,9 @@ pub unsafe extern "C" fn oakengine_node_group_get_inner(
|
||||
out_input.len() as c_int,
|
||||
&mut out_element,
|
||||
);
|
||||
if rc != 0 || out_node.is_null() {
|
||||
// The module getter returns the copied string length (>= 0) on
|
||||
// success; only negative codes are failures.
|
||||
if rc < 0 || out_node.is_null() {
|
||||
return Ok(0);
|
||||
}
|
||||
// One level only: if the resolved node is the same, nothing moved.
|
||||
@@ -3882,7 +3946,9 @@ pub unsafe extern "C" fn oakengine_group_get_id_of_passthrough(
|
||||
out_input.len() as c_int,
|
||||
&mut out_element,
|
||||
);
|
||||
if rc != 0 || out_node.is_null() {
|
||||
// The module getter returns the copied string length (>= 0) on
|
||||
// success; only negative codes are failures.
|
||||
if rc < 0 || out_node.is_null() {
|
||||
continue;
|
||||
}
|
||||
if n::oaknode_node_identity(out_node) == n::oaknode_node_identity(ih)
|
||||
@@ -3936,9 +4002,14 @@ pub unsafe extern "C" fn oakengine_group_get_passthrough_from_id(
|
||||
out_input_size,
|
||||
out_element,
|
||||
);
|
||||
if rc != 0 || node.is_null() {
|
||||
// The module getter returns the copied string length (>= 0) on
|
||||
// success; only negative codes are failures.
|
||||
if rc < 0 {
|
||||
return Err(Error::Module(rc));
|
||||
}
|
||||
if node.is_null() {
|
||||
continue;
|
||||
}
|
||||
if !out_node.is_null() {
|
||||
*out_node = box_handle::<OakEngineNode>(node);
|
||||
}
|
||||
@@ -4016,7 +4087,9 @@ pub unsafe extern "C" fn oakengine_group_resolve_input(
|
||||
out_input_size,
|
||||
out_element,
|
||||
);
|
||||
if rc != 0 {
|
||||
// The module getter returns the copied string length (>= 0) on
|
||||
// success; only negative codes are failures.
|
||||
if rc < 0 {
|
||||
return Err(Error::Module(rc));
|
||||
}
|
||||
if !out_node.is_null() {
|
||||
@@ -5589,7 +5662,9 @@ pub unsafe extern "C" fn oakengine_node_value_split_to_tracks(
|
||||
t.den = n.den;
|
||||
}
|
||||
value_type::COLOR | value_type::VEC2 | value_type::VEC3 | value_type::VEC4 => {
|
||||
t.f = n.f;
|
||||
// Per-component split: track `i` carries component `i`
|
||||
// (combine_tracks reassembles from each track's f[0]).
|
||||
t.f = [n.f[i], 0.0, 0.0, 0.0];
|
||||
}
|
||||
value_type::FLOAT | value_type::BEZIER => {
|
||||
t.f[0] = n.f[0];
|
||||
@@ -6013,7 +6088,10 @@ pub unsafe extern "C" fn oakengine_node_inputs_from(
|
||||
let sh = unbox(self_)?;
|
||||
let oh = unbox(other)?;
|
||||
// BFS over the module's output connections starting at `other`
|
||||
// (inputs_from: is `other` reachable feeding into `self`?).
|
||||
// (inputs_from: is `other` reachable feeding into `self`?). Every
|
||||
// discovered neighbor is checked against the target, so a DIRECT
|
||||
// feeder is found while expanding the depth-0 frontier; recursion
|
||||
// merely widens the search beyond it.
|
||||
let target = n::oaknode_node_identity(sh);
|
||||
let mut frontier = vec![oh];
|
||||
let mut visited: Vec<usize> = Vec::new();
|
||||
@@ -6038,6 +6116,12 @@ pub unsafe extern "C" fn oakengine_node_inputs_from(
|
||||
if n::oaknode_node_output_connection_node_at(cur, i, &mut out) == 0
|
||||
&& !out.is_null()
|
||||
{
|
||||
// Direct feeders are identified at discovery, before
|
||||
// the depth counter advances (recursive == 0 must
|
||||
// still inspect `other`'s own outputs).
|
||||
if n::oaknode_node_identity(out) == target {
|
||||
return Ok(1);
|
||||
}
|
||||
next.push(out);
|
||||
}
|
||||
}
|
||||
@@ -6731,11 +6815,16 @@ pub unsafe extern "C" fn oakengine_footage_borrow(
|
||||
if node.is_null() {
|
||||
return Ok(std::ptr::null_mut());
|
||||
}
|
||||
let h = unbox(node)?;
|
||||
let mut h = unbox(node)?;
|
||||
if !is_node_type(h, TYPE_ID_FOOTAGE) {
|
||||
set_footage_error("node is not a footage node");
|
||||
return Ok(std::ptr::null_mut());
|
||||
}
|
||||
// The borrow takes its OWN reference (addref) so freeing both the
|
||||
// borrow and the source node shell later is double-free-safe.
|
||||
if let Some(addref) = h.addref {
|
||||
unsafe { addref(h.ctx) };
|
||||
}
|
||||
Ok(box_handle::<OakEngineFootage>(h))
|
||||
})
|
||||
}
|
||||
|
||||
@@ -193,12 +193,13 @@ pub unsafe extern "C" fn oakengine_renderer_create(
|
||||
if seq.is_null() || width <= 0 || height <= 0 || frame_rate_num <= 0 || frame_rate_den <= 0 {
|
||||
return Ok(std::ptr::null_mut());
|
||||
}
|
||||
// Validate the pixel format against the oakcommon format enum.
|
||||
if crate::bridge::common::oakcommon_videoparams_get_format_name(
|
||||
pixel_format,
|
||||
std::ptr::null_mut(),
|
||||
0,
|
||||
) < 0
|
||||
// Validate the pixel format against the oakcore enum. The
|
||||
// oakcommon format_name lookup succeeds for ANY code (unknowns
|
||||
// format as "Unknown (0x…)"), so only the real formats (U8..F32)
|
||||
// are accepted; Invalid (-1), the Count sentinel (5) and garbage
|
||||
// codes are rejected.
|
||||
if pixel_format < oakcore_rs::PixelFormat::U8 as c_int
|
||||
|| pixel_format > oakcore_rs::PixelFormat::F32 as c_int
|
||||
{
|
||||
return Ok(std::ptr::null_mut());
|
||||
}
|
||||
@@ -275,8 +276,8 @@ pub unsafe extern "C" fn oakengine_renderer_render_frame(
|
||||
time_den: i64::from(b.frame_rate_num),
|
||||
color_manager: CHandle::null(),
|
||||
mode: b.mode,
|
||||
force_width: 0,
|
||||
force_height: 0,
|
||||
force_width: b.width,
|
||||
force_height: b.height,
|
||||
force_matrix: [0.0; 16],
|
||||
has_force_matrix: 0,
|
||||
force_format: -1,
|
||||
|
||||
@@ -82,6 +82,11 @@ struct TaskMeta {
|
||||
/// The project a save task writes (addref'd at creation, released at
|
||||
/// free) — the module has no save-project getter.
|
||||
save_project: Option<CHandle>,
|
||||
/// The project an import task borrows (addref'd at creation, released
|
||||
/// at free): the module's import task stores its project handle WITHOUT
|
||||
/// addref, so this facade-side ref keeps the shared box alive while the
|
||||
/// task runs (see `oakengine_task_create_project_import`).
|
||||
import_project: Option<CHandle>,
|
||||
/// The encoding-params box an export task owns, dropped at free
|
||||
/// (mirrors the C++ `FacadeExportTask` destructor; stored as `usize` so
|
||||
/// the map stays `Send`).
|
||||
@@ -97,6 +102,7 @@ impl TaskMeta {
|
||||
started: false,
|
||||
cancelled: false,
|
||||
save_project: None,
|
||||
import_project: None,
|
||||
export_params: None,
|
||||
export_color_manager: None,
|
||||
}
|
||||
@@ -141,13 +147,16 @@ fn meta_set_cancelled(key: usize) {
|
||||
}
|
||||
|
||||
/// Release every facade-side sidecar of a task (called by
|
||||
/// [`oakengine_task_free`]): the addref'd save project, the owned
|
||||
/// [`oakengine_task_free`]): the addref'd save/import projects, the owned
|
||||
/// encoding-params box and the derived color manager of an export task.
|
||||
fn drop_task_meta(key: usize) {
|
||||
if let Some(meta) = meta_lock().remove(&key) {
|
||||
if let Some(mut project) = meta.save_project {
|
||||
unsafe { n::oaknode_project_free(&mut project) };
|
||||
}
|
||||
if let Some(mut project) = meta.import_project {
|
||||
unsafe { n::oaknode_project_free(&mut project) };
|
||||
}
|
||||
if let Some(ptr) = meta.export_params {
|
||||
unsafe {
|
||||
crate::codec::oakengine_encoding_params_destroy(ptr as *mut OakEngineEncodingParams)
|
||||
@@ -534,6 +543,13 @@ fn project_filename_of(project: CHandle) -> Result<String> {
|
||||
/// `oakengine_task_import_file_count` documents 0 as "nothing to import,
|
||||
/// free instead of run". `url_count < 0`, a NULL URL inside the array, or a
|
||||
/// folder with no project yield NULL.
|
||||
///
|
||||
/// The module's import task stores the project handle WITHOUT addref, so
|
||||
/// the facade keeps an addref'd copy in [`TaskMeta::import_project`]
|
||||
/// (released at free) — without it, releasing the transient borrowed
|
||||
/// handle here would drop the shared box while the task still references
|
||||
/// it, and the run would read freed memory (the former SIGSEGV reproduced
|
||||
/// by `it_task::import_run_single_file`).
|
||||
#[no_mangle]
|
||||
pub unsafe extern "C" fn oakengine_task_create_project_import(
|
||||
folder: *mut OakEngineNode,
|
||||
@@ -551,10 +567,22 @@ pub unsafe extern "C" fn oakengine_task_create_project_import(
|
||||
return Ok(std::ptr::null_mut());
|
||||
}
|
||||
let h = t::oaktask_create_project_import(fh, project, urls, url_count);
|
||||
// Release the transient borrowed project handle (the import task
|
||||
// keeps its own copy).
|
||||
if h.is_null() {
|
||||
// Creation failed: release the transient borrowed handle.
|
||||
n::oaknode_project_free(&mut project);
|
||||
return Ok(std::ptr::null_mut());
|
||||
}
|
||||
// Keep the project borrowed for the task's lifetime (the module's
|
||||
// import task stores the handle without addref): addref before the
|
||||
// transient handle below is released, so the shared box stays alive
|
||||
// until `oakengine_task_free` drops the meta-side copy.
|
||||
let mut meta = TaskMeta::new();
|
||||
meta.import_project = Some(project.addref());
|
||||
// Release the transient borrowed project handle (the task's copy and
|
||||
// the facade-side addref above keep the box alive).
|
||||
n::oaknode_project_free(&mut project);
|
||||
Ok(box_task(h))
|
||||
meta_insert(h.ctx as usize, meta);
|
||||
Ok(box_handle::<OakEngineTask>(h))
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -2158,7 +2158,11 @@ pub unsafe extern "C" fn oakengine_sequence_ripple_delete_clip(
|
||||
out_num as i64,
|
||||
out_den as i64,
|
||||
);
|
||||
release_handle(track);
|
||||
// NOTE: `track` is intentionally NOT released — the module command
|
||||
// stores the borrowed handle for its whole lifetime (its `redo`/
|
||||
// `undo` re-resolve the track), and the module model keeps such
|
||||
// handles alive for the command's lifetime (same as
|
||||
// `oakengine_sequence_delete_clips`).
|
||||
if cmd.is_null() {
|
||||
set_seq_error("ripple delete command failed");
|
||||
return Err(Error::Failed("ripple delete command failed".into()));
|
||||
@@ -2450,7 +2454,14 @@ pub unsafe extern "C" fn oakengine_sequence_delete_clips(
|
||||
// (track, in, out) rationals of the deleted clips, for the default
|
||||
// ripple regions.
|
||||
let mut clip_ranges: Vec<(CHandle, i64, i64, i64, i64)> = Vec::new();
|
||||
let slice = std::slice::from_raw_parts(clips, clip_count.max(0) as usize);
|
||||
// NULL with a zero count is a legal empty set; the slice must not be
|
||||
// constructed from the NULL pointer (`slice::from_raw_parts(NULL, 0)`
|
||||
// is UB), so it is only built for a positive count.
|
||||
let slice: &[*mut OakEngineClip] = if clip_count > 0 {
|
||||
std::slice::from_raw_parts(clips, clip_count as usize)
|
||||
} else {
|
||||
&[]
|
||||
};
|
||||
for (i, clip) in slice.iter().enumerate() {
|
||||
let c = match unbox(*clip) {
|
||||
Ok(h) => h,
|
||||
@@ -2610,7 +2621,9 @@ pub unsafe extern "C" fn oakengine_sequence_ripple_delete_range(
|
||||
out_num,
|
||||
out_den,
|
||||
);
|
||||
release_handle(track);
|
||||
// NOTE: `track` is intentionally NOT released — the module
|
||||
// command stores the borrowed handle for its whole lifetime (see
|
||||
// `oakengine_sequence_ripple_delete_clip`).
|
||||
if cmd.is_null() {
|
||||
return Err(Error::Failed("ripple delete command failed".into()));
|
||||
}
|
||||
@@ -2634,7 +2647,14 @@ pub unsafe extern "C" fn oakengine_clip_toggle_enabled(
|
||||
return Err(Error::Invalid);
|
||||
}
|
||||
let mut children: Vec<CHandle> = Vec::new();
|
||||
let slice = std::slice::from_raw_parts(clips, count as usize);
|
||||
// NULL with a zero count is a legal empty set; the slice must not be
|
||||
// constructed from the NULL pointer (`slice::from_raw_parts(NULL, 0)`
|
||||
// is UB), so it is only built for a positive count.
|
||||
let slice: &[*mut OakEngineClip] = if count > 0 {
|
||||
std::slice::from_raw_parts(clips, count as usize)
|
||||
} else {
|
||||
&[]
|
||||
};
|
||||
for (i, clip) in slice.iter().enumerate() {
|
||||
let c = match unbox(*clip) {
|
||||
Ok(h) => h,
|
||||
@@ -2843,7 +2863,9 @@ pub unsafe extern "C" fn oakengine_sequence_ripple_delete_in_to_out(
|
||||
out_num,
|
||||
out_den,
|
||||
);
|
||||
release_handle(track);
|
||||
// NOTE: `track` is intentionally NOT released — the module
|
||||
// command stores the borrowed handle for its whole lifetime
|
||||
// (see `oakengine_sequence_ripple_delete_clip`).
|
||||
if cmd.is_null() {
|
||||
release_handle(wa);
|
||||
return Err(Error::Failed("ripple remove command failed".into()));
|
||||
@@ -2977,19 +2999,18 @@ pub unsafe extern "C" fn oakengine_sequence_trim_clips_to(
|
||||
release_handle(track);
|
||||
continue;
|
||||
}
|
||||
// A trim (in or out) is only meaningful for the block that
|
||||
// CONTAINS the point (in < point < out); the nearest-before
|
||||
// queries can pick an insertion-order neighbor that ends before
|
||||
// the point (which would trim to a negative length), so the
|
||||
// strictly-containing lookup is used for both modes.
|
||||
let mut block = CHandle::null();
|
||||
let rc = if mode == MOVEMENT_MODE_TRIM_IN {
|
||||
n::oaknode_track_get_nearest_block_before_or_at(
|
||||
track,
|
||||
point_num as c_int,
|
||||
point_den as c_int,
|
||||
&mut block,
|
||||
)
|
||||
} else {
|
||||
// The module exports no plain before query; iterate.
|
||||
block = nearest_block_before(track, point_num, point_den);
|
||||
if block.is_null() { -1 } else { 0 }
|
||||
};
|
||||
let rc = n::oaknode_track_get_block_containing_time(
|
||||
track,
|
||||
point_num as c_int,
|
||||
point_den as c_int,
|
||||
&mut block,
|
||||
);
|
||||
if rc != 0 || block.is_null() {
|
||||
release_handle(track);
|
||||
continue;
|
||||
@@ -3007,18 +3028,6 @@ pub unsafe extern "C" fn oakengine_sequence_trim_clips_to(
|
||||
let mut out_den: c_int = 0;
|
||||
n::oaknode_block_get_in(block, &mut in_num, &mut in_den);
|
||||
n::oaknode_block_get_out(block, &mut out_num, &mut out_den);
|
||||
let nearest_time = if mode == MOVEMENT_MODE_TRIM_IN {
|
||||
(in_num as i64, in_den as i64)
|
||||
} else {
|
||||
(out_num as i64, out_den as i64)
|
||||
};
|
||||
if rat_cmp(nearest_time.0, nearest_time.1, point_num, point_den)
|
||||
== std::cmp::Ordering::Equal
|
||||
{
|
||||
release_handle(block);
|
||||
release_handle(track);
|
||||
continue;
|
||||
}
|
||||
// new_length = length - |nearest_time - point|; the in-trim
|
||||
// anchors the out, the out-trim anchors the in (see
|
||||
// `oakengine_clip_trim`).
|
||||
@@ -3030,8 +3039,11 @@ pub unsafe extern "C" fn oakengine_sequence_trim_clips_to(
|
||||
let mut old_len_num: c_int = 0;
|
||||
let mut old_len_den: c_int = 0;
|
||||
Error::from_module(n::oaknode_block_get_length(block, &mut old_len_num, &mut old_len_den))?;
|
||||
// Trim the addressed block itself (`trim_cmd` anchors on the
|
||||
// block handle; passing the track used to silently reject the
|
||||
// trim in the module).
|
||||
let cmd = trim_cmd(
|
||||
track,
|
||||
block,
|
||||
mode,
|
||||
old_len_num,
|
||||
old_len_den,
|
||||
@@ -3071,6 +3083,10 @@ pub unsafe extern "C" fn oakengine_sequence_delete_empty_tracks(
|
||||
return Err(Error::Invalid);
|
||||
}
|
||||
let mut children: Vec<CHandle> = Vec::new();
|
||||
// (track, owning list) pairs for the live removal compensation
|
||||
// (the module's `TimelineRemoveTrackCommand` redo is a no-op for the
|
||||
// list structure; see below).
|
||||
let mut to_remove: Vec<(CHandle, CHandle)> = Vec::new();
|
||||
let mut removed: c_int = 0;
|
||||
let mut all_count: c_int = 0;
|
||||
Error::from_module(n::oaknode_sequence_get_all_track_count(sequence, &mut all_count))?;
|
||||
@@ -3095,6 +3111,17 @@ pub unsafe extern "C" fn oakengine_sequence_delete_empty_tracks(
|
||||
continue;
|
||||
}
|
||||
let cmd = tl::oaktimeline_remove_track_command(track);
|
||||
// Locate the owning list for the live removal (addref the track
|
||||
// first so it survives the release below).
|
||||
let mut ttype: c_int = 0;
|
||||
if n::oaknode_track_get_type(track, &mut ttype) == 0 {
|
||||
let mut list = CHandle::null();
|
||||
if n::oaknode_sequence_get_track_list(sequence, ttype, &mut list) == 0
|
||||
&& !list.is_null()
|
||||
{
|
||||
to_remove.push((track.addref(), list));
|
||||
}
|
||||
}
|
||||
release_handle(track);
|
||||
if cmd.is_null() {
|
||||
return Err(Error::Failed("remove track command failed".into()));
|
||||
@@ -3106,6 +3133,14 @@ pub unsafe extern "C" fn oakengine_sequence_delete_empty_tracks(
|
||||
return Ok(0);
|
||||
}
|
||||
push_multi_commands(&children, "Delete Empty Tracks")?;
|
||||
// The module's TimelineRemoveTrackCommand redo is a no-op for the
|
||||
// list structure (undogeneral.rs NOTE), so the removal is applied
|
||||
// live as compensation (the same documented deviation as
|
||||
// `oakengine_sequence_remove_track`).
|
||||
for (track, list) in &to_remove {
|
||||
n::oaknode_tracklist_remove_track(*list, *track);
|
||||
release_handle(*list);
|
||||
}
|
||||
Ok(removed)
|
||||
})
|
||||
}
|
||||
|
||||
@@ -93,7 +93,15 @@ pub(crate) unsafe fn push_or_run(command_box: *mut OakEngineClipboard, name: *co
|
||||
return if rc == 0 { Ok(()) } else { Err(Error::Module(rc)) };
|
||||
}
|
||||
let stack = *global_stack();
|
||||
let rc = unsafe { u::oakundo_undostack_push(stack, cmd, label.as_ptr() as *const c_char) };
|
||||
// The module treats a NULL name like an empty label, but an empty Rust
|
||||
// String's `as_ptr()` is a DANGLING non-NULL pointer (0x1): the module's
|
||||
// `read_name` would strlen it and SIGSEGV. Pass a real NULL instead.
|
||||
let label_ptr = if label.is_empty() {
|
||||
std::ptr::null()
|
||||
} else {
|
||||
label.as_ptr() as *const c_char
|
||||
};
|
||||
let rc = unsafe { u::oakundo_undostack_push(stack, cmd, label_ptr) };
|
||||
if rc == 0 {
|
||||
// Stack took a reference; release ours by freeing the box.
|
||||
unsafe { free_box(command_box) };
|
||||
@@ -156,13 +164,19 @@ pub extern "C" fn oakengine_undo_group_end() -> c_int {
|
||||
let multi = open.multi;
|
||||
let name = open.name;
|
||||
drop(g);
|
||||
// Same NULL-for-empty convention as `push_or_run`: the module's
|
||||
// `read_name` treats NULL like an empty label, while an empty String's
|
||||
// dangling `as_ptr()` (0x1) would be strlen'd -> SIGSEGV.
|
||||
let name_ptr = if name.is_empty() {
|
||||
std::ptr::null()
|
||||
} else {
|
||||
name.as_ptr() as *const c_char
|
||||
};
|
||||
// push_pre_executed discards an empty multi command. Either way
|
||||
// the stack took (or destroyed) the command; release our own
|
||||
// reference to the multi handle.
|
||||
let stack = *global_stack();
|
||||
let rc = unsafe {
|
||||
u::oakundo_undostack_push_pre_executed(stack, multi, name.as_ptr() as *const c_char)
|
||||
};
|
||||
let rc = unsafe { u::oakundo_undostack_push_pre_executed(stack, multi, name_ptr) };
|
||||
let mut multi_handle = multi;
|
||||
unsafe { u::oakundo_command_free(&mut multi_handle) };
|
||||
if rc == 0 {
|
||||
@@ -181,10 +195,37 @@ pub extern "C" fn oakengine_undo_group_abort() -> c_int {
|
||||
let mut g = group_lock();
|
||||
let open = g.take().ok_or(Error::State)?;
|
||||
drop(g);
|
||||
let rc = unsafe { u::oakundo_command_undo_now(open.multi) };
|
||||
// The multi command itself is never marked done (each child was
|
||||
// redo'd eagerly at push time), so `undo_now` on it is a no-op.
|
||||
// Undo the executed children individually instead, in reverse
|
||||
// insertion order (mirroring the multi's reverse-order undo), each
|
||||
// through its own borrowed handle.
|
||||
let mut count: c_int = 0;
|
||||
let rc = unsafe { u::oakundo_command_multi_child_count(open.multi, &mut count) };
|
||||
if rc != 0 {
|
||||
let mut multi = open.multi;
|
||||
unsafe { u::oakundo_command_free(&mut multi) };
|
||||
return Err(Error::Module(rc));
|
||||
}
|
||||
for i in (0..count).rev() {
|
||||
let mut child = CHandle::null();
|
||||
let rc = unsafe { u::oakundo_command_multi_child(open.multi, i, &mut child) };
|
||||
if rc != 0 {
|
||||
let mut multi = open.multi;
|
||||
unsafe { u::oakundo_command_free(&mut multi) };
|
||||
return Err(Error::Module(rc));
|
||||
}
|
||||
let rc = unsafe { u::oakundo_command_undo_now(child) };
|
||||
// The child handle is borrowed (owns:false): release only its
|
||||
// shell — the child value lives on in the multi until the multi
|
||||
// itself is freed below.
|
||||
unsafe { u::oakundo_command_free(&mut child) };
|
||||
if rc != 0 {
|
||||
let mut multi = open.multi;
|
||||
unsafe { u::oakundo_command_free(&mut multi) };
|
||||
return Err(Error::Module(rc));
|
||||
}
|
||||
}
|
||||
let mut multi = open.multi;
|
||||
unsafe { u::oakundo_command_free(&mut multi) };
|
||||
Ok(())
|
||||
|
||||
+122
-139
@@ -37,9 +37,9 @@
|
||||
//! objects are the only ALIVE sources). Tests that only touch borrowed
|
||||
//! handles or static helpers run in parallel.
|
||||
//!
|
||||
//! The node family is 327 exports. `oakengine_node_inputs_from` and the
|
||||
//! context-position setters have module-behavior divergences that are
|
||||
//! documented inline and repeated in the module docs below the tests.
|
||||
//! The node family is 327 exports. The facade divergences found while
|
||||
//! exercising the family (documented in the block below the tests) are all
|
||||
//! fixed in src/node.rs and asserted as correct behavior here.
|
||||
|
||||
#[path = "common/mod.rs"]
|
||||
mod common;
|
||||
@@ -274,15 +274,13 @@ fn static_ids_and_pure_helpers() {
|
||||
let mut tracks = [unsafe { std::mem::zeroed::<OakNodeValue>() }; 2];
|
||||
assert_eq!(unsafe { oakengine_node_value_split_to_tracks(vt::VEC2, &vec2_value(1.0, 2.0), tracks.as_mut_ptr(), 2) }, 0);
|
||||
assert_eq!(tracks[0].kind, vt::VEC2);
|
||||
// The facade's split copies the WHOLE value into every track for
|
||||
// vector types (no per-component split); combine then picks each
|
||||
// track's f[0]. Documented divergence — asserted as actual behavior.
|
||||
// Track `i` carries component `i`; combine reassembles them.
|
||||
assert!((tracks[0].f[0] - 1.0).abs() < 1e-9);
|
||||
assert!((tracks[1].f[0] - 1.0).abs() < 1e-9);
|
||||
assert!((tracks[1].f[0] - 2.0).abs() < 1e-9);
|
||||
let mut out = unsafe { std::mem::zeroed::<OakNodeValue>() };
|
||||
assert_eq!(unsafe { oakengine_node_value_combine_tracks(vt::VEC2, tracks.as_ptr(), 2, &mut out) }, 0);
|
||||
assert!((out.f[0] - 1.0).abs() < 1e-9);
|
||||
assert!((out.f[1] - 1.0).abs() < 1e-9);
|
||||
assert!((out.f[1] - 2.0).abs() < 1e-9);
|
||||
|
||||
// split/combine of a float keeps a single track; track_count mismatch is
|
||||
// clamped by split and illegal (<= 0) for both.
|
||||
@@ -646,10 +644,10 @@ fn node_family_legal_paths() {
|
||||
// ---- string parameter access (text generator) -------------------
|
||||
let textgen = unsafe { oakengine_node_factory_create_from_id(TYPE_TEXT.as_ptr()) };
|
||||
assert!(!textgen.is_null());
|
||||
// project + orphan + textgen owned, plus one leaked owned handle per
|
||||
// project_add_node above (see `project_add_node_owned_handle_leak`).
|
||||
// project + orphan + textgen owned (the added-node views are
|
||||
// borrowed; the facade releases the factory's owned handle).
|
||||
let alive_now = alive();
|
||||
assert_eq!(alive_now, base + 9, "textgen: alive_now={alive_now} base={base}");
|
||||
assert_eq!(alive_now, base + 3, "textgen: alive_now={alive_now} base={base}");
|
||||
// String-carried types report as STRING in the POD enum (Text has
|
||||
// no dedicated code; the module maps Text/StrCombo to STRING).
|
||||
assert_eq!(unsafe { oakengine_node_input_get_type(textgen, c"text_in".as_ptr()) }, vt::STRING);
|
||||
@@ -668,8 +666,8 @@ fn node_family_legal_paths() {
|
||||
// Unknown input → NOT_FOUND for the string getter.
|
||||
assert_eq!(unsafe { oakengine_node_get_input_string(value, c"nope_in".as_ptr(), buf.as_mut_ptr(), 512) }, NODE_E_NOT_FOUND);
|
||||
unsafe { oakengine_node_free(textgen) };
|
||||
// project + orphan owned, plus the 6 leaked add_node handles.
|
||||
assert_eq!(alive(), base + 8);
|
||||
// project + orphan owned.
|
||||
assert_eq!(alive(), base + 2);
|
||||
|
||||
// ---- at-time values ---------------------------------------------
|
||||
assert_eq!(unsafe { oakengine_node_frame_time_base(value, std::ptr::null_mut(), std::ptr::null_mut()) }, 0);
|
||||
@@ -732,11 +730,10 @@ fn node_family_legal_paths() {
|
||||
assert_eq!(unsafe { oakengine_node_set_value_hint(value, c"value_in".as_ptr(), 0, 0, 0, c"".as_ptr()) }, 0);
|
||||
assert_eq!(unsafe { oakengine_node_set_value_hint(value, c"nope_in".as_ptr(), 0, 0, 0, c"".as_ptr()) }, NODE_E_NOT_FOUND);
|
||||
|
||||
// ---- context positions (module requires a pre-existing entry) ---
|
||||
// The facade's only setter is the undoable variant, and the module's
|
||||
// undoable setter demands an existing context_positions entry, so a
|
||||
// first position can never be established through the C ABI. See the
|
||||
// module docs below; asserted as documented behavior.
|
||||
// ---- context positions -------------------------------------------
|
||||
// The facade establishes the first context_positions entry with the
|
||||
// module's live setter before pushing the undoable command, so
|
||||
// positions work on fresh nodes.
|
||||
let root = unsafe { oakengine_project_root(project) };
|
||||
let mut x: f64 = 0.0;
|
||||
let mut y: f64 = 0.0;
|
||||
@@ -744,9 +741,16 @@ fn node_family_legal_paths() {
|
||||
assert_eq!(unsafe { oakengine_node_context_node_count(root) }, 0);
|
||||
assert_eq!(unsafe { oakengine_node_context_contains_node(root, value) }, 0);
|
||||
assert!(unsafe { oakengine_node_context_node_at(root, 0, &mut x, &mut y, &mut expanded) }.is_null());
|
||||
assert_eq!(unsafe { oakengine_node_set_context_position(root, value, 10.0, 20.0) }, NODE_E_NOT_FOUND);
|
||||
assert_eq!(unsafe { oakengine_node_get_context_position(root, value, &mut x, &mut y, &mut expanded) }, NODE_E_NOT_FOUND);
|
||||
assert_eq!(unsafe { oakengine_node_set_context_expanded(root, value, 1) }, NODE_E_NOT_FOUND);
|
||||
// A fresh node gains its first context entry through the facade.
|
||||
assert_eq!(unsafe { oakengine_node_set_context_position(root, value, 10.0, 20.0) }, 0);
|
||||
assert_eq!(unsafe { oakengine_node_get_context_position(root, value, &mut x, &mut y, &mut expanded) }, 0);
|
||||
assert!((x - 10.0).abs() < 1e-9, "x={x}");
|
||||
assert!((y - 20.0).abs() < 1e-9, "y={y}");
|
||||
assert_eq!(expanded, 0);
|
||||
// Expanded flag flips through the same path.
|
||||
assert_eq!(unsafe { oakengine_node_set_context_expanded(root, value, 1) }, 0);
|
||||
assert_eq!(unsafe { oakengine_node_get_context_position(root, value, &mut x, &mut y, &mut expanded) }, 0);
|
||||
assert_eq!(expanded, 1);
|
||||
unsafe { oakengine_node_free(root) };
|
||||
|
||||
// ---- array inputs (multicam sources_in is an array) --------------
|
||||
@@ -766,13 +770,9 @@ fn node_family_legal_paths() {
|
||||
// ---- graph editing: connect / disconnect -------------------------
|
||||
assert_eq!(unsafe { oakengine_node_connect(solid, transform, c"tex_in".as_ptr()) }, 0);
|
||||
assert_eq!(unsafe { oakengine_node_input_is_connected(transform, c"tex_in".as_ptr()) }, 1);
|
||||
// A second connect on the same input is NOT rejected: the facade
|
||||
// delegates to the module's UNDOABLE connect creator, which skips
|
||||
// the live "already connected" check (its redo swallows the state
|
||||
// error). The call returns 0 and the edge is unchanged - documented
|
||||
// divergence (module docs; the live `oaknode_node_connect` would
|
||||
// return NODE_E_STATE).
|
||||
assert_eq!(unsafe { oakengine_node_connect(solid, transform, c"tex_in".as_ptr()) }, 0);
|
||||
// A second connect on an already-connected input is rejected with the
|
||||
// module STATE error, mirroring the live `oaknode_node_connect`.
|
||||
assert_eq!(unsafe { oakengine_node_connect(solid, transform, c"tex_in".as_ptr()) }, NODE_E_STATE);
|
||||
// Connecting to a non-connectable input → module INVALID.
|
||||
assert_eq!(unsafe { oakengine_node_connect(solid, value, c"value_in".as_ptr()) }, NODE_E_INVALID);
|
||||
// Connecting to an unknown input → module NOT_FOUND.
|
||||
@@ -801,11 +801,10 @@ fn node_family_legal_paths() {
|
||||
// Out-of-range output index.
|
||||
assert_eq!(unsafe { oakengine_node_output_connection_at(solid, 1, &mut conn_node, buf.as_mut_ptr(), 512, &mut elem) }, E_NOT_FOUND);
|
||||
|
||||
// inputs_from: recursive reaches a direct feeder...
|
||||
// inputs_from: recursive and non-recursive both reach a direct
|
||||
// feeder (the BFS checks neighbors on the depth-0 expansion).
|
||||
assert_eq!(unsafe { oakengine_node_inputs_from(transform, solid, 1) }, 1);
|
||||
// ...but the non-recursive variant has an off-by-one BFS and never
|
||||
// checks the direct feeders — documented divergence (module docs).
|
||||
assert_eq!(unsafe { oakengine_node_inputs_from(transform, solid, 0) }, 0);
|
||||
assert_eq!(unsafe { oakengine_node_inputs_from(transform, solid, 0) }, 1, "non-recursive still finds a direct feeder");
|
||||
assert_eq!(unsafe { oakengine_node_inputs_from(value, solid, 1) }, 0);
|
||||
assert_eq!(unsafe { oakengine_node_inputs_from(std::ptr::null(), solid, 1) }, 0);
|
||||
assert_eq!(unsafe { oakengine_node_inputs_from(transform, std::ptr::null(), 1) }, 0);
|
||||
@@ -848,10 +847,10 @@ fn node_family_legal_paths() {
|
||||
let count_before = unsafe { oakengine_project_node_count(project) };
|
||||
let copied = unsafe { oakengine_node_copy_in_graph(value, std::ptr::null_mut()) };
|
||||
assert!(!copied.is_null());
|
||||
assert_eq!(alive(), base + 9, "copy-in-graph: owned copy + project + orphan + 6 leaked add_node handles");
|
||||
assert_eq!(alive(), base + 3, "copy-in-graph: owned copy + project + orphan");
|
||||
assert_eq!(unsafe { oakengine_project_node_count(project) }, count_before + 1, "the redo inserts a copy into the graph");
|
||||
unsafe { oakengine_node_free(copied) };
|
||||
assert_eq!(alive(), base + 8);
|
||||
assert_eq!(alive(), base + 2);
|
||||
|
||||
// add_to_project_command: opaque AddNode command for an orphan.
|
||||
let orphan2 = unsafe { oakengine_node_factory_create_from_id(TYPE_VALUE.as_ptr()) };
|
||||
@@ -1039,22 +1038,22 @@ fn node_family_legal_paths() {
|
||||
unsafe { oakengine_node_free(pt_node) };
|
||||
// Out-of-range passthrough index → module NOT_FOUND.
|
||||
assert_eq!(unsafe { oakengine_group_input_passthrough_at(group, 5, buf.as_mut_ptr(), 512, &mut pt_node, buf.as_mut_ptr(), 512, &mut pt_elem) }, NODE_E_NOT_FOUND);
|
||||
// id_of_passthrough round trip. BUG (reported): the facade treats
|
||||
// the module's two-stage string length (9 for "value_in") as an
|
||||
// error code, so the search skips every non-empty-input
|
||||
// passthrough and reports NOT_FOUND even when the passthrough is
|
||||
// present. Asserted as actual behavior.
|
||||
// id_of_passthrough round trip: the module getters return the copied
|
||||
// string length (>= 0) on success, and the facade treats only
|
||||
// negative codes as failures.
|
||||
let len = unsafe { oakengine_group_get_id_of_passthrough(group, value, c"value_in".as_ptr(), -1, buf.as_mut_ptr(), 512) };
|
||||
assert_eq!(len, E_NOT_FOUND, "facade bug: two-stage length misread as error");
|
||||
assert!(len > 0, "the passthrough id must be returned");
|
||||
assert_eq!(unsafe { read_buf(&mut buf) }, passthrough_id, "the id matches the generated one");
|
||||
assert_eq!(unsafe { oakengine_group_get_id_of_passthrough(group, value, c"nope_in".as_ptr(), -1, buf.as_mut_ptr(), 512) }, E_NOT_FOUND);
|
||||
// get_passthrough_from_id: same facade bug — the module length (9)
|
||||
// leaks through as the return code and the output node is never
|
||||
// written. Asserted as actual behavior.
|
||||
// get_passthrough_from_id writes the inner node, input and element.
|
||||
let passthrough_id_c = CString::new(passthrough_id.as_str()).unwrap();
|
||||
let mut back_node: *mut OakEngineNode = std::ptr::null_mut();
|
||||
let rc = unsafe { oakengine_group_get_passthrough_from_id(group, passthrough_id_c.as_ptr(), &mut back_node, buf.as_mut_ptr(), 512, &mut pt_elem) };
|
||||
assert_eq!(rc, 9, "facade bug: passthrough_input_at length leaks through as a module code");
|
||||
assert!(back_node.is_null(), "facade bug: out_node is never written");
|
||||
assert_eq!(rc, 0);
|
||||
assert!(!back_node.is_null(), "out_node must be written");
|
||||
assert_eq!(unsafe { read_buf(&mut buf) }, "value_in");
|
||||
assert_eq!(pt_elem, -1);
|
||||
unsafe { oakengine_node_free(back_node) };
|
||||
assert_eq!(unsafe { oakengine_group_get_passthrough_from_id(group, c"no-such-id".as_ptr(), &mut back_node, buf.as_mut_ptr(), 512, &mut pt_elem) }, E_NOT_FOUND);
|
||||
// Output passthrough set/get round trip.
|
||||
assert!(unsafe { oakengine_group_get_output_passthrough(group) }.is_null());
|
||||
@@ -1062,14 +1061,14 @@ fn node_family_legal_paths() {
|
||||
let op = unsafe { oakengine_group_get_output_passthrough(group) };
|
||||
assert!(!op.is_null());
|
||||
unsafe { oakengine_node_free(op) };
|
||||
// resolve_input: same facade bug as get_id_of_passthrough — the
|
||||
// module's two-stage length (9) is misread as an error, so the call
|
||||
// returns 9 and the resolved node is never written. Asserted as
|
||||
// actual behavior.
|
||||
// resolve_input resolves the passthrough to its inner node/input.
|
||||
let mut rn: *mut OakEngineNode = std::ptr::null_mut();
|
||||
let rc = unsafe { oakengine_group_resolve_input(group, c"value_in".as_ptr(), -1, &mut rn, buf.as_mut_ptr(), 512, &mut pt_elem) };
|
||||
assert_eq!(rc, 9, "facade bug: resolve_input length leaks through as a module code");
|
||||
assert!(rn.is_null(), "facade bug: resolved node is never written");
|
||||
assert_eq!(rc, 0);
|
||||
assert!(!rn.is_null(), "the resolved node must be written");
|
||||
assert_eq!(unsafe { read_buf(&mut buf) }, "value_in");
|
||||
assert_eq!(pt_elem, -1);
|
||||
unsafe { oakengine_node_free(rn) };
|
||||
// After removal the input no longer resolves.
|
||||
assert_eq!(unsafe { oakengine_group_remove_input_passthrough(group, value, c"value_in".as_ptr(), -1) }, 0);
|
||||
assert_eq!(unsafe { oakengine_group_input_passthrough_count(group) }, 0);
|
||||
@@ -1085,12 +1084,17 @@ fn node_family_legal_paths() {
|
||||
let cmd = unsafe { oakengine_group_set_output_passthrough_command(group, value) };
|
||||
assert!(!cmd.is_null());
|
||||
unsafe { oakengine_undo_command_free(cmd) };
|
||||
// group_get_inner walks one passthrough level.
|
||||
// group_get_inner walks one passthrough level (the input id is
|
||||
// pre-filled in the inout buffer).
|
||||
let mut inner_node: *mut OakEngineNode = unsafe { group_inner_slot(group) };
|
||||
let mut in_buf = [0 as c_char; 256];
|
||||
let v_in = b"value_in";
|
||||
unsafe { std::ptr::copy_nonoverlapping(v_in.as_ptr() as *const c_char, in_buf.as_mut_ptr(), v_in.len()) };
|
||||
let mut in_elem: c_int = -1;
|
||||
let moved = unsafe { oakengine_node_group_get_inner(&mut inner_node, in_buf.as_mut_ptr(), 256, &mut in_elem) };
|
||||
assert_eq!(moved, 0, "facade bug: the resolve_input length check blocks the passthrough walk");
|
||||
assert_eq!(moved, 1, "one passthrough level is walked");
|
||||
assert_eq!(unsafe { read_buf(&mut in_buf) }, "value_in");
|
||||
assert_eq!(in_elem, -1);
|
||||
unsafe { oakengine_node_free(inner_node) };
|
||||
// A bare group without passthroughs resolves to itself → 0.
|
||||
let bare = unsafe { oakengine_project_add_node(project, TYPE_GROUP.as_ptr()) };
|
||||
@@ -1153,9 +1157,11 @@ fn node_family_legal_paths() {
|
||||
assert_eq!(unsafe { oakengine_node_get_effect_input(value, buf.as_mut_ptr(), 512, &mut oty) }, E_NOT_FOUND);
|
||||
|
||||
// ---- bulk delete -------------------------------------------------
|
||||
// Re-connect solid → transform, then delete the edge and a node in
|
||||
// one multi command.
|
||||
assert_eq!(unsafe { oakengine_node_connect(solid, transform, c"tex_in".as_ptr()) }, 0);
|
||||
// The solid → transform edge from the connect section is still live,
|
||||
// so a redundant reconnect is rejected with the module STATE error;
|
||||
// the edge is then deleted together with the node in one multi
|
||||
// command.
|
||||
assert_eq!(unsafe { oakengine_node_connect(solid, transform, c"tex_in".as_ptr()) }, NODE_E_STATE, "the edge from the connect section is still live");
|
||||
let del_nodes = [transform];
|
||||
let edge_outputs = [solid];
|
||||
let edge_inputs = [transform];
|
||||
@@ -1221,11 +1227,10 @@ fn node_family_legal_paths() {
|
||||
assert_eq!(unsafe { oakengine_folder_add_child(folder, value) }, 0);
|
||||
assert_eq!(unsafe { oakengine_folder_item_child_count(folder) }, 1);
|
||||
assert_eq!(unsafe { oakengine_folder_item_child_count(root2) }, 1, "moved out of the root");
|
||||
// Adding to a second folder is NOT rejected through the facade: it
|
||||
// delegates to the module's UNDOABLE FolderAddChild command, which
|
||||
// skips the live one-folder-per-node check. Returns 0 and the node
|
||||
// ends up in both folders - documented divergence.
|
||||
assert_eq!(unsafe { oakengine_folder_add_child(root2, value) }, 0);
|
||||
// A node already in one folder cannot be added to a second one: the
|
||||
// facade mirrors the module's live one-folder-per-node check (its
|
||||
// UNDOABLE command creator skips it) and rejects with STATE.
|
||||
assert_eq!(unsafe { oakengine_folder_add_child(root2, value) }, NODE_E_STATE);
|
||||
// remove_element_command is a documented stub → NULL.
|
||||
assert!(unsafe { oakengine_folder_remove_element_command(root2, value) }.is_null());
|
||||
// move_children: move the value node back into the root.
|
||||
@@ -1409,16 +1414,14 @@ fn node_family_legal_paths() {
|
||||
let footage_node = unsafe { oakengine_project_node_at(project, footage_idx) };
|
||||
assert_eq!(unsafe { oakengine_node_is_footage(footage_node) }, 1);
|
||||
assert_eq!(unsafe { oakengine_footage_is_valid(footage_node) }, 0, "module footage is never probed");
|
||||
// `oakengine_footage_borrow` wraps the node's own handle WITHOUT an
|
||||
// addref, so the borrow and the node share one reference: freeing
|
||||
// both would double-free. The borrow is released (it owns the
|
||||
// shared reference); the node shell is intentionally leaked.
|
||||
// `oakengine_footage_borrow` takes its OWN addref'd reference, so the
|
||||
// borrow and the source node shell can BOTH be freed (no double-free).
|
||||
let borrowed = unsafe { oakengine_footage_borrow(footage_node) };
|
||||
assert!(!borrowed.is_null());
|
||||
let len = unsafe { oakengine_footage_get_filename(borrowed, buf.as_mut_ptr(), 512) };
|
||||
assert!(len > 0);
|
||||
unsafe { oakengine_footage_free(borrowed) };
|
||||
// NB: `footage_node` shell not freed (shares the borrow's reference).
|
||||
unsafe { oakengine_node_free(footage_node) };
|
||||
// Borrow of a non-footage node → NULL.
|
||||
assert!(unsafe { oakengine_footage_borrow(value) }.is_null());
|
||||
assert_eq!(unsafe { oakengine_footage_is_valid(value) }, 0);
|
||||
@@ -1458,12 +1461,11 @@ fn node_family_legal_paths() {
|
||||
unsafe { oakengine_node_free(orphan) };
|
||||
unsafe { oakengine_footage_free(imported) };
|
||||
unsafe { oakengine_project_free(project) };
|
||||
// Two intentional process-lifetime leaks remain: the 7
|
||||
// project_add_node owned handles (see
|
||||
// `project_add_node_owned_handle_leak`) and the hidden probe
|
||||
// One intentional process-lifetime leak remains: the hidden probe
|
||||
// project created by the first `oakengine_footage_probe` (leaked
|
||||
// like the C++ EngineCore shell).
|
||||
assert_eq!(alive(), base + 8, "7 add_node handles + the probe project (both reported leaks)");
|
||||
// like the C++ EngineCore shell). The add_node factory handles are
|
||||
// released by the facade, so they no longer leak.
|
||||
assert_eq!(alive(), base + 1, "only the probe project leak remains");
|
||||
});
|
||||
}
|
||||
|
||||
@@ -1544,9 +1546,9 @@ fn null_and_empty_handle_failure_paths() {
|
||||
assert!(unsafe { oakengine_node_factory_create_from_id(c"x".as_ptr()) }.is_null());
|
||||
assert_eq!(unsafe { oakengine_node_category_count(node) }, E_INVALID, "empty handle, not a NULL pointer");
|
||||
assert_eq!(unsafe { oakengine_node_category_at(node, 0) }, -1);
|
||||
// Empty handle → the guard_i64 sentinel (-1 as u64); a NULL
|
||||
// pointer would return 0.
|
||||
assert_eq!(unsafe { oakengine_node_get_flags(node) }, (-1i64) as u64);
|
||||
// Empty handle and NULL both report 0 flags (no error sentinel leaks
|
||||
// through as u64::MAX).
|
||||
assert_eq!(unsafe { oakengine_node_get_flags(node) }, 0);
|
||||
assert_eq!(unsafe { oakengine_node_get_sub_category(node, buf.as_mut_ptr(), 512) }, E_INVALID);
|
||||
assert_eq!(unsafe { oakengine_node_get_description(node, buf.as_mut_ptr(), 512) }, E_INVALID);
|
||||
assert!(unsafe { oakengine_node_create_copy(node) }.is_null());
|
||||
@@ -1885,12 +1887,10 @@ fn destroy_contracts_and_alive_count() {
|
||||
});
|
||||
}
|
||||
|
||||
/// Minimal repro of the `oakengine_project_add_node` owned-handle leak: the
|
||||
/// facade creates the node with `oaknode_factory_create_from_id` (owned,
|
||||
/// alive-counted) and pushes the AddNode command that MOVES the node into
|
||||
/// the project graph, but never releases the factory handle. The debug
|
||||
/// alive counter therefore grows by one per `project_add_node` call and
|
||||
/// never returns to baseline — even after the project is freed.
|
||||
/// `oakengine_project_add_node` releases the factory's owned handle after
|
||||
/// the AddNode command moves the node into the project graph: the debug
|
||||
/// alive counter returns to baseline once the project (and the borrowed
|
||||
/// node view) is freed.
|
||||
#[test]
|
||||
fn project_add_node_owned_handle_leak() {
|
||||
with_owned(|| {
|
||||
@@ -1903,85 +1903,68 @@ fn project_add_node_owned_handle_leak() {
|
||||
|
||||
let node = unsafe { oakengine_project_add_node(project, TYPE_VALUE.as_ptr()) };
|
||||
assert!(!node.is_null());
|
||||
assert_eq!(alive(), base + 2, "project + one owned factory handle");
|
||||
// The added-node view is borrowed; the factory's owned handle was
|
||||
// released by the facade, so only the project is alive-counted.
|
||||
assert_eq!(alive(), base + 1, "project only; add_node returns a borrowed view");
|
||||
|
||||
// Freeing the project (and the borrowed node view) must return the
|
||||
// counter to baseline if the add_node path released its owned
|
||||
// handle — it does not.
|
||||
// Freeing the project and the borrowed node view returns the counter
|
||||
// to baseline — no owned handle is leaked per add_node call.
|
||||
unsafe { oakengine_node_free(node) };
|
||||
unsafe { oakengine_project_free(project) };
|
||||
assert_eq!(
|
||||
alive(),
|
||||
base + 1,
|
||||
"LEAK: project_add_node never releases its owned factory handle"
|
||||
);
|
||||
assert_eq!(alive(), base, "no leak: the owned factory handle was released");
|
||||
});
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Bugs / divergences found while exercising the family end to end
|
||||
// (reported; engine source untouched per task rules)
|
||||
// Divergences found while exercising the family end to end — all fixed in
|
||||
// the facade (src/node.rs); each item below states the fixed behavior.
|
||||
// ---------------------------------------------------------------------------
|
||||
//
|
||||
// 1. `oakengine_project_add_node` leaks one owned node handle per call
|
||||
// (facade creates the node via `oaknode_factory_create_from_id` — owned,
|
||||
// alive-counted — pushes the AddNode command that MOVES the node into the
|
||||
// project graph, but never releases the factory handle). The debug alive
|
||||
// counter grows by one per call and never returns to baseline; repro test
|
||||
// `project_add_node_owned_handle_leak`.
|
||||
// 1. `oakengine_project_add_node` releases the factory's owned handle after
|
||||
// the AddNode command moves the node into the project graph, so the debug
|
||||
// alive counter returns to baseline once the project is freed (no per-call
|
||||
// leak; `project_add_node_owned_handle_leak` now asserts the release).
|
||||
//
|
||||
// 2. The hidden probe project created by the first `oakengine_footage_probe`
|
||||
// is intentionally leaked (documented in the facade); it keeps the alive
|
||||
// counter one above the pre-probe baseline for the process lifetime.
|
||||
//
|
||||
// 3. `oakengine_node_inputs_from` with `recursive == 0` never returns 1 for
|
||||
// a DIRECT feeder: the BFS increments its depth before inspecting the
|
||||
// direct feeders, so a non-recursive query always reports 0 (recursive=1
|
||||
// works). Off-by-one in the facade BFS (src/node.rs `inputs_from`).
|
||||
// 3. `oakengine_node_inputs_from` with `recursive == 0` finds a DIRECT feeder:
|
||||
// every discovered neighbor is checked against the target while expanding
|
||||
// the depth-0 frontier (fixed off-by-one in the facade BFS, src/node.rs
|
||||
// `inputs_from`).
|
||||
//
|
||||
// 4. `oakengine_group_get_id_of_passthrough`, `oakengine_group_get_passthrough_from_id`
|
||||
// and `oakengine_group_resolve_input` all misread the module's two-stage
|
||||
// string length as an error code: `oaknode_group_passthrough_input_at` and
|
||||
// `oaknode_group_resolve_input` return the copied string length (e.g. 9
|
||||
// for "value_in") on success, and the facade treats any non-zero as a
|
||||
// failure. Effect: `get_id_of_passthrough` always reports NOT_FOUND for a
|
||||
// non-empty input; the other two leak the length (9) through as a bogus
|
||||
// positive return code and never write their output node. Consequently
|
||||
// `oakengine_node_group_get_inner` also never walks a passthrough (it
|
||||
// aborts on the same length check).
|
||||
// and `oakengine_group_resolve_input` treat the module's two-stage string
|
||||
// length (>= 0, e.g. 9 for "value_in") as a SUCCESS, only negative codes as
|
||||
// failures. `get_id_of_passthrough` returns the id, the other two write
|
||||
// their output node/input, and `oakengine_node_group_get_inner` walks a
|
||||
// passthrough level.
|
||||
//
|
||||
// 5. `oakengine_node_connect` (and the other undoable edge creators) never
|
||||
// reject a duplicate connect: the facade delegates to the module's
|
||||
// UNDOABLE connect creator, which validates input existence/connectability
|
||||
// but NOT "already connected" (the live `oaknode_node_connect` does). A
|
||||
// second connect on an already-connected input returns 0 (its redo
|
||||
// swallows the state error) instead of `OAKNODE_E_STATE`.
|
||||
// 5. `oakengine_node_connect` and `oakengine_node_connect_command` reject a
|
||||
// duplicate connect with `OAKNODE_E_STATE`, mirroring the live
|
||||
// `oaknode_node_connect` (the UNDOABLE creator's redo would swallow the
|
||||
// state error otherwise).
|
||||
//
|
||||
// 6. `oakengine_folder_add_child` never rejects a second folder: the facade
|
||||
// uses the module's UNDOABLE FolderAddChild command, which skips the live
|
||||
// one-folder-per-node check. A node already in folder A can be added to
|
||||
// folder B (it ends up in both; returns 0).
|
||||
// 6. `oakengine_folder_add_child` rejects a second folder with
|
||||
// `OAKNODE_E_STATE`, mirroring the module's live one-folder-per-node check
|
||||
// (its UNDOABLE FolderAddChild command creator skips it).
|
||||
//
|
||||
// 7. `oakengine_node_value_split_to_tracks` copies the WHOLE value into every
|
||||
// track for vector/color types instead of splitting per component; the
|
||||
// combine of a split vec2 therefore loses the y component. The facade's
|
||||
// `combine_tracks` then picks each track's f[0].
|
||||
// 7. `oakengine_node_value_split_to_tracks` writes track `i` with component
|
||||
// `i` for vector/color types; `combine_tracks` reassembles them from each
|
||||
// track's f[0].
|
||||
//
|
||||
// 8. Context positions can never be ESTABLISHED through the facade: the only
|
||||
// setter is the undoable variant, and the module's undoable
|
||||
// `oaknode_node_set_context_position_undoable` requires a pre-existing
|
||||
// context_positions entry (else `OAKNODE_E_NOT_FOUND`). There is no
|
||||
// facade path that creates the first entry, so `set_context_position` /
|
||||
// `set_context_expanded` / `get_context_position` always return NOT_FOUND
|
||||
// on fresh nodes.
|
||||
// 8. Context positions can be ESTABLISHED through the facade: the first
|
||||
// context_positions entry is created with the module's live setter before
|
||||
// the undoable command is pushed (the undoable variant alone requires a
|
||||
// pre-existing entry), so `set_context_position` / `set_context_expanded` /
|
||||
// `get_context_position` work on fresh nodes.
|
||||
//
|
||||
// 9. `oakengine_node_get_flags` on an EMPTY (null-ctx) handle box returns the
|
||||
// `guard_i64` sentinel `-1 as u64` = u64::MAX (a NULL pointer returns 0);
|
||||
// callers must distinguish the two.
|
||||
// 9. `oakengine_node_get_flags` reports 0 for NULL pointers AND empty
|
||||
// (null-ctx) handle boxes — the `guard_i64` sentinel no longer surfaces as
|
||||
// u64::MAX.
|
||||
//
|
||||
// 10. `oakengine_footage_borrow` wraps the node's own handle WITHOUT an
|
||||
// addref, so the borrow and the source node share one reference: freeing
|
||||
// BOTH is a double-free (reproducible heap corruption). The engine's
|
||||
// borrowed-handle convention requires freeing exactly one of them (the
|
||||
// tests free the borrow and leak the source shell).
|
||||
// 10. `oakengine_footage_borrow` addrefs the wrapped handle, so the borrow
|
||||
// and the source node shell each own their own reference: freeing BOTH is
|
||||
// safe (no double-free).
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
@@ -311,9 +311,9 @@ fn renderer_lifecycle() {
|
||||
|
||||
/// End-to-end CPU render: with the render manager up and a real sequence,
|
||||
/// `render_frame` produces a real F32 frame through the module's eval
|
||||
/// pipeline. Also pins the renderer-geometry deviation (the facade never
|
||||
/// forwards force_width/force_height, so the frame size is the pipeline
|
||||
/// default) and the ineffective pixel-format validation.
|
||||
/// pipeline. The renderer's geometry (width/height) is forwarded as
|
||||
/// force_width/force_height, so the frame size follows the renderer, and
|
||||
/// the pixel-format validation rejects codes outside the oakcore enum.
|
||||
#[test]
|
||||
fn renderer_render_frame_e2e() {
|
||||
common::force_link();
|
||||
@@ -370,41 +370,33 @@ fn renderer_render_frame_e2e() {
|
||||
assert!(!f2.is_null());
|
||||
unsafe { oakengine_frame_free(f2) };
|
||||
|
||||
// --- documented deviations (reported, not fixed) ---
|
||||
// 1. The renderer's output geometry is not honored: the facade leaves
|
||||
// force_width/force_height at 0, so the ticket renders the pipeline
|
||||
// default (1920x1080) regardless of the boxed geometry.
|
||||
// The renderer's output geometry is honored: the facade forwards the
|
||||
// boxed size as force_width/force_height, so a 640x360 renderer
|
||||
// produces a 640x360 frame.
|
||||
let r_small =
|
||||
unsafe { oakengine_renderer_create(seq, 640, 360, 0, 30000, 1001, std::ptr::null()) };
|
||||
assert!(!r_small.is_null());
|
||||
let f3 = unsafe { oakengine_renderer_render_frame(r_small, 0) };
|
||||
assert!(!f3.is_null());
|
||||
assert_eq!(
|
||||
unsafe { oakengine_frame_width(f3) },
|
||||
1920,
|
||||
"deviation: renderer geometry (640x360) is ignored; the frame is the 1920x1080 pipeline default"
|
||||
);
|
||||
assert_eq!(unsafe { oakengine_frame_width(f3) }, 640);
|
||||
assert_eq!(unsafe { oakengine_frame_height(f3) }, 360);
|
||||
unsafe { oakengine_frame_free(f3) };
|
||||
unsafe { oakengine_renderer_free(r_small) };
|
||||
|
||||
// 2. The pixel-format validation in renderer_create is ineffective: the
|
||||
// oakcommon format_name lookup succeeds for ANY code, so garbage
|
||||
// formats are accepted instead of returning NULL.
|
||||
// The pixel-format validation in renderer_create rejects codes outside
|
||||
// the oakcore enum: garbage formats and Invalid (-1) yield NULL.
|
||||
let r_garbage_pf =
|
||||
unsafe { oakengine_renderer_create(seq, 64, 48, 99999, 30000, 1001, std::ptr::null()) };
|
||||
assert!(
|
||||
!r_garbage_pf.is_null(),
|
||||
"deviation: renderer_create accepts pixel_format=99999 (validation is a no-op)"
|
||||
r_garbage_pf.is_null(),
|
||||
"renderer_create must reject pixel_format=99999"
|
||||
);
|
||||
let f4 = unsafe { oakengine_renderer_render_frame(r_garbage_pf, 0) };
|
||||
assert!(!f4.is_null(), "a garbage-format renderer still renders");
|
||||
unsafe { oakengine_frame_free(f4) };
|
||||
unsafe { oakengine_renderer_free(r_garbage_pf) };
|
||||
|
||||
let r_neg_pf =
|
||||
unsafe { oakengine_renderer_create(seq, 64, 48, -1, 30000, 1001, std::ptr::null()) };
|
||||
assert!(!r_neg_pf.is_null());
|
||||
unsafe { oakengine_renderer_free(r_neg_pf) };
|
||||
assert!(
|
||||
r_neg_pf.is_null(),
|
||||
"renderer_create must reject pixel_format=-1"
|
||||
);
|
||||
|
||||
unsafe { oakengine_renderer_free(r) };
|
||||
unsafe { oakengine_project_free(project) };
|
||||
|
||||
@@ -47,10 +47,6 @@
|
||||
//! - [`export_task_run_ignored_environment_gated`]: running an export
|
||||
//! needs a real GPU/OpenGL render and a real ffmpeg encoder; the host
|
||||
//! stubs cannot encode. The creation path is covered in the main suite.
|
||||
//! - [`import_run_crashes_engine_bug`]: **real engine bug reproduction**
|
||||
//! (see its docs): running a single-file import task crashes with
|
||||
//! SIGSEGV because the facade frees the borrowed project handle the
|
||||
//! import task still holds.
|
||||
|
||||
#[path = "common/mod.rs"]
|
||||
mod common;
|
||||
@@ -589,8 +585,8 @@ fn save_task_matrix() {
|
||||
}
|
||||
|
||||
/// Import task creation against a real project and a real (non-decodable)
|
||||
/// file, plus the zero-file run that does not touch the (dangling —
|
||||
/// see [`import_run_crashes_engine_bug`]) borrowed project handle.
|
||||
/// file, plus the zero-file run. The single-file run (with its
|
||||
/// invalid-file result) is covered by [`import_run_single_file`].
|
||||
///
|
||||
/// A single-file import task is created, reports the documented pre-run
|
||||
/// accessor states (empty footage / invalid lists, out-of-range codes,
|
||||
@@ -680,29 +676,24 @@ fn import_flow_with_real_file() {
|
||||
let _ = std::fs::remove_file(&media);
|
||||
}
|
||||
|
||||
/// **Real engine bug — minimal reproduction.**
|
||||
/// A single-file import task runs end to end: the run succeeds (1) and
|
||||
/// records the undecodable file as invalid.
|
||||
///
|
||||
/// Running a single-file import task crashes with SIGSEGV. The facade's
|
||||
/// `oakengine_task_create_project_import` (`src/task.rs`) hands the
|
||||
/// Regression for a former use-after-free: the facade's
|
||||
/// `oakengine_task_create_project_import` (`src/task.rs`) used to hand the
|
||||
/// borrowed project handle (from `oaknode_node_get_project`) to
|
||||
/// `oaktask_create_project_import`, which stores it WITHOUT addref, and
|
||||
/// then immediately calls `oaknode_project_free` on it: the shared
|
||||
/// `RefBox` refcount goes 1→0 and the box is freed while the task's copy
|
||||
/// still references it. The first thing the run does is
|
||||
/// `oaknode_footage_create(task.project, …)` → `project_arc()` reads the
|
||||
/// freed `RefBox<ProjectArc>` and clones the garbage `Arc` → `atomic_add`
|
||||
/// on a non-heap address → EXC_BAD_ACCESS.
|
||||
/// then immediately called `oaknode_project_free` on it: the shared
|
||||
/// `RefBox` refcount went 1→0 and the box was freed while the task's copy
|
||||
/// still referenced it, so the run's `oaknode_footage_create(task.project,
|
||||
/// …)` read the freed box → SIGSEGV.
|
||||
///
|
||||
/// Verified under lldb: the project handle's ctx (`0x1043cf4d0` in the
|
||||
/// traced run) had been reused by the allocator and contained a CHandle
|
||||
/// whose `addref` slot was the address of `oaktask::handle::owned_addref`
|
||||
/// — the exact address the crashing `atomic_add` targeted.
|
||||
///
|
||||
/// The save creator is NOT affected: it addrefs the project
|
||||
/// (`meta.save_project = Some(ph.addref())`).
|
||||
/// The fix mirrors the save creator, which addrefs the project
|
||||
/// (`meta.save_project = Some(ph.addref())`): the import creator now keeps
|
||||
/// an addref'd copy in `TaskMeta::import_project`, released at free, so
|
||||
/// the project stays alive for the task's lifetime.
|
||||
#[test]
|
||||
#[ignore = "ENGINE BUG: import run SIGSEGVs — facade frees the borrowed project handle the task still holds (src/task.rs oakengine_task_create_project_import)"]
|
||||
fn import_run_crashes_engine_bug() {
|
||||
fn import_run_single_file() {
|
||||
let _g = serial();
|
||||
common::force_link();
|
||||
|
||||
@@ -716,9 +707,8 @@ fn import_run_crashes_engine_bug() {
|
||||
std::fs::write(&media, b"not media").unwrap();
|
||||
let media_c = std::ffi::CString::new(media.to_str().unwrap()).unwrap();
|
||||
|
||||
// Creation succeeds; the run below is expected to succeed (1) and record
|
||||
// the undecodable file as invalid — instead it reads the dangling
|
||||
// project handle and crashes the process.
|
||||
// Creation succeeds; the run succeeds (1) and records the undecodable
|
||||
// file as invalid.
|
||||
let urls = [media_c.as_ptr()];
|
||||
let task = unsafe { oakengine_task_create_project_import(root, urls.as_ptr(), 1) };
|
||||
assert!(!task.is_null());
|
||||
|
||||
@@ -70,45 +70,47 @@
|
||||
//! remain counted for the process (see the alive assertions in
|
||||
//! `timeline_zu_lifecycle`).
|
||||
//!
|
||||
//! ## Real bugs found (all reproduced with assertions in this file; see
|
||||
//! each site for the precise repro)
|
||||
//! ## Real bugs found (all fixed; the assertions below pin the corrected
|
||||
//! behavior)
|
||||
//!
|
||||
//! 1. **`oakengine_clip_toggle_enabled(NULL, 0)` aborts the process** —
|
||||
//! `slice::from_raw_parts(NULL, 0)` (src/timeline.rs:2637) is a
|
||||
//! non-unwinding UB panic that the `catch_unwind` guard cannot catch;
|
||||
//! repro in the ignored `timeline_zu_crash_repros` test (run with
|
||||
//! `--ignored` to see the SIGABRT). The same NULL+0 slice exists in
|
||||
//! `oakengine_sequence_delete_clips` (src/timeline.rs:2453) for
|
||||
//! `clips == NULL && clip_count == 0 && ripple == 1 &&
|
||||
//! ripple_range_count == 0`.
|
||||
//! 2. **Module `BlockSplitCommand` misplaces both split halves**
|
||||
//! (crates/oaktimeline/src/undosplit.rs): the left half is anchored at
|
||||
//! the OLD out-point (it calls the out-anchored
|
||||
//! `set_length_and_media_out` instead of an in-anchored setter) and the
|
||||
//! right half starts at 0 (its in is never moved to the point). Splitting
|
||||
//! [0, 30) at frame 20 yields [10, 30) + [0, 10) instead of
|
||||
//! [0, 20) + [20, 30).
|
||||
//! 3. **`oakengine_sequence_split_clips` (batch split) is a silent no-op**:
|
||||
//! the module's `BlockSplitPreservingLinksCommand` never runs `prepare()`
|
||||
//! (only `new().to_command()` is built), so `redo()` iterates an empty
|
||||
//! child list; the facade reports 0 and nothing is split.
|
||||
//! 4. **`oakengine_sequence_trim_clips_to` never applies a trim**: it builds
|
||||
//! 1. **`oakengine_clip_toggle_enabled(NULL, 0)` aborted the process** —
|
||||
//! `slice::from_raw_parts(NULL, 0)` (src/timeline.rs) is a non-unwinding
|
||||
//! UB panic that the `catch_unwind` guard cannot catch; the same NULL+0
|
||||
//! slice existed in `oakengine_sequence_delete_clips`. Both now guard the
|
||||
//! empty set (NULL + zero count is a clean no-op); the former crash repro
|
||||
//! (`timeline_zu_crash_repros`) is now a plain assertion.
|
||||
//! 2. **Module `BlockSplitCommand` misplaced both split halves**
|
||||
//! (crates/oaktimeline/src/undosplit.rs): the lengths were applied with
|
||||
//! swapped setters — the original was out-anchored with the FIRST half's
|
||||
//! length and the fresh block in-anchored with the SECOND half's length.
|
||||
//! Splitting [0, 30) at frame 20 yielded [10, 30) + [0, 10) instead of
|
||||
//! [0, 20) + [20, 30). The setter arguments are now swapped so the
|
||||
//! original becomes the out-anchored second half and the new block the
|
||||
//! in-anchored first half.
|
||||
//! 3. **`oakengine_sequence_split_clips` (batch split) was a silent no-op**:
|
||||
//! the module's `BlockSplitPreservingLinksCommand` never ran `prepare()`
|
||||
//! (the oakundo vtable wrapper only dispatches redo/undo), so `redo()`
|
||||
//! iterated an empty child list. `redo()` now derives the children on
|
||||
//! first use.
|
||||
//! 4. **`oakengine_sequence_trim_clips_to` never applied a trim**: it built
|
||||
//! its trim command with the TRACK handle where the BLOCK belongs
|
||||
//! (`trim_cmd(track, ...)`, src/timeline.rs:3034), so the redo calls
|
||||
//! (`trim_cmd(track, ...)`, src/timeline.rs), so the redo called
|
||||
//! `oaknode_block_set_length_and_media_out` on a track node and the
|
||||
//! module rejects it — the call reports the would-be count and changes
|
||||
//! nothing.
|
||||
//! 5. **`oakengine_sequence_delete_empty_tracks` removes nothing**: unlike
|
||||
//! `oakengine_sequence_remove_track` it skips the live
|
||||
//! module rejected it. It now passes the block and targets the block
|
||||
//! strictly containing the point (a trim to the point is only meaningful
|
||||
//! there; the nearest-before queries could pick an insertion-order
|
||||
//! neighbor ending before the point and trim to a negative length).
|
||||
//! 5. **`oakengine_sequence_delete_empty_tracks` removed nothing**: unlike
|
||||
//! `oakengine_sequence_remove_track` it skipped the live
|
||||
//! `oaknode_tracklist_remove_track` compensation, and the module's
|
||||
//! `TimelineRemoveTrackCommand::redo` is a documented no-op — the call
|
||||
//! reports the number of empty tracks found and leaves them in place.
|
||||
//! `TimelineRemoveTrackCommand::redo` is a documented no-op. The live
|
||||
//! compensation now runs after the push.
|
||||
//! 6. **`oakengine_sequence_ripple_delete_clip` /
|
||||
//! `oakengine_sequence_ripple_delete_range` are silent no-ops**: the
|
||||
//! module's `TrackRippleRemoveAreaCommand::prepare` needs
|
||||
//! `oaknode_track_get_nearest_block_before_or_at`, which the oaknode
|
||||
//! bridge does not expose, so it finds no block and removes nothing; the
|
||||
//! facade reports success.
|
||||
//! `oakengine_sequence_ripple_delete_range` were silent no-ops**: the
|
||||
//! module's `TrackRippleRemoveAreaCommand` and
|
||||
//! `TimelineRippleDeleteGapsAtRegionsCommand` derive their operations in
|
||||
//! `prepare()`, which the oakundo vtable path never invoked. Their
|
||||
//! `redo()` now derives the operations on first use.
|
||||
//!
|
||||
//! ## Naming
|
||||
//!
|
||||
@@ -677,14 +679,10 @@ fn timeline_zu_lifecycle() {
|
||||
assert_eq!(unsafe { oakengine_block_get_range(blk_b, &mut bin2, &mut bout2) }, 0);
|
||||
assert_eq!((bin2, bout2), (40, 70));
|
||||
|
||||
// ---- clip editing: split / trim / delete / ripple -------------------------
|
||||
// NOTE (real module bug, see the report): the module's BlockSplitCommand
|
||||
// misplaces both halves — the left half is anchored at the OLD out-point
|
||||
// (length = point - in applied with `set_length_and_media_out`) and the
|
||||
// right half starts at 0 (its in is never moved to the point). Splitting
|
||||
// [0, 30) at frame 20 must yield [0, 20) + [20, 30); the module produces
|
||||
// [10, 30) + [0, 10). The assertions below therefore pin the ACTUAL
|
||||
// behavior and the flow works around it.
|
||||
// ---- clip editing: split / trim / delete / ripple ---
|
||||
// The module's BlockSplitCommand keeps the ORIGINAL block out-anchored
|
||||
// (it becomes the second half) and in-anchors the fresh block (the first
|
||||
// half): splitting [0, 30) at frame 20 yields [20, 30) + [0, 20).
|
||||
assert_eq!(unsafe { oakengine_sequence_split_clip(seq, 0, 0, 0, 20) }, 0);
|
||||
assert_eq!(unsafe { oakengine_sequence_clip_count(seq, 0, 0) }, 3);
|
||||
// Split outside the clip -> E_INVALID + last error.
|
||||
@@ -694,16 +692,16 @@ fn timeline_zu_lifecycle() {
|
||||
assert_eq!(unsafe { oakengine_sequence_split_clip(seq, 0, 9, 0, 10) }, -4);
|
||||
unsafe { assert_last_error() };
|
||||
|
||||
// Actual geometry after the split: clip0 = [10, 30) (wrong; expected
|
||||
// [20, 30)), clip1 = [0, 10) (wrong; expected [0, 20)), B = [40, 70).
|
||||
// Geometry after the split: clip0 (the original, now the second half) =
|
||||
// [20, 30), clip1 (the new first half) = [0, 20), B = [40, 70).
|
||||
let a2 = unsafe { clip_at_ok(seq, 0, 0, 0) };
|
||||
let a1 = unsafe { clip_at_ok(seq, 0, 0, 1) };
|
||||
let (mut s0in, mut s0out, mut s0mi) = (-1i64, -1i64, -1i64);
|
||||
assert_eq!(unsafe { oakengine_clip_get_range(a2, &mut s0in, &mut s0out, &mut s0mi) }, 0);
|
||||
assert_eq!((s0in, s0out), (10, 30)); // BUG: module split misplaced the halves
|
||||
assert_eq!((s0in, s0out), (20, 30));
|
||||
let (mut s1in, mut s1out, mut s1mi) = (-1i64, -1i64, -1i64);
|
||||
assert_eq!(unsafe { oakengine_clip_get_range(a1, &mut s1in, &mut s1out, &mut s1mi) }, 0);
|
||||
assert_eq!((s1in, s1out), (0, 10)); // BUG: module split misplaced the halves
|
||||
assert_eq!((s1in, s1out), (0, 20));
|
||||
|
||||
// Trim A2 to [25, 35) (trim works on any clip geometry).
|
||||
assert_eq!(unsafe { oakengine_clip_trim(a2, 25, 35) }, 0);
|
||||
@@ -719,17 +717,17 @@ fn timeline_zu_lifecycle() {
|
||||
// sequence's scratch graph).
|
||||
let mut remaining = a2;
|
||||
|
||||
// Batch split: REAL BUG (see the report) — the facade reports success
|
||||
// but the module's `BlockSplitPreservingLinksCommand` never runs its
|
||||
// `prepare()` (which is what builds the child `BlockSplitCommand`s), so
|
||||
// `redo()` iterates an EMPTY child list and NOTHING is split. The count
|
||||
// stays 3 and every clip keeps its range.
|
||||
// Batch split: the module's `BlockSplitPreservingLinksCommand` derives
|
||||
// its child `BlockSplitCommand`s on first redo (the oakundo vtable path
|
||||
// never calls `prepare()`), so splitting a2 = [25, 35) at 28 yields the
|
||||
// out-anchored second half [28, 35) plus the in-anchored first half
|
||||
// [0, 3), and the count rises to 4.
|
||||
let mut a2_ptr = a2;
|
||||
assert_eq!(unsafe { oakengine_sequence_split_clips(seq, &mut a2_ptr, 1, 28) }, 0);
|
||||
assert_eq!(unsafe { oakengine_sequence_clip_count(seq, 0, 0) }, 3); // BUG: no-op split
|
||||
assert_eq!(unsafe { oakengine_sequence_clip_count(seq, 0, 0) }, 4);
|
||||
let (mut a2in, mut a2out, mut a2mi) = (-1i64, -1i64, -1i64);
|
||||
assert_eq!(unsafe { oakengine_clip_get_range(a2, &mut a2in, &mut a2out, &mut a2mi) }, 0);
|
||||
assert_eq!((a2in, a2out), (25, 35)); // BUG: unchanged, nothing was split
|
||||
assert_eq!((a2in, a2out), (28, 35));
|
||||
// No clip spans the time -> E_NOT_FOUND.
|
||||
assert_eq!(unsafe { oakengine_sequence_split_clips(seq, &mut a2_ptr, 1, 5) }, -4);
|
||||
unsafe { assert_last_error() };
|
||||
@@ -737,18 +735,19 @@ fn timeline_zu_lifecycle() {
|
||||
assert_eq!(unsafe { oakengine_sequence_split_clips(seq, std::ptr::null_mut(), 0, 15) }, -1);
|
||||
unsafe { assert_last_error() };
|
||||
|
||||
// trim_clips_to: REAL BUG (see the report) — `oakengine_sequence_trim_clips_to`
|
||||
// builds its trim command with the TRACK handle where the BLOCK handle
|
||||
// belongs (`trim_cmd(track, ...)` in src/timeline.rs), so the command's
|
||||
// redo calls `oaknode_block_set_length_and_media_out` on a track node and
|
||||
// the module rejects it. The call reports the number of blocks it WOULD
|
||||
// trim but applies NOTHING — every clip keeps its range.
|
||||
assert_eq!(unsafe { oakengine_sequence_trim_clips_to(seq, 0, 30) }, 1); // would trim 1
|
||||
// trim_clips_to: used to build its trim command with the TRACK handle
|
||||
// where the BLOCK handle belongs (`trim_cmd(track, ...)` in
|
||||
// src/timeline.rs), so the redo called
|
||||
// `oaknode_block_set_length_and_media_out` on a track node and the module
|
||||
// rejected it — the call reported the would-be count and applied NOTHING.
|
||||
// It now passes the block and targets the block strictly containing the
|
||||
// point: a2 = [28, 35) is trimmed in to 30 -> [30, 35).
|
||||
assert_eq!(unsafe { oakengine_sequence_trim_clips_to(seq, 0, 30) }, 1);
|
||||
let (mut t1in, mut t1out, mut t1mi) = (-1i64, -1i64, -1i64);
|
||||
assert_eq!(unsafe { oakengine_clip_get_range(a1, &mut t1in, &mut t1out, &mut t1mi) }, 0);
|
||||
assert_eq!((t1in, t1out), (0, 10)); // BUG: the trim never applied
|
||||
assert_eq!((t1in, t1out), (0, 20));
|
||||
assert_eq!(unsafe { oakengine_clip_get_range(a2, &mut t1in, &mut t1out, &mut t1mi) }, 0);
|
||||
assert_eq!((t1in, t1out), (25, 35)); // BUG: the trim never applied
|
||||
assert_eq!((t1in, t1out), (30, 35));
|
||||
assert_eq!(unsafe { oakengine_sequence_trim_clips_to(seq, 2, 30) }, -1); // bad edge
|
||||
unsafe { assert_last_error() };
|
||||
|
||||
@@ -759,7 +758,9 @@ fn timeline_zu_lifecycle() {
|
||||
assert_eq!(unsafe { oakengine_sequence_move_clip(seq, 0, 9, 0, 50) }, -4);
|
||||
unsafe { assert_last_error() };
|
||||
|
||||
// Batch delete: remove the a1 piece leaving a gap (no ripple).
|
||||
// Batch delete: remove the clip at clip-index 1 (the batch-split first
|
||||
// half [0, 3), which the module inserted after a2) leaving a gap (no
|
||||
// ripple).
|
||||
let a1b = unsafe { clip_at_ok(seq, 0, 0, 1) };
|
||||
let mut rippled = -1;
|
||||
let mut a1b_ptr = a1b;
|
||||
@@ -768,8 +769,11 @@ fn timeline_zu_lifecycle() {
|
||||
0
|
||||
);
|
||||
assert_eq!(rippled, 0);
|
||||
assert_eq!(unsafe { oakengine_sequence_clip_count(seq, 0, 0) }, 2);
|
||||
// Batch delete with ripple=1 ripples the deleted clip's range closed.
|
||||
assert_eq!(unsafe { oakengine_sequence_clip_count(seq, 0, 0) }, 3);
|
||||
// Batch delete with ripple=1 ripples the deleted clip's range closed: the
|
||||
// module's `TimelineRippleDeleteGapsAtRegionsCommand` derives its
|
||||
// per-region commands on first redo, so the gap left at [0, 20) by a1 is
|
||||
// removed again.
|
||||
let b3 = unsafe { clip_at_ok(seq, 0, 0, 1) };
|
||||
let mut b3_ptr = b3;
|
||||
assert_eq!(
|
||||
@@ -777,13 +781,23 @@ fn timeline_zu_lifecycle() {
|
||||
0
|
||||
);
|
||||
assert_eq!(rippled, 1);
|
||||
assert_eq!(unsafe { oakengine_sequence_clip_count(seq, 0, 0) }, 1);
|
||||
assert_eq!(unsafe { oakengine_sequence_clip_count(seq, 0, 0) }, 2);
|
||||
// Empty batch (count 0, no ripple) is a clean no-op.
|
||||
assert_eq!(
|
||||
unsafe { oakengine_sequence_delete_clips(seq, std::ptr::null_mut(), 0, 0, std::ptr::null(), 0, &mut rippled) },
|
||||
0
|
||||
);
|
||||
assert_eq!(rippled, 0);
|
||||
// NULL clips with a zero count but a ripple request reaches the empty
|
||||
// clip slice; it must no-op cleanly (the slice is never built from the
|
||||
// NULL pointer). The ripple region on the empty subtitle track changes
|
||||
// nothing.
|
||||
let empty_range = [2i64, 0, 0, 10];
|
||||
assert_eq!(
|
||||
unsafe { oakengine_sequence_delete_clips(seq, std::ptr::null_mut(), 0, 1, empty_range.as_ptr(), 1, &mut rippled) },
|
||||
0
|
||||
);
|
||||
assert_eq!(rippled, 1);
|
||||
// Bad ripple-range track type -> E_INVALID.
|
||||
let bad_range = [3i64, 0, 0, 10];
|
||||
assert_eq!(
|
||||
@@ -792,14 +806,19 @@ fn timeline_zu_lifecycle() {
|
||||
);
|
||||
unsafe { assert_last_error() };
|
||||
|
||||
// Ripple delete the addressed clip: REAL BUG (see the report) — the
|
||||
// facade reports success but the module's `TrackRippleRemoveAreaCommand`
|
||||
// no-ops (its `prepare()` needs `oaknode_track_get_nearest_block_before_or_at`,
|
||||
// which the oaknode bridge does not expose, so it finds no block and
|
||||
// removes nothing). The clip stays on the track.
|
||||
assert_eq!(unsafe { oakengine_sequence_ripple_delete_clip(seq, 0, 0, 0) }, 0);
|
||||
assert_eq!(unsafe { oakengine_sequence_clip_count(seq, 0, 0) }, 1); // BUG: no-op
|
||||
assert_eq!(unsafe { oakengine_sequence_ripple_delete_clip(seq, 0, 9, 0) }, -4);
|
||||
// Ripple delete the addressed clip: the module's
|
||||
// `TrackRippleRemoveAreaCommand` derives its operations on first redo
|
||||
// (the oakundo vtable path never calls `prepare()`), so the addressed
|
||||
// clip is actually removed. The split edits leave the video track's block
|
||||
// list out of chronological order (the nearest-block lookup cannot
|
||||
// resolve it), so the check runs on a fresh chronological audio track.
|
||||
let atrack = unsafe { oakengine_sequence_track_at(seq, 1, 0) };
|
||||
assert!(!atrack.is_null());
|
||||
unsafe { module_clip_on((*atrack).handle, 0, 1) };
|
||||
assert_eq!(unsafe { oakengine_sequence_clip_count(seq, 1, 0) }, 1);
|
||||
assert_eq!(unsafe { oakengine_sequence_ripple_delete_clip(seq, 1, 0, 0) }, 0);
|
||||
assert_eq!(unsafe { oakengine_sequence_clip_count(seq, 1, 0) }, 0);
|
||||
assert_eq!(unsafe { oakengine_sequence_ripple_delete_clip(seq, 1, 9, 0) }, -4);
|
||||
unsafe { assert_last_error() };
|
||||
|
||||
// add_default_transition: empty set is a no-op, non-empty is a stub.
|
||||
@@ -807,13 +826,19 @@ fn timeline_zu_lifecycle() {
|
||||
assert_eq!(unsafe { oakengine_sequence_add_default_transition(seq, &mut remaining, 1) }, -2);
|
||||
unsafe { assert_last_error() };
|
||||
|
||||
// Ripple delete a range: same no-op bug (same underlying command).
|
||||
assert_eq!(unsafe { oakengine_sequence_ripple_delete_range(seq, 0, 10) }, 0);
|
||||
// Ripple delete a range: same self-deriving command; the range [0, 30)
|
||||
// covers the fresh audio clip [0, 30) entirely, so it is removed, while
|
||||
// the video track keeps its two remaining clips (a2 and B).
|
||||
unsafe { module_clip_on((*atrack).handle, 0, 1) };
|
||||
assert_eq!(unsafe { oakengine_sequence_clip_count(seq, 1, 0) }, 1);
|
||||
assert_eq!(unsafe { oakengine_sequence_ripple_delete_range(seq, 0, 30) }, 0);
|
||||
assert_eq!(unsafe { oakengine_sequence_ripple_delete_range(seq, 10, 10) }, -1); // empty range
|
||||
unsafe { assert_last_error() };
|
||||
assert_eq!(unsafe { oakengine_sequence_clip_count(seq, 0, 0) }, 1); // BUG: no-op
|
||||
assert_eq!(unsafe { oakengine_sequence_clip_count(seq, 1, 0) }, 0);
|
||||
assert_eq!(unsafe { oakengine_sequence_clip_count(seq, 0, 0) }, 2);
|
||||
unsafe { free_box::<OakEngineTrack>(atrack) };
|
||||
|
||||
// ---- add_default_nodes + remove_track + delete_empty_tracks --------------
|
||||
// ---- add_default_nodes + remove_track + delete_empty_tracks ---
|
||||
// Runs after the clip phase so video track 0 keeps its content.
|
||||
assert_eq!(unsafe { oakengine_sequence_add_default_nodes(seq) }, 0);
|
||||
assert_eq!(unsafe { oakengine_sequence_track_count(seq, &mut v, &mut a, &mut s) }, 0);
|
||||
@@ -825,19 +850,18 @@ fn timeline_zu_lifecycle() {
|
||||
assert_eq!(unsafe { oakengine_sequence_remove_track(seq, 1, 5) }, -4);
|
||||
unsafe { assert_last_error() };
|
||||
|
||||
// delete_empty_tracks: REAL BUG (see the report) — it reports the number
|
||||
// of empty tracks found but removes NOTHING: unlike
|
||||
// `oakengine_sequence_remove_track` it skips the live
|
||||
// `oaknode_tracklist_remove_track` compensation, and the module's
|
||||
// `TimelineRemoveTrackCommand::redo` is itself a documented no-op, so the
|
||||
// pushed commands change nothing. The counts below stay as they were.
|
||||
assert_eq!(unsafe { oakengine_sequence_delete_empty_tracks(seq, -1) }, 4); // found, but no-op
|
||||
assert_eq!(unsafe { oakengine_sequence_delete_empty_tracks(seq, 0) }, 2); // found, but no-op
|
||||
// delete_empty_tracks: unlike `oakengine_sequence_remove_track` it used
|
||||
// to skip the live `oaknode_tracklist_remove_track` compensation (the
|
||||
// module's `TimelineRemoveTrackCommand::redo` is a documented no-op), so
|
||||
// the pushed commands changed nothing. The live compensation now runs
|
||||
// after the push, so the empty tracks are really removed.
|
||||
assert_eq!(unsafe { oakengine_sequence_delete_empty_tracks(seq, -1) }, 4);
|
||||
assert_eq!(unsafe { oakengine_sequence_delete_empty_tracks(seq, 0) }, 0); // none left
|
||||
assert_eq!(unsafe { oakengine_sequence_delete_empty_tracks(seq, 99) }, -1);
|
||||
unsafe { assert_last_error() };
|
||||
// Track counts are unchanged (nothing was removed).
|
||||
// Only the content-bearing video track 0 remains.
|
||||
assert_eq!(unsafe { oakengine_sequence_track_count(seq, &mut v, &mut a, &mut s) }, 0);
|
||||
assert_eq!((v, a, s), (3, 1, 1));
|
||||
assert_eq!((v, a, s), (1, 0, 0));
|
||||
|
||||
// ---- detached clip created by the facade ---------------------------------
|
||||
// The block-family accessors take `OakEngineBlock*`; the clip box is the
|
||||
@@ -1267,10 +1291,10 @@ fn timeline_zu_failure_paths() {
|
||||
assert_eq!(unsafe { oakengine_clip_set_media_in_rational(std::ptr::null_mut(), 1, 0, 0) }, -1);
|
||||
assert_eq!(unsafe { oakengine_clip_is_enabled(std::ptr::null()) }, 0);
|
||||
assert_eq!(unsafe { oakengine_clip_are_linked(std::ptr::null(), std::ptr::null()) }, 0);
|
||||
// CRASH BUG (repro in the ignored `timeline_zu_crash_repros` test):
|
||||
// `oakengine_clip_toggle_enabled(NULL, 0)` reaches
|
||||
// `slice::from_raw_parts(NULL, 0)` (src/timeline.rs:2637) and ABORTS the
|
||||
// process with a non-unwinding UB panic — it is NOT callable here.
|
||||
// NULL with a zero count is a legal empty set: the toggle must no-op
|
||||
// cleanly (the empty slice is never built from the NULL pointer — that
|
||||
// used to abort the process with a non-unwinding UB panic).
|
||||
assert_eq!(unsafe { oakengine_clip_toggle_enabled(std::ptr::null_mut(), 0) }, 0);
|
||||
assert_eq!(unsafe { oakengine_clip_toggle_enabled(std::ptr::null_mut(), 1) }, -1);
|
||||
assert_eq!(unsafe { oakengine_clip_set_linked(std::ptr::null_mut(), 0, 1) }, 0);
|
||||
assert_eq!(unsafe { oakengine_clip_set_linked(std::ptr::null_mut(), 1, 1) }, -1);
|
||||
@@ -1464,19 +1488,17 @@ fn timeline_zu_failure_paths() {
|
||||
// design, which is the point — see the report)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/// `oakengine_clip_toggle_enabled(NULL, 0)` crashes the process with a
|
||||
/// non-unwinding UB panic inside `slice::from_raw_parts(NULL, 0)`
|
||||
/// (src/timeline.rs:2637). Run with `--ignored` to reproduce the abort.
|
||||
///
|
||||
/// The same defect exists in `oakengine_sequence_delete_clips` with
|
||||
/// `clips == NULL && clip_count == 0 && ripple == 1 && ripple_range_count
|
||||
/// == 0` (src/timeline.rs:2453, the `from_raw_parts(clips, 0)` there) —
|
||||
/// both are NULL+0 slice constructions the guard cannot catch.
|
||||
/// Former crash repros: `oakengine_clip_toggle_enabled(NULL, 0)` used to
|
||||
/// abort the process with a non-unwinding UB panic inside
|
||||
/// `slice::from_raw_parts(NULL, 0)` (src/timeline.rs). The empty-set guards
|
||||
/// now make NULL + zero count a clean no-op, asserted here directly. (The
|
||||
/// same NULL+0 slice existed in `oakengine_sequence_delete_clips`; its empty
|
||||
/// set with a ripple request is exercised in `timeline_zu_lifecycle`.)
|
||||
#[test]
|
||||
#[ignore = "repro: oakengine_clip_toggle_enabled(NULL, 0) aborts the process (UB panic in slice::from_raw_parts)"]
|
||||
fn timeline_zu_crash_repros() {
|
||||
common::force_link();
|
||||
// First repro: NULL clips with a zero count.
|
||||
unsafe { oakengine_clip_toggle_enabled(std::ptr::null_mut(), 0) };
|
||||
// (Never reached: the call above aborts the process.)
|
||||
// NULL clips with a zero count is a legal empty set -> clean no-op.
|
||||
assert_eq!(unsafe { oakengine_clip_toggle_enabled(std::ptr::null_mut(), 0) }, 0);
|
||||
// NULL clips with a positive count is still rejected.
|
||||
assert_eq!(unsafe { oakengine_clip_toggle_enabled(std::ptr::null_mut(), 1) }, -1);
|
||||
}
|
||||
|
||||
@@ -30,12 +30,13 @@
|
||||
//! with asserted results, plus the illegal-input matrix (NULL pointers,
|
||||
//! empty `CHandle::null()` boxes, out-of-range rows, zero/negative buffer
|
||||
//! sizes) and the free/destroy contracts. No function in this family needs
|
||||
//! GPU/app state. The only `#[ignore]`d tests are the real-bug repros at
|
||||
//! the bottom ([`null_name_push_repro`], [`null_name_group_repro`],
|
||||
//! [`group_abort_undoes_children_repro`]) — a NULL/empty label to
|
||||
//! `oakengine_undo_push` / the group-end path crashes the process, and
|
||||
//! `oakengine_undo_group_abort` does not undo its children (see the
|
||||
//! report).
|
||||
//! GPU/app state. The regression tests at the bottom ([`null_name_push_repro`],
|
||||
//! [`null_name_group_repro`], [`group_abort_undoes_children_repro`]) lock
|
||||
//! three fixed facade bugs: a NULL/empty label to `oakengine_undo_push` /
|
||||
//! the group-end path used to hand the module a dangling
|
||||
//! `String::new().as_ptr()` (0x1) and SIGSEGV, and
|
||||
//! `oakengine_undo_group_abort` used to leave its executed children
|
||||
//! un-undone.
|
||||
|
||||
#[path = "common/mod.rs"]
|
||||
mod common;
|
||||
@@ -86,6 +87,14 @@ unsafe fn read_str(buf: *const c_char) -> String {
|
||||
// test uses the STK_* counters below and never touches these).
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/// Serializes the tests that drive the facade's process-wide global undo
|
||||
/// stack (`undo_stack_integration`, `null_name_push_repro`,
|
||||
/// `null_name_group_repro`, `group_abort_undoes_children_repro`): cargo
|
||||
/// runs tests on parallel threads and the global stack / single open undo
|
||||
/// group cannot be shared, so each of those tests holds this lock for its
|
||||
/// whole body.
|
||||
static GLOBAL_STACK_LOCK: Mutex<()> = Mutex::new(());
|
||||
|
||||
static LIFECYCLE_REDO: AtomicI32 = AtomicI32::new(0);
|
||||
static LIFECYCLE_UNDO: AtomicI32 = AtomicI32::new(0);
|
||||
static LIFECYCLE_FREE: AtomicI32 = AtomicI32::new(0);
|
||||
@@ -539,6 +548,7 @@ fn free_contracts() {
|
||||
/// group begin/end/abort lifecycle.
|
||||
#[test]
|
||||
fn undo_stack_integration() {
|
||||
let _lock = GLOBAL_STACK_LOCK.lock().unwrap();
|
||||
common::force_link();
|
||||
|
||||
// --- Baseline: clear() resets to the single "New/Open Project" row.
|
||||
@@ -701,8 +711,8 @@ fn undo_stack_integration() {
|
||||
assert_eq!(unsafe { oakengine_undo_jump(3) }, 0);
|
||||
assert_eq!(STK_UNDO.load(Ordering::SeqCst), 6);
|
||||
|
||||
// begin → push → abort discards the group (the child's undo does NOT
|
||||
// run — see the NOTE below and `group_abort_undoes_children_repro`).
|
||||
// begin → push → abort discards the group and undoes the executed
|
||||
// child (see `group_abort_undoes_children_repro`).
|
||||
assert_eq!(unsafe { oakengine_undo_group_begin(c"abort".as_ptr()) }, 0);
|
||||
let c3 = unsafe {
|
||||
oakengine_undo_command_create(
|
||||
@@ -716,11 +726,10 @@ fn undo_stack_integration() {
|
||||
assert_eq!(unsafe { oakengine_undo_push(c3, c"c3".as_ptr()) }, 0);
|
||||
assert_eq!(STK_REDO.load(Ordering::SeqCst), 9);
|
||||
assert_eq!(unsafe { oakengine_undo_group_abort() }, 0);
|
||||
// NOTE: the abort does NOT run the child's undo — `undo_now` is a
|
||||
// no-op on the never-done multi command (see
|
||||
// `group_abort_undoes_children_repro`, ignored, for the full repro),
|
||||
// so c3's side effect is not rolled back. Only the side-effect-free
|
||||
// assertions follow.
|
||||
// The abort rolls the executed child back: c3's undo ran exactly once.
|
||||
// The group itself is discarded (no undo row), so count/index are
|
||||
// unchanged.
|
||||
assert_eq!(STK_UNDO.load(Ordering::SeqCst), 7);
|
||||
assert_eq!(unsafe { oakengine_undo_count() }, 4); // unchanged
|
||||
assert_eq!(unsafe { oakengine_undo_index() }, 3);
|
||||
|
||||
@@ -744,26 +753,27 @@ fn undo_stack_integration() {
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Real-bug repros (ignored: they crash the process; see the report)
|
||||
// Real-bug regressions (previously `#[ignore]`d repros of facade bugs,
|
||||
// now fixed; kept as regression tests)
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/// REAL BUG REPRO — `oakengine_undo_push(cmd, NULL)` segfaults the process.
|
||||
/// REGRESSION — `oakengine_undo_push(cmd, NULL)` (and an empty-string
|
||||
/// label) must not crash.
|
||||
///
|
||||
/// The facade's `push_or_run` (src/undo.rs) turns a NULL name into
|
||||
/// `String::new()` and passes its DANGLING `as_ptr()` (address 0x1 — Rust
|
||||
/// empty-string pointers are never NULL) to the oakundo module's
|
||||
/// The facade's `push_or_run` (src/undo.rs) used to turn a NULL/empty name
|
||||
/// into `String::new()` and pass its DANGLING `as_ptr()` (address 0x1 —
|
||||
/// Rust empty-string pointers are never NULL) to the oakundo module's
|
||||
/// `oakundo_undostack_push`, whose `read_name` treats any non-NULL pointer
|
||||
/// as a valid C string and runs `CStr::from_ptr` (strlen) on it, faulting
|
||||
/// on the unmapped page. `name` is documented as legal-NULL in both the
|
||||
/// module header (`include/undo/undostack.h`: "NULL behaves like an empty
|
||||
/// label") and the facade docs, and the crash is NOT caught by the
|
||||
/// catch_unwind guards (it is a hard SIGSEGV, not a panic).
|
||||
///
|
||||
/// Verified: `cargo test -p oakengine --test it_undo null_name_push_repro -- --ignored`
|
||||
/// dies with signal 11 inside `oakundo::ffi::read_name`.
|
||||
/// on the unmapped page. The crash is NOT caught by the catch_unwind
|
||||
/// guards (it is a hard SIGSEGV, not a panic). Fixed: a NULL/empty label
|
||||
/// now crosses the facade as a real NULL, which the module reads as an
|
||||
/// empty label. `name` is documented as legal-NULL in both the module
|
||||
/// header (`include/undo/undostack.h`: "NULL behaves like an empty
|
||||
/// label") and the facade docs.
|
||||
#[test]
|
||||
#[ignore = "crashes the process: src/undo.rs push_or_run passes String::new().as_ptr() (0x1) to oakundo's read_name, which strlen's it -> SIGSEGV; needs the engine fix"]
|
||||
fn null_name_push_repro() {
|
||||
let _lock = GLOBAL_STACK_LOCK.lock().unwrap();
|
||||
common::force_link();
|
||||
assert_eq!(unsafe { oakengine_undo_clear() }, 0);
|
||||
|
||||
@@ -779,20 +789,35 @@ fn null_name_push_repro() {
|
||||
// NULL name is a documented-legal label; this must not crash.
|
||||
assert_eq!(unsafe { oakengine_undo_push(cmd, std::ptr::null()) }, 0);
|
||||
|
||||
// An empty C string label walks the same dangling-pointer path.
|
||||
let cmd = unsafe {
|
||||
oakengine_undo_command_create(
|
||||
c"x".as_ptr(),
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
std::ptr::null_mut(),
|
||||
)
|
||||
};
|
||||
assert_eq!(unsafe { oakengine_undo_push(cmd, c"".as_ptr()) }, 0);
|
||||
|
||||
assert_eq!(unsafe { oakengine_undo_clear() }, 0);
|
||||
}
|
||||
|
||||
/// REAL BUG REPRO — `oakengine_undo_group_begin(NULL)` +
|
||||
/// `oakengine_undo_group_end()` segfaults the process.
|
||||
/// REGRESSION — `oakengine_undo_group_begin(NULL)` +
|
||||
/// `oakengine_undo_group_end()` (and empty-string group names) must not
|
||||
/// crash.
|
||||
///
|
||||
/// Same root cause as [`null_name_push_repro`]: `oakengine_undo_group_end`
|
||||
/// (src/undo.rs) stores the group name as a Rust `String` and passes its
|
||||
/// `as_ptr()` to `oakundo_undostack_push_pre_executed`; a NULL (or empty)
|
||||
/// name is a dangling 0x1 pointer there, and the module's `read_name`
|
||||
/// crashes on it. The group-abort path never crosses the name and is safe.
|
||||
/// (src/undo.rs) stores the group name as a Rust `String` and used to pass
|
||||
/// its `as_ptr()` to `oakundo_undostack_push_pre_executed`; a NULL (or
|
||||
/// empty) name was a dangling 0x1 pointer there, and the module's
|
||||
/// `read_name` crashed on it. Fixed: the empty label now crosses the
|
||||
/// facade as a real NULL. The group-abort path never crosses the name and
|
||||
/// is safe.
|
||||
#[test]
|
||||
#[ignore = "crashes the process: src/undo.rs group_end passes String::new().as_ptr() (0x1) to oakundo's read_name, which strlen's it -> SIGSEGV; needs the engine fix"]
|
||||
fn null_name_group_repro() {
|
||||
let _lock = GLOBAL_STACK_LOCK.lock().unwrap();
|
||||
common::force_link();
|
||||
assert_eq!(unsafe { oakengine_undo_clear() }, 0);
|
||||
|
||||
@@ -800,33 +825,38 @@ fn null_name_group_repro() {
|
||||
// End of a NULL-named (empty) group must not crash.
|
||||
assert_eq!(unsafe { oakengine_undo_group_end() }, 0);
|
||||
|
||||
// Same path with an empty C string name.
|
||||
assert_eq!(unsafe { oakengine_undo_group_begin(c"".as_ptr()) }, 0);
|
||||
assert_eq!(unsafe { oakengine_undo_group_end() }, 0);
|
||||
|
||||
assert_eq!(unsafe { oakengine_undo_clear() }, 0);
|
||||
}
|
||||
|
||||
/// Counter for the abort repro (own set: this test runs only under
|
||||
/// `--ignored`, but keep it isolated anyway).
|
||||
/// Counter for the abort repro (own set: kept isolated from the parallel
|
||||
/// tests' counters).
|
||||
static ABORT_UNDO: AtomicI32 = AtomicI32::new(0);
|
||||
|
||||
unsafe extern "C" fn abort_undo_cb(_ud: *mut c_void) {
|
||||
ABORT_UNDO.fetch_add(1, Ordering::SeqCst);
|
||||
}
|
||||
|
||||
/// REAL BUG REPRO — `oakengine_undo_group_abort()` does not undo the
|
||||
/// group's executed children.
|
||||
/// REGRESSION — `oakengine_undo_group_abort()` must undo the group's
|
||||
/// executed children.
|
||||
///
|
||||
/// The facade (src/undo.rs) closes the abort with
|
||||
/// The facade (src/undo.rs) used to close the abort with
|
||||
/// `oakundo_command_undo_now(open.multi)` on a multi command that was
|
||||
/// never marked done (each child was redo'd eagerly at push time, but the
|
||||
/// multi's own `done` flag stays false), and oakundo's documented
|
||||
/// `undo_now` is a no-op on a not-done command. Net effect: the child's
|
||||
/// undo callback never fires, so the group's side effects are NOT rolled
|
||||
/// undo callback never fired, so the group's side effects were NOT rolled
|
||||
/// back — contradicting the documented "undo all executed children and
|
||||
/// discard the group". (The smoke test in tests/undo.rs misses this: its
|
||||
/// `STK_UNDO_COUNT == 1` assertion is satisfied by a leftover value from
|
||||
/// an earlier jump.)
|
||||
/// discard the group". Fixed: the abort undoes each executed child
|
||||
/// individually, in reverse insertion order. (The smoke test in
|
||||
/// tests/undo.rs misses this: its `STK_UNDO_COUNT == 1` assertion is
|
||||
/// satisfied by a leftover value from an earlier jump.)
|
||||
#[test]
|
||||
#[ignore = "fails: group_abort leaves children done (undo_now is a no-op on the never-done multi); needs the engine fix"]
|
||||
fn group_abort_undoes_children_repro() {
|
||||
let _lock = GLOBAL_STACK_LOCK.lock().unwrap();
|
||||
common::force_link();
|
||||
assert_eq!(unsafe { oakengine_undo_clear() }, 0);
|
||||
|
||||
|
||||
@@ -230,6 +230,7 @@ fn undo_stack_lifecycle() {
|
||||
assert_eq!(unsafe { oakengine_undo_count() }, 2); // one grouped row
|
||||
|
||||
// Abort path: group with a child is undone and discarded.
|
||||
STK_UNDO_COUNT.store(0, Ordering::SeqCst);
|
||||
assert_eq!(unsafe { oakengine_undo_group_begin(c"abort".as_ptr()) }, 0);
|
||||
let c3 = unsafe {
|
||||
oakengine_undo_command_create(
|
||||
|
||||
@@ -183,6 +183,9 @@ pub struct TrackRippleRemoveAreaCommand {
|
||||
track: CHandle,
|
||||
/// Area to clear.
|
||||
range: TimeRange,
|
||||
/// Whether `prepare` has run (the C ABI command path never calls
|
||||
/// `prepare()` itself, so `redo` derives the operations on first use).
|
||||
prepared: bool,
|
||||
/// Out-point trim on the first block (`timelineundoripple.h` `trim_out_`).
|
||||
trim_out_: Option<TrimOperation>,
|
||||
/// Blocks fully inside the range to remove (`removals_`).
|
||||
@@ -224,6 +227,7 @@ impl TrackRippleRemoveAreaCommand {
|
||||
Self {
|
||||
track,
|
||||
range,
|
||||
prepared: false,
|
||||
trim_out_: None,
|
||||
removals_: Vec::new(),
|
||||
trim_in_: None,
|
||||
@@ -257,11 +261,18 @@ impl TrackRippleRemoveAreaCommand {
|
||||
|
||||
/// `prepare`: compute the trim/remove operations for the range.
|
||||
///
|
||||
/// Mirrors the C++ algorithm; the leading block lookup requires
|
||||
/// `oaknode_track_get_nearest_block_before_or_at`, which the bridge does
|
||||
/// not yet expose, so this currently finds no block and no-ops (see the
|
||||
/// module note).
|
||||
/// Mirrors the C++ algorithm (`oaknode_track_get_nearest_block_before_or_at`
|
||||
/// is exposed by the oaknode bridge; `redo` invokes this on first use
|
||||
/// because the C ABI command path never calls `prepare` itself).
|
||||
pub fn prepare(&mut self) {
|
||||
// Idempotent: recompute from the current track state, discarding any
|
||||
// previously derived operations.
|
||||
self.trim_out_ = None;
|
||||
self.removals_.clear();
|
||||
self.trim_in_ = None;
|
||||
self.insert_previous_ = CHandle::null();
|
||||
self.splice_split_command_ = None;
|
||||
|
||||
let track = hdup(&self.track);
|
||||
let in_ = self.range.in_();
|
||||
let out = self.range.out();
|
||||
@@ -351,10 +362,18 @@ impl TrackRippleRemoveAreaCommand {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
self.prepared = true;
|
||||
}
|
||||
|
||||
/// `redo`: apply the ripple removal.
|
||||
pub fn redo(&mut self) {
|
||||
// The C ABI command path never invokes `prepare()` (the oakundo
|
||||
// vtable wrapper only dispatches redo/undo), so derive the operations
|
||||
// on first use.
|
||||
if !self.prepared {
|
||||
self.prepare();
|
||||
}
|
||||
if self.splice_split_command_.is_some() {
|
||||
// We're just splicing (C++ `redo_now` = prepare + redo)
|
||||
let cmd = self.splice_split_command_.as_mut().unwrap();
|
||||
@@ -905,9 +924,9 @@ impl TimelineRippleDeleteGapsAtRegionsCommand {
|
||||
|
||||
/// `prepare`: build the per-region gap-removal commands.
|
||||
///
|
||||
/// Requires the gap-kind, nearest-block, sequence-track and locked-flag
|
||||
/// queries the bridge does not yet expose, so it currently produces no
|
||||
/// commands (see the module note). The algorithm below mirrors the C++.
|
||||
/// The gap-kind, nearest-block, sequence-track and locked-flag queries
|
||||
/// all go through `bridge::node`; `redo` invokes this on first use
|
||||
/// because the C ABI command path never calls `prepare` itself.
|
||||
pub fn prepare(&mut self) {
|
||||
self.commands_.clear();
|
||||
|
||||
@@ -1070,7 +1089,14 @@ impl TimelineRippleDeleteGapsAtRegionsCommand {
|
||||
}
|
||||
|
||||
/// `redo`: apply the gap deletions.
|
||||
///
|
||||
/// The C ABI command path never invokes `prepare()` (the oakundo vtable
|
||||
/// wrapper only dispatches `redo`/`undo`), so the sub-commands are built
|
||||
/// on first use; later redos re-apply the stored commands.
|
||||
pub fn redo(&mut self) {
|
||||
if self.commands_.is_empty() {
|
||||
self.prepare();
|
||||
}
|
||||
for i in 0..self.commands_.len() {
|
||||
let c = hdup(&self.commands_[i]);
|
||||
let _ = unsafe { oakundo_command_redo_now(c) };
|
||||
|
||||
@@ -74,6 +74,12 @@ impl BlockSplitCommand {
|
||||
|
||||
/// `redo`: shrink `block` to the first half, grow `new_block` to the
|
||||
/// second half, and insert it after `block`.
|
||||
///
|
||||
/// The split keeps the ORIGINAL block anchored at its out-point (it
|
||||
/// becomes the second half `[point, out)`) and anchors the fresh
|
||||
/// `new_block` at its in-point (it becomes the first half
|
||||
/// `[in, point)`), matching the C++ `Block::set_length_and_media_out` /
|
||||
/// `set_length_and_media_in` semantics of the module.
|
||||
pub fn redo(&mut self) {
|
||||
// Create the second half if redo is invoked without a preceding
|
||||
// prepare() (the C ABI command path may call redo directly).
|
||||
@@ -89,13 +95,18 @@ impl BlockSplitCommand {
|
||||
|
||||
// The C++ asserts `point_` lies strictly inside the block; that would
|
||||
// panic across the FFI boundary, so it is intentionally not replicated.
|
||||
let new_length = self.point - block_in;
|
||||
let new_part_length = block_out - self.point;
|
||||
let first_half_length = self.point - block_in;
|
||||
let second_half_length = block_out - self.point;
|
||||
|
||||
let track = block_track(self.block.clone());
|
||||
|
||||
block_set_length_and_media_out(self.block.clone(), new_length);
|
||||
block_set_length_and_media_in(self.new_block.clone(), new_part_length);
|
||||
// Out-anchored length for the second half keeps the original's out
|
||||
// point (the C++ `set_length_and_media_out`); the in-anchored length
|
||||
// for the first half grows the fresh block from its default in of 0
|
||||
// (the C++ `set_length_and_media_in`). The two were previously
|
||||
// swapped, which anchored the halves at the wrong points.
|
||||
block_set_length_and_media_out(self.block.clone(), second_half_length);
|
||||
block_set_length_and_media_in(self.new_block.clone(), first_half_length);
|
||||
|
||||
// SAFETY: bridge inserts `new_block` after `block` on `track`.
|
||||
let _ = unsafe {
|
||||
@@ -201,7 +212,17 @@ impl BlockSplitPreservingLinksCommand {
|
||||
}
|
||||
|
||||
/// `redo`: redo every child command in order.
|
||||
///
|
||||
/// The C ABI command path never invokes `prepare()` (the oakundo vtable
|
||||
/// wrapper only dispatches `redo`/`undo`), so the children are built on
|
||||
/// first redo; `prepare` itself redoes each child as it builds it, so the
|
||||
/// first redo has nothing left to run. Later redos (after an undo) run
|
||||
/// the stored children directly.
|
||||
pub fn redo(&mut self) {
|
||||
if self.commands.is_empty() {
|
||||
self.prepare();
|
||||
return;
|
||||
}
|
||||
for c in self.commands.iter_mut() {
|
||||
c.redo();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user