From 2b89729b293272f7dbc9a7bd973f098c928210fe Mon Sep 17 00:00:00 2001 From: Davide Faconti Date: Sat, 10 Oct 2026 17:52:44 +0200 Subject: [PATCH 1/5] refactor(sdk): reuse the shared colour writer and one resolved-topics helper The ImageAnnotations codec writes colours through builtin_wire::writeColor and normalizedToByte instead of byte-identical local copies; the local decoder stays, because annotation colours default alpha to 1 and skip unknown fields. create() and createV2() share detail::collectResolvedTopics for the count-then-fill retry. Drop the no-op reserved = 0 after value initialisation, and note that snapshot v2 is a full deep copy to acquire when the catalog may have changed, not per tick. Co-Authored-By: Claude Opus 5.5 --- .../include/pj_base/sdk/plugin_data_api.hpp | 96 ++++++++----------- .../src/builtin/image_annotations_codec.cpp | 28 ++---- 2 files changed, 50 insertions(+), 74 deletions(-) diff --git a/pj_base/include/pj_base/sdk/plugin_data_api.hpp b/pj_base/include/pj_base/sdk/plugin_data_api.hpp index e8a49477..47a846dc 100644 --- a/pj_base/include/pj_base/sdk/plugin_data_api.hpp +++ b/pj_base/include/pj_base/sdk/plugin_data_api.hpp @@ -1335,6 +1335,8 @@ class ToolboxHostView { /// in one deep copy. The two halves are NOT read atomically: the scalar /// catalog and the object-topic list are each consistent on their own, but a /// topic created between the two reads may appear in one and not the other. + /// The copy covers every field of every topic: acquire it when the catalog may + /// have changed, not on every timer tick. /// @since 0.36.0 [[nodiscard]] Expected catalogSnapshotV2() const { if (!valid()) { @@ -1684,13 +1686,39 @@ namespace detail { raw.outputs = out_abi.data(); raw.output_count = out_abi.size(); raw.time_flags = time_flags; - raw.reserved = 0; raw.window_start_ns = request.window.has_value() ? request.window->first : 0; raw.window_end_ns = request.window.has_value() ? request.window->second : 0; raw.time_ns = request.instant_ns.value_or(0); return raw; } +/// Count-then-fill of the resolved topic names an upsert returns. Pre-sizes to the +/// declared outputs (8 when none, e.g. an auto-named ephemeral preview) and re-issues +/// `call(out, capacity, &count, &err)` once when the host resolved more: a re-upsert +/// with identical args is idempotent. +template +[[nodiscard]] Expected> collectResolvedTopics(std::size_t declared_outputs, Call&& call) { + const uint64_t capacity = declared_outputs == 0 ? 8 : declared_outputs; + std::vector resolved(capacity); + uint64_t count = 0; + PJ_error_t err{}; + if (!call(resolved.data(), resolved.size(), &count, &err)) { + return unexpected(errorToString(err)); + } + if (count > capacity) { + resolved.assign(count, PJ_string_view_t{}); + if (!call(resolved.data(), resolved.size(), &count, &err)) { + return unexpected(errorToString(err)); + } + } + std::vector topics; + topics.reserve(count); + for (uint64_t i = 0; i < count && i < resolved.size(); ++i) { + topics.emplace_back(toStringView(resolved[i])); + } + return topics; +} + } // namespace detail /// C++ wrapper around PJ_data_processors_host_t for plugins that submit WHOLE-SERIES @@ -1750,40 +1778,14 @@ class DataProcessorsHostView { for (const auto& name : outputs) { out_abi.push_back(toAbiString(name)); } - PJ_error_t err{}; - // Convert the immutable scalar args once — they are identical across the (rare) - // count-then-fill retry below. - const PJ_string_view_t id_abi = toAbiString(id); - const PJ_string_view_t kind_abi = toAbiString(kind); - const PJ_string_view_t language_abi = toAbiString(language); - const PJ_string_view_t script_abi = toAbiString(script); - const PJ_string_view_t params_abi = toAbiString(params_json); - // The resolved sink name(s) are filled on the SAME upsert call. Pre-size to the - // caller's output count (the host resolves exactly that many for non-empty - // outputs); for an auto-named ephemeral preview reserve headroom and grow once if - // the host resolved more than fit — a re-upsert with identical args is idempotent. - uint64_t capacity = out_abi.empty() ? 8 : out_abi.size(); - std::vector resolved(capacity); - uint64_t count = 0; - if (!host_.vtable->create_data_processor( - host_.ctx, id_abi, kind_abi, language_abi, in_abi.data(), in_abi.size(), out_abi.data(), out_abi.size(), - script_abi, params_abi, flags, resolved.data(), resolved.size(), &count, &err)) { - return unexpected(errorToString(err)); - } - if (count > capacity) { - resolved.assign(count, PJ_string_view_t{}); - if (!host_.vtable->create_data_processor( - host_.ctx, id_abi, kind_abi, language_abi, in_abi.data(), in_abi.size(), out_abi.data(), out_abi.size(), - script_abi, params_abi, flags, resolved.data(), resolved.size(), &count, &err)) { - return unexpected(errorToString(err)); - } - } - std::vector topics; - topics.reserve(count); - for (uint64_t i = 0; i < count && i < resolved.size(); ++i) { - topics.emplace_back(toStringView(resolved[i])); - } - return topics; + // The resolved sink name(s) are filled on the SAME upsert call. + return detail::collectResolvedTopics( + out_abi.size(), [&](PJ_string_view_t* out, uint64_t capacity, uint64_t* count, PJ_error_t* err) { + return host_.vtable->create_data_processor( + host_.ctx, toAbiString(id), toAbiString(kind), toAbiString(language), in_abi.data(), in_abi.size(), + out_abi.data(), out_abi.size(), toAbiString(script), toAbiString(params_json), flags, out, capacity, + count, err); + }); } /// Convenience: create a kind="transform" node (DerivedEngine timeseries). `outputs` @@ -1928,25 +1930,10 @@ class DataProcessorsHostView { std::vector in_abi; std::vector out_abi; const PJ_data_processor_request_t raw = detail::toAbiRequest(request, in_abi, out_abi); - PJ_error_t err{}; - uint64_t capacity = out_abi.empty() ? 8 : out_abi.size(); - std::vector resolved(capacity); - uint64_t count = 0; - if (!host_.vtable->create_data_processor_v2(host_.ctx, &raw, resolved.data(), resolved.size(), &count, &err)) { - return unexpected(errorToString(err)); - } - if (count > capacity) { - resolved.assign(count, PJ_string_view_t{}); - if (!host_.vtable->create_data_processor_v2(host_.ctx, &raw, resolved.data(), resolved.size(), &count, &err)) { - return unexpected(errorToString(err)); - } - } - std::vector topics; - topics.reserve(count); - for (uint64_t i = 0; i < count && i < resolved.size(); ++i) { - topics.emplace_back(toStringView(resolved[i])); - } - return topics; + return detail::collectResolvedTopics( + out_abi.size(), [&](PJ_string_view_t* out, uint64_t capacity, uint64_t* count, PJ_error_t* err) { + return host_.vtable->create_data_processor_v2(host_.ctx, &raw, out, capacity, count, err); + }); } /// Start an evaluation and return its handle. `request.id` naming an @@ -1970,7 +1957,6 @@ class DataProcessorsHostView { const PJ_data_processor_request_t raw = detail::toAbiRequest(request, in_abi, out_abi); PJ_evaluation_budget_t raw_budget{}; raw_budget.struct_size = sizeof(PJ_evaluation_budget_t); - raw_budget.reserved = 0; raw_budget.max_millis = budget.max_millis; raw_budget.max_bytes = budget.max_bytes; raw_budget.max_evaluations = budget.max_evaluations; diff --git a/pj_base/src/builtin/image_annotations_codec.cpp b/pj_base/src/builtin/image_annotations_codec.cpp index cfc5ca28..5cf94848 100644 --- a/pj_base/src/builtin/image_annotations_codec.cpp +++ b/pj_base/src/builtin/image_annotations_codec.cpp @@ -3,7 +3,6 @@ #include "pj_base/builtin/image_annotations_codec.hpp" -#include #include #include #include @@ -15,9 +14,11 @@ namespace PJ { namespace { +using builtin_wire::normalizedToByte; using builtin_wire::Reader; using builtin_wire::Tag; using builtin_wire::WireType; +using builtin_wire::writeColor; using builtin_wire::Writer; using sdk::AnnotationTopology; using sdk::CircleAnnotation; @@ -32,13 +33,6 @@ void writePoint2(Writer& writer, const Point2& point) { writer.doubleField(2, point.y); } -void writeAnnotationColor(Writer& writer, const ColorRGBA& color) { - writer.doubleField(1, static_cast(color.r) / 255.0); - writer.doubleField(2, static_cast(color.g) / 255.0); - writer.doubleField(3, static_cast(color.b) / 255.0); - writer.doubleField(4, static_cast(color.a) / 255.0); -} - uint32_t topologyToEnum(AnnotationTopology topology) { switch (topology) { case AnnotationTopology::kPoints: @@ -60,13 +54,13 @@ void writePointsAnnotation(Writer& writer, const PointsAnnotation& points) { writer.message(3, [&](Writer& nested) { writePoint2(nested, point); }); } - writer.message(4, [&](Writer& nested) { writeAnnotationColor(nested, points.color); }); + writer.message(4, [&](Writer& nested) { writeColor(nested, points.color); }); for (const auto& color : points.colors) { - writer.message(5, [&](Writer& nested) { writeAnnotationColor(nested, color); }); + writer.message(5, [&](Writer& nested) { writeColor(nested, color); }); } - writer.message(6, [&](Writer& nested) { writeAnnotationColor(nested, points.fill_color); }); + writer.message(6, [&](Writer& nested) { writeColor(nested, points.fill_color); }); writer.doubleField(7, points.thickness); } @@ -74,15 +68,15 @@ void writeCircleAnnotation(Writer& writer, const CircleAnnotation& circle) { writer.message(2, [&](Writer& nested) { writePoint2(nested, circle.center); }); writer.doubleField(3, circle.radius * 2.0); writer.doubleField(4, circle.thickness); - writer.message(5, [&](Writer& nested) { writeAnnotationColor(nested, circle.fill_color); }); - writer.message(6, [&](Writer& nested) { writeAnnotationColor(nested, circle.color); }); + writer.message(5, [&](Writer& nested) { writeColor(nested, circle.fill_color); }); + writer.message(6, [&](Writer& nested) { writeColor(nested, circle.color); }); } void writeTextAnnotation(Writer& writer, const TextAnnotation& text) { writer.message(2, [&](Writer& nested) { writePoint2(nested, text.position); }); writer.string(3, text.text); writer.doubleField(4, text.font_size); - writer.message(5, [&](Writer& nested) { writeAnnotationColor(nested, text.color); }); + writer.message(5, [&](Writer& nested) { writeColor(nested, text.color); }); } AnnotationTopology mapTopology(uint64_t type) { @@ -101,11 +95,6 @@ AnnotationTopology mapTopology(uint64_t type) { } } -uint8_t normalizedToByte(double value) { - value = std::clamp(value, 0.0, 1.0); - return static_cast(value * 255.0 + 0.5); -} - bool decodePoint2(Reader& reader, Point2& out) { while (!reader.eof()) { Tag tag; @@ -128,6 +117,7 @@ bool decodePoint2(Reader& reader, Point2& out) { return true; } +// Not builtin_wire::decodeColor: annotation colours default alpha to 1 and skip unknown fields. bool decodeAnnotationColor(Reader& reader, ColorRGBA& out) { double r = 0.0; double g = 0.0; From e9134229e476067c06c4c094cfa18553efcdf264 Mon Sep 17 00:00:00 2001 From: Davide Faconti Date: Sat, 10 Oct 2026 17:52:50 +0200 Subject: [PATCH 2/5] refactor(builtin): one payload-assign helper, deduced table sizes, named pixel datatypes - detail::assignPayloadBytes replaces four identical buffer_assign lambdas (PointCloud, Image, DepthImage, VideoFrame). - Every FieldTable::fields array deduces its size: a hand-counted N with too few initialisers left trailing descriptors with null accessors. - Image and DepthImage pixel layouts name PointField::Datatype and derive the byte size from bytesPerElement instead of the magic tags 2, 4 and 7. - PinholeIntrinsics::fromK validates K once, so a per-pixel loop checks only its own inputs; unprojectPixel stays as the one-pixel form. - field_table.hpp says list elements are addressable one by one, so a binder reads list_at lazily instead of materialising the list per access. - The unprojectPixel test moves inside the PJ namespace with its include at the top, and covers PinholeIntrinsics. Co-Authored-By: Claude Opus 5.5 --- .../pj_base/builtin/camera_info_fields.hpp | 2 +- .../pj_base/builtin/depth_image_fields.hpp | 38 ++++++-------- .../pj_base/builtin/depth_image_utils.hpp | 52 +++++++++++++------ .../include/pj_base/builtin/field_table.hpp | 14 ++++- .../builtin/frame_transforms_fields.hpp | 12 ++--- .../builtin/image_annotations_fields.hpp | 12 ++--- .../include/pj_base/builtin/image_fields.hpp | 50 +++++++++--------- .../pj_base/builtin/point_cloud_fields.hpp | 18 ++----- .../pj_base/builtin/scene_entities_fields.hpp | 28 +++++----- .../pj_base/builtin/video_frame_fields.hpp | 12 ++--- pj_base/tests/depth_image_codec_test.cpp | 24 +++++---- 11 files changed, 141 insertions(+), 121 deletions(-) diff --git a/pj_base/include/pj_base/builtin/camera_info_fields.hpp b/pj_base/include/pj_base/builtin/camera_info_fields.hpp index d9e357ef..2ee36b63 100644 --- a/pj_base/include/pj_base/builtin/camera_info_fields.hpp +++ b/pj_base/include/pj_base/builtin/camera_info_fields.hpp @@ -22,7 +22,7 @@ namespace PJ::sdk { /// `distortion_model`). template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&CameraInfo::timestamp_ns>("timestamp_ns"), field<&CameraInfo::frame_id>("frame_id"), field<&CameraInfo::width>("width"), diff --git a/pj_base/include/pj_base/builtin/depth_image_fields.hpp b/pj_base/include/pj_base/builtin/depth_image_fields.hpp index 06931040..7f56b3bc 100644 --- a/pj_base/include/pj_base/builtin/depth_image_fields.hpp +++ b/pj_base/include/pj_base/builtin/depth_image_fields.hpp @@ -12,24 +12,25 @@ #include "pj_base/builtin/depth_image.hpp" #include "pj_base/builtin/field_table.hpp" +#include "pj_base/builtin/point_cloud.hpp" namespace PJ::sdk { namespace detail { -/// Bytes-per-pixel for a `DepthImage::encoding` value, or 0 when the string -/// is not one of the two encodings `depth_image.hpp`'s doc comment documents -/// ("16UC1" — millimeters as uint16 — or "32FC1" — meters as float32). -/// `DepthImage::encoding` is an open string (like `Image::encoding`), so an -/// unrecognized value is expected, not an error. -[[nodiscard]] constexpr uint32_t depthImageBytesPerPixel(std::string_view encoding) noexcept { +/// Pixel datatype of a `DepthImage::encoding` value, or `kUnknown` when the +/// string is not one of the two encodings `depth_image.hpp`'s doc comment +/// documents ("16UC1" — millimeters as uint16 — or "32FC1" — meters as +/// float32). `DepthImage::encoding` is an open string (like `Image::encoding`), +/// so an unrecognized value is expected, not an error. +[[nodiscard]] constexpr PointField::Datatype depthImageDatatype(std::string_view encoding) noexcept { if (encoding == "16UC1") { - return 2; + return PointField::Datatype::kUint16; } if (encoding == "32FC1") { - return 4; + return PointField::Datatype::kFloat32; } - return 0; + return PointField::Datatype::kUnknown; } } // namespace detail @@ -37,19 +38,19 @@ namespace detail { /// `kBuffer` descriptor for `DepthImage::data`. Unlike `Image`, `DepthImage` /// has no `row_step` member, so `buffer()` derives it as `width * bpp` (no /// row padding representable) whenever `bpp` (from -/// `detail::depthImageBytesPerPixel(encoding)`) is known; an unrecognized +/// `bytesPerElement(detail::depthImageDatatype(encoding))`) is known; an unrecognized /// encoding reports `record_step = row_step = 0` and `bytes` alone (its full /// span) is the payload — same convention as `imageDataField()` /// (`image_fields.hpp`). `channels` always carries exactly one entry naming -/// the encoding string. `buffer_assign()` follows the same -/// take-ownership-and-re-anchor idiom as `pointCloudDataField()`. +/// the encoding string. `buffer_assign()` is `detail::assignPayloadBytes`. [[nodiscard]] constexpr FieldDescriptor depthImageDataField(std::string_view name) { FieldDescriptor d{}; d.name = name; d.kind = FieldKind::kBuffer; d.buffer = [](const void* p) -> BufferLayout { const auto& image = *static_cast(p); - const uint32_t bpp = detail::depthImageBytesPerPixel(image.encoding); + const PointField::Datatype datatype = detail::depthImageDatatype(image.encoding); + const uint32_t bpp = bytesPerElement(datatype); BufferLayout layout; layout.bytes = image.data; layout.record_step = bpp; @@ -60,16 +61,11 @@ namespace detail { BufferLayout::Channel{ .name = image.encoding, .offset = 0, - .datatype = bpp == 2 ? uint8_t{4} : (bpp == 4 ? uint8_t{7} : uint8_t{0}), // 4=uint16, 7=float32 + .datatype = static_cast(datatype), .count = bpp == 0 ? 0 : uint32_t{1}}); return layout; }; - d.buffer_assign = [](void* p, std::vector bytes) { - auto& image = *static_cast(p); - const PayloadView view = makePayloadView(std::move(bytes)); - image.data = view.bytes; - image.anchor = view.anchor; - }; + d.buffer_assign = &detail::assignPayloadBytes; return d; } @@ -81,7 +77,7 @@ namespace detail { /// (`std::vector`, size depends on `distortion_model`). template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&DepthImage::width>("width"), field<&DepthImage::height>("height"), field<&DepthImage::encoding>("encoding"), diff --git a/pj_base/include/pj_base/builtin/depth_image_utils.hpp b/pj_base/include/pj_base/builtin/depth_image_utils.hpp index d77506f7..3acd9c85 100644 --- a/pj_base/include/pj_base/builtin/depth_image_utils.hpp +++ b/pj_base/include/pj_base/builtin/depth_image_utils.hpp @@ -23,27 +23,49 @@ namespace PJ { namespace sdk { -/// Unproject a rectified pixel with positive metric depth using a conventional -/// pinhole intrinsic matrix. Rejects singular/nonfinite intrinsics and unsupported -/// skew/projective terms; distortion correction belongs to the caller. -[[nodiscard]] inline std::optional> unprojectPixel( - const std::array& k, double u, double v, double depth_m) noexcept { - for (double value : k) { - if (!std::isfinite(value)) { +/// Conventional pinhole intrinsics, validated once so a per-pixel loop checks only +/// its own inputs. +struct PinholeIntrinsics { + double fx = 0.0; + double fy = 0.0; + double cx = 0.0; + double cy = 0.0; + + /// Rejects singular/nonfinite intrinsics and unsupported skew/projective terms; + /// distortion correction belongs to the caller. + [[nodiscard]] static std::optional fromK(const std::array& k) noexcept { + for (double value : k) { + if (!std::isfinite(value)) { + return std::nullopt; + } + } + if (k[0] <= 0 || k[4] <= 0 || k[1] != 0 || k[3] != 0 || k[6] != 0 || k[7] != 0 || k[8] != 1) { return std::nullopt; } + return PinholeIntrinsics{.fx = k[0], .fy = k[4], .cx = k[2], .cy = k[5]}; } - if (!std::isfinite(u) || !std::isfinite(v) || !std::isfinite(depth_m) || depth_m <= 0 || k[0] <= 0 || k[4] <= 0 || - k[1] != 0 || k[3] != 0 || k[6] != 0 || k[7] != 0 || k[8] != 1) { - return std::nullopt; - } - std::array result{(u - k[2]) * depth_m / k[0], (v - k[5]) * depth_m / k[4], depth_m}; - for (double value : result) { - if (!std::isfinite(value)) { + + /// Unproject a rectified pixel with positive metric depth. + [[nodiscard]] std::optional> unproject(double u, double v, double depth_m) const noexcept { + if (!std::isfinite(u) || !std::isfinite(v) || !std::isfinite(depth_m) || depth_m <= 0) { return std::nullopt; } + std::array result{(u - cx) * depth_m / fx, (v - cy) * depth_m / fy, depth_m}; + for (double value : result) { + if (!std::isfinite(value)) { + return std::nullopt; + } + } + return result; } - return result; +}; + +/// One-pixel form of `PinholeIntrinsics::fromK(k)->unproject(u, v, depth_m)`; a loop +/// over an image should call `fromK` once instead. +[[nodiscard]] inline std::optional> unprojectPixel( + const std::array& k, double u, double v, double depth_m) noexcept { + const auto intrinsics = PinholeIntrinsics::fromK(k); + return intrinsics ? intrinsics->unproject(u, v, depth_m) : std::nullopt; } /// Rectification rotation. For a DepthImage with empty distortion_model diff --git a/pj_base/include/pj_base/builtin/field_table.hpp b/pj_base/include/pj_base/builtin/field_table.hpp index 9c0a7d69..753182cf 100644 --- a/pj_base/include/pj_base/builtin/field_table.hpp +++ b/pj_base/include/pj_base/builtin/field_table.hpp @@ -153,7 +153,8 @@ struct FieldDescriptor { /// For `kind == kList`: element count and element address — populated for /// both a growable list (`std::vector`) and a fixed-size one - /// (`std::array`). + /// (`std::array`). Elements are addressable one by one: a binder should + /// read `list_at(i)` lazily rather than materialize the whole list per access. size_t (*list_size)(const void*) = nullptr; const void* (*list_at)(const void*, size_t) = nullptr; @@ -336,6 +337,17 @@ inline constexpr bool kHasFieldTableImpl = false; template inline constexpr bool kHasFieldTableImpl::view)>> = true; +/// `buffer_assign` of a builtin whose payload is a `data` view plus its `anchor`: +/// takes ownership of `bytes` (`makePayloadView`), `data` views them and `anchor` +/// keeps them alive. +template +void assignPayloadBytes(void* p, std::vector bytes) { + auto& object = *static_cast(p); + const PayloadView view = makePayloadView(std::move(bytes)); + object.data = view.bytes; + object.anchor = view.anchor; +} + } // namespace detail /// True iff `FieldTable` has a specialization (i.e. `T` is describable), diff --git a/pj_base/include/pj_base/builtin/frame_transforms_fields.hpp b/pj_base/include/pj_base/builtin/frame_transforms_fields.hpp index 8a4bc0cc..ba2876b5 100644 --- a/pj_base/include/pj_base/builtin/frame_transforms_fields.hpp +++ b/pj_base/include/pj_base/builtin/frame_transforms_fields.hpp @@ -18,7 +18,7 @@ namespace PJ::sdk { /// Fields of `Vector2`: `x`, `y`. template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&Vector2::x>("x"), field<&Vector2::y>("y"), }; @@ -28,7 +28,7 @@ struct FieldTable { /// Fields of `Vector3`: `x`, `y`, `z`. template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&Vector3::x>("x"), field<&Vector3::y>("y"), field<&Vector3::z>("z"), @@ -39,7 +39,7 @@ struct FieldTable { /// Fields of `Quaternion`: `x`, `y`, `z`, `w`. template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&Quaternion::x>("x"), field<&Quaternion::y>("y"), field<&Quaternion::z>("z"), @@ -51,7 +51,7 @@ struct FieldTable { /// Fields of `Pose`: `position` (Vector3), `orientation` (Quaternion). template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&Pose::position>("position"), field<&Pose::orientation>("orientation"), }; @@ -62,7 +62,7 @@ struct FieldTable { /// `child_frame_id` (strings), `translation` (Vector3), `rotation` (Quaternion). template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&FrameTransform::timestamp>("timestamp"), field<&FrameTransform::parent_frame_id>("parent_frame_id"), field<&FrameTransform::child_frame_id>("child_frame_id"), @@ -75,7 +75,7 @@ struct FieldTable { /// Fields of `FrameTransforms`: `transforms` (list of `FrameTransform`). template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&FrameTransforms::transforms>("transforms"), }; static constexpr FieldTableView view{"FrameTransforms", Span(fields)}; diff --git a/pj_base/include/pj_base/builtin/image_annotations_fields.hpp b/pj_base/include/pj_base/builtin/image_annotations_fields.hpp index 18dd6dff..281c3e84 100644 --- a/pj_base/include/pj_base/builtin/image_annotations_fields.hpp +++ b/pj_base/include/pj_base/builtin/image_annotations_fields.hpp @@ -18,7 +18,7 @@ namespace PJ::sdk { /// Fields of `Point2`: `x`, `y`. template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&Point2::x>("x"), field<&Point2::y>("y"), }; @@ -28,7 +28,7 @@ struct FieldTable { /// Fields of `ColorRGBA`: `r`, `g`, `b`, `a` (each `uint8_t`, read/written as `double`). template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&ColorRGBA::r>("r"), field<&ColorRGBA::g>("g"), field<&ColorRGBA::b>("b"), @@ -41,7 +41,7 @@ struct FieldTable { /// `Point2`), `thickness`, `color`, `colors` (list of `ColorRGBA`), `fill_color`. template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&PointsAnnotation::topology>("topology"), field<&PointsAnnotation::points>("points"), field<&PointsAnnotation::thickness>("thickness"), field<&PointsAnnotation::color>("color"), field<&PointsAnnotation::colors>("colors"), field<&PointsAnnotation::fill_color>("fill_color"), @@ -53,7 +53,7 @@ struct FieldTable { /// `color`, `fill_color`. template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&CircleAnnotation::center>("center"), field<&CircleAnnotation::radius>("radius"), field<&CircleAnnotation::thickness>("thickness"), field<&CircleAnnotation::color>("color"), field<&CircleAnnotation::fill_color>("fill_color"), @@ -64,7 +64,7 @@ struct FieldTable { /// Fields of `TextAnnotation`: `position` (Point2), `font_size`, `color`, `text`. template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&TextAnnotation::position>("position"), field<&TextAnnotation::font_size>("font_size"), field<&TextAnnotation::color>("color"), @@ -77,7 +77,7 @@ struct FieldTable { /// (string), `points`, `circles`, `texts` (lists of the primitive structs above). template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&ImageAnnotations::timestamp>("timestamp"), field<&ImageAnnotations::image_topic>("image_topic"), field<&ImageAnnotations::points>("points"), field<&ImageAnnotations::circles>("circles"), field<&ImageAnnotations::texts>("texts"), diff --git a/pj_base/include/pj_base/builtin/image_fields.hpp b/pj_base/include/pj_base/builtin/image_fields.hpp index 0f380b6e..82c44cb8 100644 --- a/pj_base/include/pj_base/builtin/image_fields.hpp +++ b/pj_base/include/pj_base/builtin/image_fields.hpp @@ -12,21 +12,24 @@ #include "pj_base/builtin/field_table.hpp" #include "pj_base/builtin/image.hpp" +#include "pj_base/builtin/point_cloud.hpp" namespace PJ::sdk { namespace detail { -/// Per-pixel layout of a raw (uncompressed) `Image` encoding: the wire -/// datatype tag (mirrors `PointField::Datatype` — 2 = uint8, 4 = uint16) and -/// how many consecutive values of that datatype make up one pixel. `bytes` -/// is the resulting pixel size, i.e. what `imageDataField()`'s buffer -/// descriptor reports as `record_step`. A compressed or unrecognized -/// encoding has no static per-pixel size, so every field is left 0. +/// Per-pixel layout of a raw (uncompressed) `Image` encoding: the datatype +/// and how many consecutive values of it make up one pixel. `bytes()` is the +/// pixel size, i.e. what `imageDataField()`'s buffer descriptor reports as +/// `record_step`. A compressed or unrecognized encoding has no static +/// per-pixel size: `kUnknown`, count 0. struct ImagePixelLayout { - uint8_t datatype = 0; + PointField::Datatype datatype = PointField::Datatype::kUnknown; uint32_t count = 0; - uint32_t bytes = 0; + + [[nodiscard]] constexpr uint32_t bytes() const noexcept { + return count * bytesPerElement(datatype); + } }; /// Resolves `ImagePixelLayout` for the documented raw encodings ("rgb8", @@ -43,14 +46,14 @@ struct ImagePixelLayout { switch (*parsed) { case CommonImageEncoding::rgb8: case CommonImageEncoding::bgr8: - return {.datatype = 2, .count = 3, .bytes = 3}; // 2 = uint8 + return {.datatype = PointField::Datatype::kUint8, .count = 3}; case CommonImageEncoding::rgba8: case CommonImageEncoding::bgra8: - return {.datatype = 2, .count = 4, .bytes = 4}; // 2 = uint8 + return {.datatype = PointField::Datatype::kUint8, .count = 4}; case CommonImageEncoding::mono8: - return {.datatype = 2, .count = 1, .bytes = 1}; // 2 = uint8 + return {.datatype = PointField::Datatype::kUint8, .count = 1}; case CommonImageEncoding::mono16: - return {.datatype = 4, .count = 1, .bytes = 2}; // 4 = uint16 + return {.datatype = PointField::Datatype::kUint16, .count = 1}; case CommonImageEncoding::jpeg: case CommonImageEncoding::png: case CommonImageEncoding::qoi: @@ -72,9 +75,7 @@ struct ImagePixelLayout { /// encodings only, per `image.hpp`'s doc comment). `channels` always carries /// exactly one entry naming the encoding string, so a consumer that gets a /// zeroed `record_step` still learns which codec/layout produced the bytes. -/// `buffer_assign()` takes ownership of the new bytes the same way -/// `pointCloudDataField()` does (`point_cloud_fields.hpp`): `data` views them -/// and `anchor` keeps them alive. +/// `buffer_assign()` is `detail::assignPayloadBytes`. [[nodiscard]] constexpr FieldDescriptor imageDataField(std::string_view name) { FieldDescriptor d{}; d.name = name; @@ -84,21 +85,20 @@ struct ImagePixelLayout { const detail::ImagePixelLayout pixel = detail::imagePixelLayout(image.encoding); BufferLayout layout; layout.bytes = image.data; - layout.record_step = pixel.bytes; + layout.record_step = pixel.bytes(); layout.record_count = - pixel.bytes == 0 ? 0 : static_cast(image.width) * static_cast(image.height); + pixel.bytes() == 0 ? 0 : static_cast(image.width) * static_cast(image.height); layout.row_step = image.row_step; layout.is_bigendian = image.is_bigendian; layout.channels.push_back( - BufferLayout::Channel{.name = image.encoding, .offset = 0, .datatype = pixel.datatype, .count = pixel.count}); + BufferLayout::Channel{ + .name = image.encoding, + .offset = 0, + .datatype = static_cast(pixel.datatype), + .count = pixel.count}); return layout; }; - d.buffer_assign = [](void* p, std::vector bytes) { - auto& image = *static_cast(p); - const PayloadView view = makePayloadView(std::move(bytes)); - image.data = view.bytes; - image.anchor = view.anchor; - }; + d.buffer_assign = &detail::assignPayloadBytes; return d; } @@ -109,7 +109,7 @@ struct ImagePixelLayout { /// `field_table.hpp`). template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&Image::width>("width"), field<&Image::height>("height"), field<&Image::encoding>("encoding"), diff --git a/pj_base/include/pj_base/builtin/point_cloud_fields.hpp b/pj_base/include/pj_base/builtin/point_cloud_fields.hpp index 9aa31426..834ec007 100644 --- a/pj_base/include/pj_base/builtin/point_cloud_fields.hpp +++ b/pj_base/include/pj_base/builtin/point_cloud_fields.hpp @@ -17,7 +17,7 @@ namespace PJ::sdk { /// Fields of `PointField`: `name`, `offset`, `datatype` (enum), `count`. template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&PointField::name>("name"), field<&PointField::offset>("offset"), field<&PointField::datatype>("datatype"), @@ -30,11 +30,8 @@ struct FieldTable { /// record bytes plus a per-channel layout read from `fields` / `point_step` / /// `row_step` / `width` / `height` / `is_bigendian` (record count is /// `width * height`, with `height == 0` read as one row; a zero `point_step` -/// yields zero records). `buffer_assign()` takes ownership of the new bytes: -/// `data` views them and `anchor` keeps them alive, the same idiom the codec -/// uses (`makePayloadView`). `anchor` itself is not a table field. Concrete on -/// purpose: no other buffer-bearing builtin carries a channel layout, so a -/// generic factory would describe a family of one. +/// yields zero records). `buffer_assign()` is `detail::assignPayloadBytes`. +/// `anchor` itself is not a table field. [[nodiscard]] constexpr FieldDescriptor pointCloudDataField(std::string_view name) { FieldDescriptor d{}; d.name = name; @@ -56,12 +53,7 @@ struct FieldTable { } return layout; }; - d.buffer_assign = [](void* p, std::vector bytes) { - auto& cloud = *static_cast(p); - const PayloadView view = makePayloadView(std::move(bytes)); - cloud.data = view.bytes; - cloud.anchor = view.anchor; - }; + d.buffer_assign = &detail::assignPayloadBytes; return d; } @@ -70,7 +62,7 @@ struct FieldTable { /// `buffer_assign()` and is not listed. template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&PointCloud::width>("width"), field<&PointCloud::height>("height"), field<&PointCloud::point_step>("point_step"), diff --git a/pj_base/include/pj_base/builtin/scene_entities_fields.hpp b/pj_base/include/pj_base/builtin/scene_entities_fields.hpp index ee8787c0..4ffbdccf 100644 --- a/pj_base/include/pj_base/builtin/scene_entities_fields.hpp +++ b/pj_base/include/pj_base/builtin/scene_entities_fields.hpp @@ -25,7 +25,7 @@ namespace PJ::sdk { /// Fields of `Point3`: `x`, `y`, `z`. template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&Point3::x>("x"), field<&Point3::y>("y"), field<&Point3::z>("z"), @@ -36,7 +36,7 @@ struct FieldTable { /// Fields of `KeyValuePair`: `key`, `value`. template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&KeyValuePair::key>("key"), field<&KeyValuePair::value>("value"), }; @@ -47,7 +47,7 @@ struct FieldTable { /// `head_length`, `head_diameter`, `color`. template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&ArrowPrimitive::pose>("pose"), field<&ArrowPrimitive::shaft_length>("shaft_length"), field<&ArrowPrimitive::shaft_diameter>("shaft_diameter"), @@ -61,7 +61,7 @@ struct FieldTable { /// Fields of `CubePrimitive`: `pose`, `size`, `color`. template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&CubePrimitive::pose>("pose"), field<&CubePrimitive::size>("size"), field<&CubePrimitive::color>("color"), @@ -72,7 +72,7 @@ struct FieldTable { /// Fields of `SpherePrimitive`: `pose`, `size`, `color`. template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&SpherePrimitive::pose>("pose"), field<&SpherePrimitive::size>("size"), field<&SpherePrimitive::color>("color"), @@ -84,7 +84,7 @@ struct FieldTable { /// `top_scale`, `color`. template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&CylinderPrimitive::pose>("pose"), field<&CylinderPrimitive::size>("size"), field<&CylinderPrimitive::bottom_scale>("bottom_scale"), @@ -99,7 +99,7 @@ struct FieldTable { /// of `ColorRGBA`), `indices` (scalar list of `uint32_t` -> kNumber). template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&LinePrimitive::type>("type"), field<&LinePrimitive::pose>("pose"), field<&LinePrimitive::thickness>("thickness"), field<&LinePrimitive::scale_invariant>("scale_invariant"), field<&LinePrimitive::points>("points"), field<&LinePrimitive::color>("color"), @@ -113,7 +113,7 @@ struct FieldTable { /// `uint32_t` -> kNumber). template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&TrianglePrimitive::pose>("pose"), field<&TrianglePrimitive::points>("points"), field<&TrianglePrimitive::color>("color"), field<&TrianglePrimitive::colors>("colors"), field<&TrianglePrimitive::indices>("indices"), @@ -125,7 +125,7 @@ struct FieldTable { /// `scale_invariant`, `color`, `text`. template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&TextPrimitive::pose>("pose"), field<&TextPrimitive::billboard>("billboard"), field<&TextPrimitive::font_size>("font_size"), field<&TextPrimitive::scale_invariant>("scale_invariant"), field<&TextPrimitive::color>("color"), field<&TextPrimitive::text>("text"), @@ -136,7 +136,7 @@ struct FieldTable { /// Fields of `AxesPrimitive`: `pose`, `length`, `thickness`, `scale_invariant`. template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&AxesPrimitive::pose>("pose"), field<&AxesPrimitive::length>("length"), field<&AxesPrimitive::thickness>("thickness"), @@ -151,7 +151,7 @@ struct FieldTable { /// `Span`+`BufferAnchor` pair described through a `kBuffer` descriptor such as `pointCloudDataField()` instead). template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&ModelPrimitive::pose>("pose"), field<&ModelPrimitive::scale>("scale"), field<&ModelPrimitive::color>("color"), field<&ModelPrimitive::override_color>("override_color"), field<&ModelPrimitive::url>("url"), field<&ModelPrimitive::media_type>("media_type"), @@ -167,7 +167,7 @@ struct FieldTable { /// `triangles`, `texts`, `models`, `axes`). template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&SceneEntity::timestamp>("timestamp"), field<&SceneEntity::frame_id>("frame_id"), field<&SceneEntity::id>("id"), @@ -190,7 +190,7 @@ struct FieldTable { /// Fields of `SceneEntityDeletion`: `type` (enum), `timestamp` (int64), `id`. template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&SceneEntityDeletion::type>("type"), field<&SceneEntityDeletion::timestamp>("timestamp"), field<&SceneEntityDeletion::id>("id"), @@ -202,7 +202,7 @@ struct FieldTable { /// `deletions` (list of `SceneEntityDeletion`). template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&SceneEntities::entities>("entities"), field<&SceneEntities::deletions>("deletions"), }; diff --git a/pj_base/include/pj_base/builtin/video_frame_fields.hpp b/pj_base/include/pj_base/builtin/video_frame_fields.hpp index 146987c1..fecd3d13 100644 --- a/pj_base/include/pj_base/builtin/video_frame_fields.hpp +++ b/pj_base/include/pj_base/builtin/video_frame_fields.hpp @@ -22,8 +22,7 @@ namespace PJ::sdk { /// naming the codec (`format`), so a consumer that gets a zeroed /// `record_step` still learns which codec produced the bytes — same /// convention as `imageDataField()` (`image_fields.hpp`) for a compressed -/// `Image`. `buffer_assign()` follows the same take-ownership-and-re-anchor -/// idiom as `pointCloudDataField()`. +/// `Image`. `buffer_assign()` is `detail::assignPayloadBytes`. [[nodiscard]] constexpr FieldDescriptor videoFrameDataField(std::string_view name) { FieldDescriptor d{}; d.name = name; @@ -35,12 +34,7 @@ namespace PJ::sdk { layout.channels.push_back(BufferLayout::Channel{.name = frame.format, .offset = 0, .datatype = 0, .count = 0}); return layout; }; - d.buffer_assign = [](void* p, std::vector bytes) { - auto& frame = *static_cast(p); - const PayloadView view = makePayloadView(std::move(bytes)); - frame.data = view.bytes; - frame.anchor = view.anchor; - }; + d.buffer_assign = &detail::assignPayloadBytes; return d; } @@ -49,7 +43,7 @@ namespace PJ::sdk { /// `buffer_assign()` and is not listed. template <> struct FieldTable { - static constexpr std::array fields{ + static constexpr std::array fields{ field<&VideoFrame::timestamp_ns>("timestamp_ns"), field<&VideoFrame::frame_id>("frame_id"), field<&VideoFrame::format>("format"), diff --git a/pj_base/tests/depth_image_codec_test.cpp b/pj_base/tests/depth_image_codec_test.cpp index df106515..15156ba8 100644 --- a/pj_base/tests/depth_image_codec_test.cpp +++ b/pj_base/tests/depth_image_codec_test.cpp @@ -5,10 +5,12 @@ #include +#include #include #include #include +#include "pj_base/builtin/depth_image_utils.hpp" #include "protobuf_wire_test_helpers.hpp" namespace PJ { @@ -68,20 +70,22 @@ TEST(DepthImageCodecTest, RoundTripPlumbBobDistortion) { } } -} // namespace -} // namespace PJ - -#include "pj_base/builtin/depth_image_utils.hpp" - -TEST(DepthImageUtils, UnprojectsRectifiedMetricDepthAndRejectsInvalidIntrinsics) { +TEST(DepthImageUtilsTest, UnprojectsRectifiedMetricDepthAndRejectsInvalidIntrinsics) { std::array k{2, 0, 1, 0, 4, 1, 0, 0, 1}; - auto p = PJ::sdk::unprojectPixel(k, 3, 5, 2); + auto p = sdk::unprojectPixel(k, 3, 5, 2); ASSERT_TRUE(p); EXPECT_EQ(*p, (std::array{2, 2, 2})); - EXPECT_FALSE(PJ::sdk::unprojectPixel(k, 3, 5, 0)); + const auto intrinsics = sdk::PinholeIntrinsics::fromK(k); + ASSERT_TRUE(intrinsics); + EXPECT_EQ(intrinsics->unproject(3, 5, 2), p); + EXPECT_FALSE(sdk::unprojectPixel(k, 3, 5, 0)); k[0] = 0; - EXPECT_FALSE(PJ::sdk::unprojectPixel(k, 3, 5, 2)); + EXPECT_FALSE(sdk::unprojectPixel(k, 3, 5, 2)); k[0] = 2; k[1] = 1; - EXPECT_FALSE(PJ::sdk::unprojectPixel(k, 3, 5, 2)); + EXPECT_FALSE(sdk::unprojectPixel(k, 3, 5, 2)); + EXPECT_FALSE(sdk::PinholeIntrinsics::fromK(k)); } + +} // namespace +} // namespace PJ From 5eeafc2f064035b3d25231799f355e7ac7a39b72 Mon Sep 17 00:00:00 2001 From: Davide Faconti Date: Sat, 10 Oct 2026 17:52:56 +0200 Subject: [PATCH 3/5] fix(sdk): ObjectTopicMetadataBuilder::snapshot(false) clears the key snapshot(false) after snapshot() left pj_snapshot set although the comment promised the incremental default. It now removes the key. The key's comment also called it a "field table" stream, a name taken by FieldTable in the same release; it is a snapshot stream. Co-Authored-By: Claude Opus 5.5 --- pj_base/include/pj_base/sdk/object_topic_metadata.hpp | 8 +++++--- pj_base/tests/object_topic_metadata_test.cpp | 9 ++++++++- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/pj_base/include/pj_base/sdk/object_topic_metadata.hpp b/pj_base/include/pj_base/sdk/object_topic_metadata.hpp index 2431c0ed..e8ccaaa4 100644 --- a/pj_base/include/pj_base/sdk/object_topic_metadata.hpp +++ b/pj_base/include/pj_base/sdk/object_topic_metadata.hpp @@ -18,7 +18,7 @@ namespace PJ::sdk { /// @since 0.21.0 inline constexpr std::string_view kBuiltinObjectTypeMetadataKey = "builtin_object_type"; -/// Canonical metadata key marking a "field table" stream. Value `"true"` on a +/// Canonical metadata key marking a snapshot stream. Value `"true"` on a /// SceneEntities/ImageAnnotations topic means EVERY entry is a complete /// clear-and-replace snapshot, so a stateless consumer may render each entry /// alone without accumulating state across prior entries. Absent (or any @@ -88,12 +88,14 @@ class ObjectTopicMetadataBuilder { } /// Mark this topic as a snapshot stream (see kSnapshotMetadataKey): every - /// entry is a complete clear-and-replace snapshot. Sets the key when - /// `value` is true; leaves it unset (the default, incremental) when false. + /// entry is a complete clear-and-replace snapshot. `false` removes the key + /// again (incremental, the default). /// @since 0.36.0 ObjectTopicMetadataBuilder& snapshot(bool value = true) { if (value) { strings_.insert_or_assign(std::string(kSnapshotMetadataKey), std::string("true")); + } else { + strings_.erase(std::string(kSnapshotMetadataKey)); } return *this; } diff --git a/pj_base/tests/object_topic_metadata_test.cpp b/pj_base/tests/object_topic_metadata_test.cpp index dcd40bfe..0c0bbda1 100644 --- a/pj_base/tests/object_topic_metadata_test.cpp +++ b/pj_base/tests/object_topic_metadata_test.cpp @@ -209,11 +209,18 @@ TEST(ObjectTopicMetadataBuilderTest, SnapshotFlagRoundTrips) { ASSERT_TRUE(snapshot_true); EXPECT_EQ(*snapshot_true, R"({"builtin_object_type":"kSceneEntities","pj_snapshot":"true"})"); - // snapshot(false) is a no-op: the key stays unset (incremental, the default). + // snapshot(false) leaves the key unset (incremental, the default), also after a snapshot(true). const auto snapshot_false = ObjectTopicMetadataBuilder().builtinObjectType(BuiltinObjectType::kImageAnnotations).snapshot(false).build(); ASSERT_TRUE(snapshot_false); EXPECT_EQ(*snapshot_false, R"({"builtin_object_type":"kImageAnnotations"})"); + const auto snapshot_reset = ObjectTopicMetadataBuilder() + .builtinObjectType(BuiltinObjectType::kImageAnnotations) + .snapshot() + .snapshot(false) + .build(); + ASSERT_TRUE(snapshot_reset); + EXPECT_EQ(*snapshot_reset, R"({"builtin_object_type":"kImageAnnotations"})"); // Omitting snapshot() entirely leaves the key unset too. const auto no_snapshot = ObjectTopicMetadataBuilder().builtinObjectType(BuiltinObjectType::kImageAnnotations).build(); From 9409e61f58aaa17d8d691d55d25aa8bc6e16a269 Mon Sep 17 00:00:00 2001 From: Davide Faconti Date: Sat, 10 Oct 2026 17:52:57 +0200 Subject: [PATCH 4/5] test(sdk): share the wire-bytes comparison and the test manifest field_table_test compares serialized bytes through expectSameWireBytes instead of nine copies of the same five lines. plugin_catalog_test builds its manifests with manifestWith(key, value) on one minimal manifest constant. Co-Authored-By: Claude Opus 5.5 --- pj_base/tests/field_table_test.cpp | 65 ++++++++---------------- pj_plugins/tests/plugin_catalog_test.cpp | 42 ++++++++------- 2 files changed, 40 insertions(+), 67 deletions(-) diff --git a/pj_base/tests/field_table_test.cpp b/pj_base/tests/field_table_test.cpp index eadd3f70..07d09117 100644 --- a/pj_base/tests/field_table_test.cpp +++ b/pj_base/tests/field_table_test.cpp @@ -168,6 +168,17 @@ void copyThroughTable(const FieldTableView& table, const void* src, void* dst) { } } +// Serializes both objects through the canonical wire codec and expects identical +// bytes: the comparison for builtins whose payload has no meaningful operator==. +template +void expectSameWireBytes(const T& src, const T& dst) { + auto src_bytes = PJ::serializeBuiltinObject(PJ::sdk::BuiltinObject(src)); + auto dst_bytes = PJ::serializeBuiltinObject(PJ::sdk::BuiltinObject(dst)); + ASSERT_TRUE(src_bytes) << src_bytes.error(); + ASSERT_TRUE(dst_bytes) << dst_bytes.error(); + EXPECT_EQ(*src_bytes, *dst_bytes); +} + // --------------------------------------------------------------------------- // Sample data — populated, so every accessor path is exercised. // --------------------------------------------------------------------------- @@ -603,11 +614,7 @@ TEST(FieldTableTest, GenericCopyMatchesCodecRoundTrip) { copyThroughTable(FieldTable::view, &src, &dst); EXPECT_EQ(dst, src); - auto src_bytes = PJ::serializeBuiltinObject(PJ::sdk::BuiltinObject(src)); - auto dst_bytes = PJ::serializeBuiltinObject(PJ::sdk::BuiltinObject(dst)); - ASSERT_TRUE(src_bytes) << src_bytes.error(); - ASSERT_TRUE(dst_bytes) << dst_bytes.error(); - EXPECT_EQ(*src_bytes, *dst_bytes); + expectSameWireBytes(src, dst); } { const ImageAnnotations src = makeSampleImageAnnotations(); @@ -618,11 +625,7 @@ TEST(FieldTableTest, GenericCopyMatchesCodecRoundTrip) { copyThroughTable(FieldTable::view, &src, &dst); EXPECT_EQ(dst, src); - auto src_bytes = PJ::serializeBuiltinObject(PJ::sdk::BuiltinObject(src)); - auto dst_bytes = PJ::serializeBuiltinObject(PJ::sdk::BuiltinObject(dst)); - ASSERT_TRUE(src_bytes) << src_bytes.error(); - ASSERT_TRUE(dst_bytes) << dst_bytes.error(); - EXPECT_EQ(*src_bytes, *dst_bytes); + expectSameWireBytes(src, dst); } { const SceneEntities src = makeSampleSceneEntities(); @@ -632,11 +635,7 @@ TEST(FieldTableTest, GenericCopyMatchesCodecRoundTrip) { copyThroughTable(FieldTable::view, &src, &dst); EXPECT_EQ(dst, src); - auto src_bytes = PJ::serializeBuiltinObject(PJ::sdk::BuiltinObject(src)); - auto dst_bytes = PJ::serializeBuiltinObject(PJ::sdk::BuiltinObject(dst)); - ASSERT_TRUE(src_bytes) << src_bytes.error(); - ASSERT_TRUE(dst_bytes) << dst_bytes.error(); - EXPECT_EQ(*src_bytes, *dst_bytes); + expectSameWireBytes(src, dst); } } @@ -659,11 +658,7 @@ TEST(FieldTableTest, ImageBufferAndOptionalRoundTrip) { EXPECT_TRUE(min_field->has_value(&dst)); EXPECT_FLOAT_EQ(static_cast(min_field->get_number(&dst)), *src.compressed_depth_min); - auto src_bytes = PJ::serializeBuiltinObject(PJ::sdk::BuiltinObject(src)); - auto dst_bytes = PJ::serializeBuiltinObject(PJ::sdk::BuiltinObject(dst)); - ASSERT_TRUE(src_bytes) << src_bytes.error(); - ASSERT_TRUE(dst_bytes) << dst_bytes.error(); - EXPECT_EQ(*src_bytes, *dst_bytes); + expectSameWireBytes(src, dst); } // An Image with both optionals absent (the common case: only @@ -682,11 +677,7 @@ TEST(FieldTableTest, ImageOptionalAbsentRoundTrip) { ASSERT_NE(min_field, nullptr); EXPECT_FALSE(min_field->has_value(&dst)); - auto src_bytes = PJ::serializeBuiltinObject(PJ::sdk::BuiltinObject(src)); - auto dst_bytes = PJ::serializeBuiltinObject(PJ::sdk::BuiltinObject(dst)); - ASSERT_TRUE(src_bytes) << src_bytes.error(); - ASSERT_TRUE(dst_bytes) << dst_bytes.error(); - EXPECT_EQ(*src_bytes, *dst_bytes); + expectSameWireBytes(src, dst); } // DepthImage has no operator== either; exercises kBuffer (data, derived @@ -708,11 +699,7 @@ TEST(FieldTableTest, DepthImageFixedArrayAndBufferRoundTrip) { EXPECT_EQ(dst.K, src.K); EXPECT_EQ(dst.D, src.D); - auto src_bytes = PJ::serializeBuiltinObject(PJ::sdk::BuiltinObject(src)); - auto dst_bytes = PJ::serializeBuiltinObject(PJ::sdk::BuiltinObject(dst)); - ASSERT_TRUE(src_bytes) << src_bytes.error(); - ASSERT_TRUE(dst_bytes) << dst_bytes.error(); - EXPECT_EQ(*src_bytes, *dst_bytes); + expectSameWireBytes(src, dst); } // CameraInfo has operator== (no byte blob), so both the value comparison and @@ -737,11 +724,7 @@ TEST(FieldTableTest, CameraInfoFixedArrayRoundTrip) { copyThroughTable(FieldTable::view, &src, &dst); EXPECT_EQ(dst, src); - auto src_bytes = PJ::serializeBuiltinObject(PJ::sdk::BuiltinObject(src)); - auto dst_bytes = PJ::serializeBuiltinObject(PJ::sdk::BuiltinObject(dst)); - ASSERT_TRUE(src_bytes) << src_bytes.error(); - ASSERT_TRUE(dst_bytes) << dst_bytes.error(); - EXPECT_EQ(*src_bytes, *dst_bytes); + expectSameWireBytes(src, dst); } // VideoFrame has no operator== either; exercises kBuffer for a format with @@ -762,11 +745,7 @@ TEST(FieldTableTest, VideoFrameBufferRoundTrip) { VideoFrame dst; copyThroughTable(FieldTable::view, &src, &dst); - auto src_bytes = PJ::serializeBuiltinObject(PJ::sdk::BuiltinObject(src)); - auto dst_bytes = PJ::serializeBuiltinObject(PJ::sdk::BuiltinObject(dst)); - ASSERT_TRUE(src_bytes) << src_bytes.error(); - ASSERT_TRUE(dst_bytes) << dst_bytes.error(); - EXPECT_EQ(*src_bytes, *dst_bytes); + expectSameWireBytes(src, dst); } // The buffer descriptor's layout for a raw, uncompressed encoding: a 4x3 @@ -864,11 +843,7 @@ TEST(FieldTableTest, PointCloudBufferRoundTrip) { PointCloud dst; copyThroughTable(FieldTable::view, &src, &dst); - auto src_bytes = PJ::serializeBuiltinObject(PJ::sdk::BuiltinObject(src)); - auto dst_bytes = PJ::serializeBuiltinObject(PJ::sdk::BuiltinObject(dst)); - ASSERT_TRUE(src_bytes) << src_bytes.error(); - ASSERT_TRUE(dst_bytes) << dst_bytes.error(); - EXPECT_EQ(*src_bytes, *dst_bytes); + expectSameWireBytes(src, dst); } TEST(FieldTableTest, PointCloudOrganizedBufferLayout) { diff --git a/pj_plugins/tests/plugin_catalog_test.cpp b/pj_plugins/tests/plugin_catalog_test.cpp index 290de63b..6f944fac 100644 --- a/pj_plugins/tests/plugin_catalog_test.cpp +++ b/pj_plugins/tests/plugin_catalog_test.cpp @@ -40,9 +40,16 @@ std::string pluginFileName(const std::string& stem) { #endif } +constexpr std::string_view kMinimalManifest = R"({"id":"sdk-test","name":"SDK Test","version":"1.0.0"})"; + +// kMinimalManifest plus one `"key":json_value` member. +std::string manifestWith(std::string_view key, std::string_view json_value) { + return std::string(kMinimalManifest.substr(0, kMinimalManifest.size() - 1)) + ",\"" + std::string(key) + + "\":" + std::string(json_value) + "}"; +} + std::string manifestWithMinSdk(std::string_view json_value) { - return std::string(R"({"id":"sdk-test","name":"SDK Test","version":"1.0.0","min_sdk_required":)") + - std::string(json_value) + "}"; + return manifestWith("min_sdk_required", json_value); } class PluginCatalogTest : public ::testing::Test { @@ -296,44 +303,35 @@ TEST_F(PluginCatalogTest, SuggestedSdkVersionRoundTripsAndDefaultsEmpty) { } TEST_F(PluginCatalogTest, BadgeRoundTripsAndDefaultsEmpty) { - auto with_field = decodeManifest( - "static:sdk-test", PluginFamily::kToolbox, - R"({"id":"sdk-test","name":"SDK Test","version":"1.0.0","badge":"AI"})"); + auto with_field = decodeManifest("static:sdk-test", PluginFamily::kToolbox, manifestWith("badge", R"("AI")")); ASSERT_TRUE(with_field.has_value()) << with_field.error(); EXPECT_EQ(with_field->badge, "AI"); - auto without_field = decodeManifest( - "static:sdk-test", PluginFamily::kToolbox, R"({"id":"sdk-test","name":"SDK Test","version":"1.0.0"})"); + auto without_field = decodeManifest("static:sdk-test", PluginFamily::kToolbox, kMinimalManifest); ASSERT_TRUE(without_field.has_value()) << without_field.error(); EXPECT_TRUE(without_field->badge.empty()); - auto wrong_type = decodeManifest( - "static:sdk-test", PluginFamily::kToolbox, R"({"id":"sdk-test","name":"SDK Test","version":"1.0.0","badge":3})"); - EXPECT_FALSE(wrong_type.has_value()); + EXPECT_FALSE(decodeManifest("static:sdk-test", PluginFamily::kToolbox, manifestWith("badge", "3")).has_value()); } TEST_F(PluginCatalogTest, CustomTopicsEditorRoundTripsAndDefaultsFalse) { - auto declared = decodeManifest( - "static:sdk-test", PluginFamily::kToolbox, - R"({"id":"sdk-test","name":"SDK Test","version":"1.0.0","custom_topics_editor":true})"); + auto declared = + decodeManifest("static:sdk-test", PluginFamily::kToolbox, manifestWith("custom_topics_editor", "true")); ASSERT_TRUE(declared.has_value()) << declared.error(); EXPECT_TRUE(declared->custom_topics_editor); - auto explicit_false = decodeManifest( - "static:sdk-test", PluginFamily::kToolbox, - R"({"id":"sdk-test","name":"SDK Test","version":"1.0.0","custom_topics_editor":false})"); + auto explicit_false = + decodeManifest("static:sdk-test", PluginFamily::kToolbox, manifestWith("custom_topics_editor", "false")); ASSERT_TRUE(explicit_false.has_value()) << explicit_false.error(); EXPECT_FALSE(explicit_false->custom_topics_editor); - auto absent = decodeManifest( - "static:sdk-test", PluginFamily::kToolbox, R"({"id":"sdk-test","name":"SDK Test","version":"1.0.0"})"); + auto absent = decodeManifest("static:sdk-test", PluginFamily::kToolbox, kMinimalManifest); ASSERT_TRUE(absent.has_value()) << absent.error(); EXPECT_FALSE(absent->custom_topics_editor); - auto wrong_type = decodeManifest( - "static:sdk-test", PluginFamily::kToolbox, - R"({"id":"sdk-test","name":"SDK Test","version":"1.0.0","custom_topics_editor":"yes"})"); - EXPECT_FALSE(wrong_type.has_value()); + EXPECT_FALSE( + decodeManifest("static:sdk-test", PluginFamily::kToolbox, manifestWith("custom_topics_editor", R"("yes")")) + .has_value()); } TEST_F(PluginCatalogTest, MinSdkRequiredAcceptsEmptyString) { From 7793f4d5a1b8c8d2f5304ee31318466f07da25da Mon Sep 17 00:00:00 2001 From: Davide Faconti Date: Sat, 10 Oct 2026 17:53:05 +0200 Subject: [PATCH 5/5] docs(sdk): describe 0.36.0 as its net change, and fix the stale contract text - CHANGELOG 0.36.0 lists what changed since 0.35.0. Entries narrating the unreleased line's own history (a rename, a struct that "grew", text the previous draft "would have" broken) are gone, and so is the contradiction between the 64-handle / 64 MiB numbers and "host policy". The Host contract line is unchanged. - The toolbox guide no longer states those limits as numbers, and says createV2 serves every kind. - LIFETIME no longer says "exactly one of three" before listing four: a pinned node can also be history-exempt. - The capability rule keeps one statement (plugin_data_api.h) and the guide's table; ARCHITECTURE.md and the plugin skill point to them. - Fix the stale skill note that the annotations codec drops image_topic and the timestamp, the reference to a buffer<>() factory that does not exist, and the hand-kept list of tabled types (describe() is the list). - Note that a zero ImageAnnotations timestamp reads as absent, that an empty scene_view kind clears the view, and add the new helpers to sdk-utilities. Co-Authored-By: Claude Opus 5.5 --- .../references/builtin-objects.md | 6 +- .../plotjuggler-plugin/references/toolbox.md | 16 +- CHANGELOG.md | 249 ++++++------------ docs/builtin_type.md | 11 +- docs/image_annotations_format.md | 2 +- docs/sdk-utilities.md | 4 + pj_base/CLAUDE.md | 3 +- pj_base/include/pj_base/plugin_data_api.h | 3 +- .../include/pj_plugins/sdk/widget_data.hpp | 1 + pj_plugins/docs/ARCHITECTURE.md | 5 +- pj_plugins/docs/toolbox-guide.md | 13 +- 11 files changed, 111 insertions(+), 202 deletions(-) diff --git a/.claude/skills/plotjuggler-plugin/references/builtin-objects.md b/.claude/skills/plotjuggler-plugin/references/builtin-objects.md index 8c62eda4..21aced34 100644 --- a/.claude/skills/plotjuggler-plugin/references/builtin-objects.md +++ b/.claude/skills/plotjuggler-plugin/references/builtin-objects.md @@ -114,10 +114,8 @@ exactly once. Do not keep a raw pointer into anything the closure captured. - `ImageAnnotations.image_topic` explicitly names the base image topic; camera calibration correlates by **name convention** (`/camera_info` ↔ `/image_raw`), not by a reference. -- **Association fields ride *outside* the serialized bytes.** The annotations - codec deliberately does not encode `image_topic` (or a timestamp) into the - payload — the store keys entries by (topic, timestamp), and association is a - topic/metadata convention. Don't expect a codec round-trip to carry them. +- The annotations codec carries the top-level `timestamp` (only when non-zero) and + `image_topic` (only when non-empty); see `docs/image_annotations_format.md`. ## Codec round-trips are not bit-exact diff --git a/.claude/skills/plotjuggler-plugin/references/toolbox.md b/.claude/skills/plotjuggler-plugin/references/toolbox.md index 6d271c35..4e063655 100644 --- a/.claude/skills/plotjuggler-plugin/references/toolbox.md +++ b/.claude/skills/plotjuggler-plugin/references/toolbox.md @@ -80,19 +80,9 @@ PJ_TOOLBOX_PLUGIN(MyToolbox, ## Detecting host features (SDK 0.36.0) -One rule, in the header comment of `pj_base/plugin_data_api.h`; never test a -version string: - -- ABI service feature: a named `hasX()` on the view - (`DataProcessorsHostView::hasTypedRequests()`, `PlotTabHostView::hasSceneTabs()`, - `ToolboxHostView::hasCatalogSnapshotV2()`). -- Flag-bit feature (`PJ_DATA_PROCESSOR_FLAG_INFER_OUTPUTS`): do not probe; an older - host rejects the bit. Its floor is the `hasX()` of the slot carrying it. -- Build-dependent behaviour (Python for on_demand): probe by doing it, - `validateScript("on_demand", "python", "return {}")`. -- Dialog feature (`scene_view`): `hostHas(DialogHostCapability::kEmbedsSceneViews)` - inside the dialog. -- Manifest metadata (`badge`, `custom_topics_editor`): declarative, no probe. +Never test a version string. The rule is in the header comment of +`pj_base/plugin_data_api.h`; `pj_plugins/docs/toolbox-guide.md` → "Detecting what +the host can do" has the table with one example per feature kind. On-demand requests: `createV2` returns `//` for object and number outputs (read a number series by exactly that string) and the bare name for a diff --git a/CHANGELOG.md b/CHANGELOG.md index 48460b97..51cc204d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,174 +7,91 @@ All notable changes to `plotjuggler_sdk` are recorded here. Versioning policy is Host contract: extended: PJ_toolbox_host_vtable_t::acquire_catalog_snapshot_v2, PJ_data_processors_host_vtable_t::create_data_processor_v2, PJ_data_processors_host_vtable_t::submit_evaluation, PJ_data_processors_host_vtable_t::poll_evaluation, PJ_data_processors_host_vtable_t::release_evaluation, pj.plot_tabs.v1 tail slots (create_tab_v2, attach_topic, detach_topic, focus_tab), PJ_DATA_PROCESSOR_TIME_FLAG_WINDOW, PJ_DATA_PROCESSOR_TIME_FLAG_INSTANT, PJ_DATA_PROCESSOR_FLAG_INFER_OUTPUTS (floor 0.36.0) -- Add `uint64_t reserved[2]` (must be 0) to `PJ_data_processor_output_t`, which is - an array element with a fixed stride: the struct grows from 32 to 48 bytes. The - C++ wrapper zeroes it; a host rejects an output with nonzero `reserved`. -- Deprecate the `":"` output suffix of `create_data_processor`: it - still works, new clients use `create_data_processor_v2` with - `PJ_data_processor_output_t`; it will be removed in a future version. -- Evaluation budgets and live-handle / report-byte limits are host policy: `0` = - host default, each field is clamped to a host-defined maximum, and a submit is - never rejected for asking more (`coverage.stopped` says which budget ended the - run). The documented default and cap numbers are removed. Docs only. -- Document the unknown-value rule of the evaluation report: an unrecognised string - value in a key the client knows (e.g. `coverage.stopped`, `outputs[].status`) - is an error, never success; `outputs[].type` `"unknown"` is a defined value. - The names inferred by `INFER_OUTPUTS` are chosen by the host and must be read - from the report. Docs only. -- Document the optional `done` / `total` keys of the on_demand `series` block of - `data_processor_config` (instants evaluated / known total); a client tolerates - their absence. Docs only. -- Document the visibility rule for ephemeral data processors: `list_data_processor_ids` - hides them, but `data_processor_config` by exact id answers the owning plugin - (hosts no longer reject it), so a plugin can read its preview's `series` progress. - Docs only: no ABI change. -- Document the `chart_auto_zoom` semantics (`setChartAutoZoom`): omitted fits until - the user zooms or pans, `true` fits now and resumes auto-fit, `false` keeps the - user's view. Docs only: no ABI change. +Data processors (`pj.data_processors.v1`): + +- Add `kind="on_demand"`: objects or numbers evaluated at a consumer-requested time. + `out_topics` returns `//` for object and number outputs (a number + output's series key; absent from the catalog when the recipe cannot run in series + mode) and the bare name for string outputs; read results by the returned string. + Python is an optional per-host on_demand language, probed with `validateScript`. + `create_data_processor` (v1) accepts on_demand outputs as `":"`; that + suffix is deprecated, new clients use `create_data_processor_v2`. +- Add the typed request: `PJ_data_processor_request_t`, `PJ_data_processor_output_t` + (with `uint64_t reserved[2]`, must be 0), `PJ_evaluation_budget_t`, and + `create_data_processor_v2` (`DataProcessorsHostView::createV2`): typed outputs, a + label and, with `PJ_DATA_PROCESSOR_TIME_FLAG_INSTANT`, a pinned evaluation time. + `struct_size` is read-prefix: a host rejects a size below + `PJ_DATA_PROCESSOR_REQUEST_V1_MIN_SIZE` / `PJ_EVALUATION_BUDGET_V1_MIN_SIZE` and + accepts a larger one; a later field is announced by a `flags`/`time_flags` bit. +- Add asynchronous evaluation: `submit_evaluation` / `poll_evaluation` / + `release_evaluation` (`submitEvaluation` / `pollEvaluation` / `releaseEvaluation`) + with `PJ_DATA_PROCESSOR_TIME_FLAG_WINDOW` / `INSTANT`. `poll_evaluation` is the only + way to read a result, as a JSON report (`{"coverage":{...},"bundles":[...]}`, one + bundle per evaluated instant, `*_ns` as raw int64 dataset nanoseconds). + `coverage.stopped` says why a run ended, `coverage.error` is present only when it is + `"error"`, `coverage.gaps` lists retention gaps. An unrecognised value of a known key + is an error, never success; `pollEvaluation` reports an unknown state as an error. + Budgets: `0` = host default, each field clamped to a host maximum, never rejected for + asking more. Live handles and retained report bytes are bounded by host policy; + polling needs the host event loop to turn. +- Add `PJ_DATA_PROCESSOR_FLAG_INFER_OUTPUTS` (on_demand): a transient evaluation with + no declared outputs reports the inferred names and types in a root `"outputs"` + array; on create the declared outputs are a binding hint learned from such a trial. +- Add `DataProcessorsHostView::hasTypedRequests()`: true iff the host serves the four + typed-request slots. +- Document node lifetime by flag and kind (persistent, EPHEMERAL, HISTORY_EXEMPT, + pinned); the per-kind meaning of `label`, WINDOW and INSTANT; that + `data_processor_config` answers the owning plugin for its ephemeral ids (while + `list_data_processor_ids` hides them), reports `history_exempt` for every kind and, + for on_demand, an optional `series` block with `done` / `total`. + +Toolbox host and plot tabs: + +- Add catalog snapshot v2: `acquire_catalog_snapshot_v2` + (`ToolboxHostView::catalogSnapshotV2`, `hasCatalogSnapshotV2()`) returns the scalar + catalog plus every object topic (`PJ_object_topic_info_t`: dataset, builtin type, + entry count, raw time range). The two halves are not read atomically. +- Scene 3D/2D tabs are kinds of `pj.plot_tabs.v1` tabs: tail slots `create_tab_v2`, + `attach_topic`, `detach_topic`, `focus_tab` (`PlotTabHostView::createTabV2` / + `attachTopic` / `detachTopic` / `focusTab` / `hasSceneTabs`), NULL when the host has + no scene workspace. The plot `tab_config` JSON is unchanged; re-creating a plot tab + keeps the host layout and clears its curves; `attach_topic` is idempotent. +- State the capability-detection rule once, in the header comment of + `plugin_data_api.h`. + +Dialogs and manifest: + - Add `WidgetData::setSceneView` / `setSceneTopics` / `clearSceneView` (keys - `scene_view`, `scene_topics`) and the matching `WidgetDataView::sceneView` / - `sceneTopics`: a QFrame carrying `scene_view` becomes an embedded 3D/2D object - view bound to object topics and following the playback cursor. Hosts without - support leave the frame empty. Dialog protocol only: no ABI change. -- Add `PJ_DATA_PROCESSOR_FLAG_INFER_OUTPUTS` (on_demand): a transient evaluation - with no declared outputs infers each output's name and type from the script's - returned value and reports them in a root `"outputs"` array of the - `poll_evaluation` report; on create/create_v2 the declared outputs are a - binding hint learned from such a trial. -- Add compile-time field tables (`pj_base/builtin/field_table.hpp`) describing - the members of builtin object structs, so a generic binder (e.g. a script - engine) can read/write any described field by name without per-type glue - code. Per-type specializations live in `frame_transforms_fields.hpp`, - `image_annotations_fields.hpp`, `point_cloud_fields.hpp`, and - `scene_entities_fields.hpp`; `field_table_registry.hpp` exposes - `describe(BuiltinObjectType)` to look one up from the runtime tag carried - by a `BuiltinObject`. `FrameTransforms`, `ImageAnnotations`, `PointCloud`, - and `SceneEntities` were the first four tabled types (the next entry adds four - more); `PointCloud` - exposes its packed-bytes `data` field through a `kBuffer` descriptor - (`buffer<>()`) resolving a `BufferLayout` view rather than a plain - get/set pair. Client-side only: no ABI or wire-format change. -- Add field tables for `Image`, `DepthImage`, `CameraInfo`, and `VideoFrame` - (`image_fields.hpp`, `depth_image_fields.hpp`, `camera_info_fields.hpp`, - `video_frame_fields.hpp`); `describe(BuiltinObjectType)` now covers 8 - types (there is no count constant: `FieldTableTest.DescribeCoversExactlyTheTabledTypes` - pins the set). `Image::data`/`DepthImage::data`/`VideoFrame::data` each get a - `kBuffer` descriptor whose layout is derived from the encoding/format - string: a raw `Image` encoding (e.g. "rgb8") or a recognized `DepthImage` - encoding ("16UC1"/"32FC1") resolves a static per-pixel `record_step`; - a compressed encoding or a `VideoFrame`'s codec bitstream has none, so - `record_step`/`record_count` are 0 and the encoding/format string still - comes through the buffer's sole channel name. `FieldKind` gains two new - cases to describe these structs honestly rather than faking them: - `kOptionalNumber` (a nullable number, `has_value()` + `get_number`/ - `set_number`, for `Image::compressed_depth_min`/`compressed_depth_max`) - and a fixed-size `kList` (for a `std::array` member — e.g. - `CameraInfo::K`/`R`/`P`, `DepthImage::K` — written through the new - `list_replace` accessor instead of `list_emplace`/`list_clear`, since the - element count never changes). Client-side only: no ABI or wire-format - change. -- Add catalog snapshot v2: `PJ_toolbox_host_vtable_t::acquire_catalog_snapshot_v2` - (`ToolboxHostView::catalogSnapshotV2`) returns a second, ABI-VERSIONED - snapshot struct (`PJ_catalog_snapshot_v2_t`) carrying the scalar catalog of - `acquire_catalog_snapshot` PLUS every object topic — dataset, builtin type, - entry count, raw time range (`PJ_object_topic_info_t`). A new struct rather - than a tail-append, because object topics are an ARRAY ELEMENT with a FIXED - STRIDE: a field that cannot be zero-defaulted needs a new struct + slot, - never a layout change to an existing one. -- Add the typed data-processor request vocabulary (`PJ_data_processor_request_t`, - `PJ_data_processor_output_t`, `PJ_evaluation_budget_t`) and - `PJ_data_processors_host_vtable_t::create_data_processor_v2` - (`DataProcessorsHostView::createV2`): typed outputs, a human-readable label, - and — with `PJ_DATA_PROCESSOR_TIME_FLAG_INSTANT` — a pinned evaluation time - for an on_demand finding. This is the ABI surface the `kind="on_demand"` - doc-comment on `PJ_data_processors_host_vtable_t` previously described as - not existing yet; `create_data_processor` (v1) stays valid for the - `":"` output-suffix grammar. -- Add asynchronous on_demand evaluation: `submit_evaluation`/`poll_evaluation`/ - `release_evaluation` (`DataProcessorsHostView::submitEvaluation`/ - `pollEvaluation`/`releaseEvaluation`), gated by the new - `PJ_DATA_PROCESSOR_TIME_FLAG_WINDOW`/`PJ_DATA_PROCESSOR_TIME_FLAG_INSTANT` - bits. A submitted evaluation returns a handle; a host may complete the work - before returning or in the background, and `poll_evaluation` is the only way - to read the result either way, as a JSON report - (`{"coverage":{...},"bundles":[...]}` — one bundle per requested instant, one - entry per output, `*_ns` values as raw int64 dataset nanoseconds). -- Add `DataProcessorsHostView::hasTypedRequests()`: true iff the host serves - `create_data_processor_v2` and `submit_evaluation`/`poll_evaluation`/ - `release_evaluation`, so a plugin can gate typed-request UI without probing. -- Scene 3D/2D tabs are kinds of `pj.plot_tabs.v1` tabs: tail slots - `create_tab_v2`, `attach_topic`, `detach_topic`, `focus_tab` (C++: - `PlotTabHostView::createV2/attachTopic/detachTopic/focus/hasSceneTabs`). The - slots are NULL when the host has no scene workspace; the plot `tab_config` - JSON is unchanged. -- Add the `pj_snapshot` object-topic metadata key - (`ObjectTopicMetadataBuilder::snapshot`): marks a SceneEntities/ - ImageAnnotations topic whose every entry is a complete clear-and-replace - snapshot, so a stateless consumer may render each entry alone without - replaying the topic's history. -- Add the optional manifest string `badge` (`PluginDescriptor::badge`, "" when - absent): a short label a host may show next to objects the plugin creates. - Older manifests and hosts are unaffected. -- Record the `struct_size` rule of the typed data-processor request and of - `PJ_evaluation_budget_t` as READ-PREFIX: a host reads the prefix it knows, - rejects a `struct_size` below the v1 layout and ACCEPTS a larger one; a field - appended later is zero-defaultable and announced by a new `flags`/`time_flags` - bit, so an older host rejects the bit, never the size. The previous text - (reject any `struct_size` larger than the host's own) would have made the - first appended field break every newer plugin on a 0.36 host. The C++ - wrapper keeps sending `sizeof`, which is correct under this rule. -- `create_data_processor_v2` / `create_data_processor` (`kind="on_demand"`): - `out_topics` now returns `//` for number outputs as well as - object outputs (the series key of series mode; absent from the catalog when - the recipe cannot run in series mode); string outputs stay the bare name. - A reader must use the returned string, not rebuild it from the output name. -- `DataProcessorsHostView::pollEvaluation` returns an error for an unknown - evaluation state ("unknown evaluation state N") instead of reading it as - pending, which made a caller poll forever. -- Document `coverage.error` (present only when `coverage.stopped == "error"`) - and `coverage.gaps` in the `poll_evaluation` report, plus the 64-handle / - 64 MiB limits and that polling needs the host event loop to turn. -- Add `ToolboxHostView::hasCatalogSnapshotV2()`: true iff the host serves - `acquire_catalog_snapshot_v2`. One `hasX()` per ABI service feature is the - capability rule, stated once in the `plugin_data_api.h` header comment - (ABI feature = tail slot behind a `hasX()`; flag-bit feature = no probe, an - older host rejects the bit; build-dependent behaviour = probe by doing it; - dialog feature = `PJ_dialog_host_info_t::capabilities`; manifest metadata = - no probe). -- Add the dialog host capability `PJ_DIALOG_HOST_EMBEDS_SCENE_VIEWS` - (`1 << 5`, C++ `DialogHostCapability::kEmbedsSceneViews`): the host embeds - `scene_view` / `scene_topics` frames. `DialogPluginBase` (and so - `DialogPluginTyped`) gains `hostHas(capability)` - over the host info already delivered by the existing `set_host_info` slot; - a host that never delivers it reports 0. A dialog gates its scene UI on the - bit instead of probing a different service. No ABI change. + `scene_view`, `scene_topics`) and `WidgetDataView::sceneView` / `sceneTopics`: a + panel QFrame becomes an embedded 3D/2D object view following the playback cursor. + Gate it on the new dialog host capability `PJ_DIALOG_HOST_EMBEDS_SCENE_VIEWS` + (`1 << 5`, `DialogHostCapability::kEmbedsSceneViews`) through the new + `DialogPluginBase::hostHas()`. Dialog protocol only, no ABI change. +- Document `chart_auto_zoom` (`setChartAutoZoom`): omitted fits until the user zooms + or pans, `true` fits now and resumes auto-fit, `false` keeps the user's view. +- Add the optional manifest string `badge` (`PluginDescriptor::badge`): a short label + a host may show next to objects the plugin creates (at most 8 characters advised). - Add the optional manifest flag `custom_topics_editor` - (`PluginDescriptor::custom_topics_editor`, false when absent): the toolbox - that edits the host's user-defined topics. A host keys the "+" button and - the ownership of those rows on it instead of a hard-coded plugin id. -- Add `sdk::kDerivedMetadataKey` (`"pj_derived"`) and `sdk::kDerivedOnDemandValue` - (`"on_demand"`) in `object_topic_metadata.hpp`: the topic metadata key a host - sets on object topics derived by an on_demand data processor. -- Add `sdk::unprojectPixel` (`depth_image_utils.hpp`): unprojects a rectified - pixel with positive metric depth through a pinhole `K`; rejects singular or - non-finite intrinsics and skew/projective terms. Client-side only. -- RENAME (the 0.36 line is unreleased, so nothing shipped breaks): - `PlotTabHostView::createV2` -> `createTabV2` and `focus` -> `focusTab`, so - they no longer read like `DataProcessorsHostView::createV2` or a UI focus. - `feature_floors.json` keys follow. -- Every slot, struct and wrapper added in 0.36 now carries `@since 0.36.0`. -- Docs: the contract text now states, once each, the lifetime of a processor by - flag and kind (persistent / EPHEMERAL / HISTORY_EXEMPT / pinned), what each - kind does with `label`, WINDOW and INSTANT, that Python is an optional - per-host on_demand language probed with `validateScript`, that a plot-tab - re-create keeps the host's layout and clears the curves, that `attach_topic` - is idempotent, that the two halves of catalog snapshot v2 are not atomic, - that a `scene_view` frame exists in panels only and a failed attach is not - retried until the topic set changes, and that the empty-badge fallback is host - policy (at most 8 characters advised). `data_processor_config` reports - `history_exempt` for every kind, not only transforms and markers. -- ImageAnnotations wire carries the top-level timestamp and image_topic; + (`PluginDescriptor::custom_topics_editor`): the toolbox that edits the host's + user-defined topics. + +Builtin objects (client-side, no host-contract change): + +- Add compile-time field tables (`pj_base/builtin/field_table.hpp`, one + `_fields.hpp` per tabled struct, `describe(BuiltinObjectType)` in + `field_table_registry.hpp`) for `FrameTransforms`, `ImageAnnotations`, `PointCloud`, + `SceneEntities`, `Image`, `DepthImage`, `CameraInfo` and `VideoFrame`: a generic + binder reads and writes any described field by name. Packed payloads are `kBuffer` + descriptors; nullable numbers are `kOptionalNumber`; `std::array` members are + fixed-size `kList`s written through `list_replace`. +- Add `sdk::PinholeIntrinsics` and `sdk::unprojectPixel` (`depth_image_utils.hpp`): + unproject a rectified pixel with positive metric depth through a pinhole `K`, + validating `K` once per image. +- Add the object-topic metadata keys `pj_snapshot` + (`ObjectTopicMetadataBuilder::snapshot`: every entry is a complete clear-and-replace + snapshot) and `pj_derived` / `kDerivedOnDemandValue` (set by a host on topics an + on_demand processor derives). +- The ImageAnnotations wire carries the top-level `timestamp` and `image_topic`; additive, old readers skip them. ## [0.35.0] diff --git a/docs/builtin_type.md b/docs/builtin_type.md index 09cf0331..78ea5510 100644 --- a/docs/builtin_type.md +++ b/docs/builtin_type.md @@ -680,19 +680,16 @@ compile time — name, shape (`FieldKind`), and type-erased get/set accessors so a generic consumer (e.g. a script binder) can read or write any described field by name without hand-written per-type glue. A struct opts in with a `FieldTable` specialization built from `field<&T::member>("member")` calls -(`buffer<>()` for a packed record buffer like `PointCloud::data`); nested +(a `kBuffer` factory such as `pointCloudDataField()` for a packed record buffer +like `PointCloud::data`); nested structs and lists of described structs link to their own table through `FieldDescriptor::nested`, so a consumer walks an arbitrarily deep struct tree with one generic recursive routine. `field_table_registry.hpp` exposes `describe(BuiltinObjectType)` to look up a table from the runtime tag a `BuiltinObject` carries. -`FrameTransforms`, `ImageAnnotations`, `PointCloud`, `SceneEntities`, `Image`, -`DepthImage`, `CameraInfo`, and `VideoFrame` are tabled today -(`frame_transforms_fields.hpp`, `image_annotations_fields.hpp`, -`point_cloud_fields.hpp`, `scene_entities_fields.hpp`, `image_fields.hpp`, -`depth_image_fields.hpp`, `camera_info_fields.hpp`, `video_frame_fields.hpp`; -`describe()` in `field_table_registry.hpp` returns a table for exactly these eight). `Image::data`, +Each tabled builtin has a `_fields.hpp` header; `describe()` in +`field_table_registry.hpp` is the authoritative list. `Image::data`, `DepthImage::data`, and `VideoFrame::data` each expose their packed pixel / bitstream bytes through a `kBuffer` descriptor the same way `PointCloud::data` does, with the per-record layout derived from the encoding/format string diff --git a/docs/image_annotations_format.md b/docs/image_annotations_format.md index e402716d..b0c3a52c 100644 --- a/docs/image_annotations_format.md +++ b/docs/image_annotations_format.md @@ -66,7 +66,7 @@ the renderable annotation fields and ignores the rest: | `ImageAnnotations.circles` | Mapped to `ImageAnnotations::circles`. | | `ImageAnnotations.points` | Mapped to `ImageAnnotations::points`. | | `ImageAnnotations.texts` | Mapped to `ImageAnnotations::texts`. | -| Top-level `timestamp` | Serialized when non-zero, decoded into `ImageAnnotations::timestamp`. | +| Top-level `timestamp` | Serialized when non-zero, decoded into `ImageAnnotations::timestamp`; a timestamp of 0 is indistinguishable from an absent one. | | `image_topic` (field 6, string) | Serialized when non-empty, decoded into `ImageAnnotations::image_topic`. | | Top-level `metadata` | Not serialized or decoded today. | | Per-annotation `timestamp` | Not serialized or decoded today. | diff --git a/docs/sdk-utilities.md b/docs/sdk-utilities.md index 16b09913..457220a0 100644 --- a/docs/sdk-utilities.md +++ b/docs/sdk-utilities.md @@ -60,6 +60,10 @@ CMake helpers become available through `find_package(plotjuggler_sdk)`. | `PJ::sdk::MediaMetadataBuilder, ObjectTopicMetadataBuilder` | Build media hints and canonical renderer metadata | `pj_base/sdk/media_metadata.hpp; pj_base/sdk/object_topic_metadata.hpp` | | `PJ::sdk::ObjectBytes` | Own object-read bytes through move-only RAII | `pj_base/sdk/object_bytes.hpp` | | `PJ::sdk::ArrowSchemaHolder, ArrowArrayHolder, ArrowStreamHolder` | Own Arrow out-params; successful writes transfer ownership, failed writes retain it | `pj_base/sdk/arrow.hpp` | +| `PJ::sdk::FieldTable, field<&T::member>, describe` | Read/write builtin struct members by name in a generic binder; lists are element-addressable | `pj_base/builtin/field_table.hpp; pj_base/builtin/field_table_registry.hpp` | +| `PJ::sdk::PinholeIntrinsics, unprojectPixel` | Unproject rectified metric depth pixels; validate K once per image | `pj_base/builtin/depth_image_utils.hpp` | +| `PJ::sdk::fieldCovered, PJ_HAS_TAIL_SLOT` | struct_size checks before reading an appended field or tail slot | `pj_base/sdk/plugin_data_api.hpp; pj_base/plugin_data_api.h` | +| `PJ::sdk::DialogPluginBase::hostHas` | Gate dialog UI on a host dialog-capability bit | `pj_plugins/sdk/dialog_plugin_base.hpp` | | `pj_embed_file, pj_configure_plugin` | Embed UI/manifest assets and configure plugin exports/build rules | `cmake/PjPlugin.cmake` | Outside this SDK, reuse pj-official-plugins common/: diff --git a/pj_base/CLAUDE.md b/pj_base/CLAUDE.md index 7957b0a0..ffe0bdde 100644 --- a/pj_base/CLAUDE.md +++ b/pj_base/CLAUDE.md @@ -18,7 +18,8 @@ build details. It must not depend on Qt, `pj_datastore` or `pj_plugins`. - `include/pj_base/builtin/` — 18 builtin struct headers (`*.hpp`) and all 18 wire codecs (`*_codec.hpp`). Numeric tags are stable; values 2 and 12 are permanently reserved. Also contains the tagged, type-erased `BuiltinObject` - holder and type-erased codec dispatcher. + holder and type-erased codec dispatcher, and the compile-time field tables + (`field_table.hpp`, one `_fields.hpp` per tabled struct). - `include/pj_base/sdk/` — C++ SDK over the ABI: DataSource + Toolbox `*_plugin_base.hpp`, `service_registry.hpp`/`service_traits.hpp`, host views, Arrow RAII holders, `testing/`. - `include/pj_base/*_protocol.h`, `plugin_data_api.h`, `builtin_object_abi.h`, `plugin_abi_export.hpp` — the stable C-ABI surface for DataSource/MessageParser/Toolbox (the Dialog protocol header lives in `pj_plugins/dialog_protocol/`). - `proto/pj/` — canonical `.proto` wire contracts for the builtin types (see its README). diff --git a/pj_base/include/pj_base/plugin_data_api.h b/pj_base/include/pj_base/plugin_data_api.h index aee09f1f..c7156c7a 100644 --- a/pj_base/include/pj_base/plugin_data_api.h +++ b/pj_base/include/pj_base/plugin_data_api.h @@ -1044,7 +1044,8 @@ typedef struct { * recreates or removes it). PJ_DATA_PROCESSOR_FLAG_INFER_OUTPUTS: see * its definition below. Reserved bits must be 0. * - * LIFETIME — a node is exactly one of three things, by flag and kind: + * LIFETIME — by flag and kind. A node is PERSISTENT unless it is EPHEMERAL; + * HISTORY_EXEMPT and PINNED each qualify a persisted node and may be combined: * - PERSISTENT (default): saved in the layout; the host's undo/redo and layout load * may replace, recreate or remove it like any other node. It survives plugin * unload and a session reload. diff --git a/pj_plugins/dialog_protocol/include/pj_plugins/sdk/widget_data.hpp b/pj_plugins/dialog_protocol/include/pj_plugins/sdk/widget_data.hpp index 784a3f2f..095a4355 100644 --- a/pj_plugins/dialog_protocol/include/pj_plugins/sdk/widget_data.hpp +++ b/pj_plugins/dialog_protocol/include/pj_plugins/sdk/widget_data.hpp @@ -649,6 +649,7 @@ class WidgetData { /// views exist in PANELS (non-modal toolbox dialogs) only; a modal dialog never /// embeds one. If the host fails to attach a requested topic it does NOT retry: /// the attach is attempted again only when the requested topic set changes. + /// An empty `kind` clears the view, like clearSceneView(). /// @since 0.36.0 WidgetData& setSceneView(std::string_view name, std::string_view kind) { entry(name)["scene_view"] = std::string(kind); diff --git a/pj_plugins/docs/ARCHITECTURE.md b/pj_plugins/docs/ARCHITECTURE.md index 70ee1911..dbf8edac 100644 --- a/pj_plugins/docs/ARCHITECTURE.md +++ b/pj_plugins/docs/ARCHITECTURE.md @@ -261,9 +261,8 @@ service registry, error out-params, and typed borrowed-dialog patterns): (outputs learned from the script). Python is an optional on_demand language of some hosts, probed with `validateScript`. The request's `struct_size` follows a read-prefix rule: a host accepts a larger size and a later field is - announced by a flag bit. The capability-detection rule (tail slot behind a - `hasX()`; flag bit with no probe; probe by doing; dialog capability bit; - manifest metadata) is stated once in `plugin_data_api.h`. Input names may carry the dataset + announced by a flag bit. The capability-detection rule is stated once in + `plugin_data_api.h`. Input names may carry the dataset qualifier `dataset_source:topic/field`, so a name several loaded datasets share is addressed rather than guessed — rules are normative in `plugin_data_api.h` (DATASET-QUALIFIED NAMES), shared parser/composer in diff --git a/pj_plugins/docs/toolbox-guide.md b/pj_plugins/docs/toolbox-guide.md index 615ad36d..f2199e0d 100644 --- a/pj_plugins/docs/toolbox-guide.md +++ b/pj_plugins/docs/toolbox-guide.md @@ -201,7 +201,7 @@ data store. | `appendArrowStream(topic, stream, ts_col)` | Hand an `ArrowArrayStream*` (Arrow C Data Interface) to the host for bulk ingest. Same ownership rule as the source write path: success transfers, failure retains. | | `catalogSnapshot()` | Acquire a read-only snapshot of all data sources, topics, and fields. | | `hasCatalogSnapshotV2()` (0.36.0) | True iff the host serves snapshot v2. Check it before `catalogSnapshotV2()`; without it object topics are not enumerable. | -| `catalogSnapshotV2()` (0.36.0) | Like `catalogSnapshot()`, plus every object topic (`objectTopics()`) with its dataset, builtin type, entry count and raw time range, in one deep copy. The two halves are not read atomically: a topic created between the reads may appear in one half only. | +| `catalogSnapshotV2()` (0.36.0) | Like `catalogSnapshot()`, plus every object topic (`objectTopics()`) with its dataset, builtin type, entry count and raw time range, in one deep copy. The two halves are not read atomically: a topic created between the reads may appear in one half only. The copy covers every field of every topic, so acquire it when the catalog may have changed, not per tick. | | `readSeriesArrow(field, schema*, array*)` | Read one field's full time series into host-owned `ArrowSchema` + `ArrowArray` out-params (two columns: `timestamp` int64 ns, then the typed field value). | | `registerObjectTopic(source, name, type[, extra_metadata])` | Register a built-in media/object topic under a data source. The typed overload emits the canonical `builtin_object_type` renderer metadata and returns an `ObjectTopicHandle`. | | `registerObjectTopic(source, name, metadata_json)` | Raw-metadata overload for custom or untyped object topics. The store retains the JSON verbatim. | @@ -317,8 +317,9 @@ treat it as unknown and say the host is too old; do not guess a scalar. `createV2` and the `submitEvaluation` / `pollEvaluation` / `releaseEvaluation` trio; gate typed-request UI on it, never on a version. -`DataProcessorsHostView::createV2(request)` upserts a `kind="on_demand"` node like -`createOnDemand`, but the request is typed (`DataProcessorRequest`): outputs carry +`DataProcessorsHostView::createV2(request)` upserts a node of any kind like +`create`, but the request is typed (`DataProcessorRequest`; typed outputs other +than `number` are for `kind="on_demand"`): outputs carry a `DataProcessorOutput{name, type}` pair instead of a `":"` string suffix, plus an optional human-readable `label` and, with `instant_ns` set, a pinned evaluation time. `create_data_processor` (v1) remains valid for the @@ -368,9 +369,9 @@ count as a JSON integer). `coverage.stopped` says why the evaluation ended and `coverage.error` carries the reason only when it is `"error"`; an empty `bundles` list alone does not mean "no sample". `coverage.gaps` lists retention gaps. On `kFailed` it is `{"error":"..."}`; a state the SDK does not know is returned as -an error, never as pending. A host keeps at most 64 live handles and 64 MiB of -reserved report bytes: release finished handles, and poll from a timer so the -host's event loop can finish the work. Handles are per host object, increasing, never reused. `releaseEvaluation(handle)` cancels a +an error, never as pending. Host policy bounds the live handles and the retained +report bytes: release finished handles, and poll from a timer so the host's event +loop can finish the work. Handles are per host object, increasing, never reused. `releaseEvaluation(handle)` cancels a pending evaluation and frees its result; releasing an unknown or already-released handle is an error.