diff --git a/adapters/imaging/scripts/image.cpp b/adapters/imaging/scripts/image.cpp index e3e8d73..a616b2e 100644 --- a/adapters/imaging/scripts/image.cpp +++ b/adapters/imaging/scripts/image.cpp @@ -184,7 +184,7 @@ auto Adapters::Imaging::Scripts::Image::fulfill( } static const image_convolve_operations table = { - [](const void* source, image_kernel kernel, image_object* output) { + [](const void* source, const image_kernel* kernel, image_object* output) { return reply( static_cast(source)->convolve( kernel, static_cast(source)->convolution), @@ -260,7 +260,8 @@ auto Adapters::Imaging::Scripts::Image::fulfill( } case IMAGE_INPUT_KERNEL: { static const image_convolve_operations operations = { - [](const void* source, image_kernel kernel, image_object* output) { + [](const void* source, const image_kernel* kernel, + image_object* output) { const auto& selected = *static_cast(source); return reply( selected.image->convolve(kernel, selected.callable), output); @@ -321,11 +322,11 @@ auto Adapters::Imaging::Scripts::Image::invert() const } auto Adapters::Imaging::Scripts::Image::convolve( - image_kernel kernel, + const image_kernel* kernel, const godot::Callable& callable) const -> Utility::Result { const auto invalid = Godot::Imaging::Contracts::Kernel::validate( - kernel.width, kernel.height, {kernel.values, kernel.count}); + kernel->width, kernel->height, {kernel->values, kernel->count}); if (!invalid.is_empty()) { return invalid; } @@ -334,11 +335,12 @@ auto Adapters::Imaging::Scripts::Image::convolve( // coefficient array for one call. A Godot owned array closes that lifetime // boundary without exposing a caller's native storage to the script. godot::PackedFloat32Array weights; - weights.resize(kernel.count); + weights.resize(kernel->count); Core::Data::copy( - reinterpret_cast(weights.ptrw()), kernel.values, kernel.count); + reinterpret_cast(weights.ptrw()), kernel->values, kernel->count); return receive( - Invocation::call(callable, errors, weights, kernel.width, kernel.height)); + Invocation::call( + callable, errors, weights, kernel->width, kernel->height)); } auto Adapters::Imaging::Scripts::Image::composite( diff --git a/adapters/imaging/scripts/image.hpp b/adapters/imaging/scripts/image.hpp index 126e443..0ca7978 100644 --- a/adapters/imaging/scripts/image.hpp +++ b/adapters/imaging/scripts/image.hpp @@ -68,7 +68,7 @@ class Image : public Godot::Imaging::Publication::Image { -> Perimortem::Utility::Result; auto invert() const -> Perimortem::Utility::Result; - auto convolve(image_kernel, const godot::Callable&) const + auto convolve(const image_kernel*, const godot::Callable&) const -> Perimortem::Utility::Result; auto composite(image_object, const godot::Callable&) const -> Perimortem::Utility::Result; diff --git a/imaging/contracts/convolve.hpp b/imaging/contracts/convolve.hpp index ca7ebe1..815b168 100644 --- a/imaging/contracts/convolve.hpp +++ b/imaging/contracts/convolve.hpp @@ -41,7 +41,7 @@ class Convolve { // Success transfers one image reference for the enclosing image owner to // adopt. Error bytes remain borrowed until the next provider call. explicit constexpr Convolve(Api api) : api(api) {} - auto apply(image_kernel kernel) const -> Perimortem::Utility:: + auto apply(const image_kernel* kernel) const -> Perimortem::Utility:: Result { image_object output = {}; const auto error = api.operations->apply(api.source, kernel, &output); diff --git a/imaging/contracts/image.h b/imaging/contracts/image.h index f8e67aa..b467aad 100644 --- a/imaging/contracts/image.h +++ b/imaging/contracts/image.h @@ -78,8 +78,10 @@ typedef struct image_invert_operations { image_error (*apply)(const void*, image_object*); } image_invert_operations; +// The caller supplies a nonnull kernel record and keeps it and its coefficient +// storage alive until apply returns. The provider borrows both for this call. typedef struct image_convolve_operations { - image_error (*apply)(const void*, image_kernel, image_object*); + image_error (*apply)(const void*, const image_kernel*, image_object*); } image_convolve_operations; typedef struct image_composite_operations { diff --git a/imaging/cpu/convolve.cpp b/imaging/cpu/convolve.cpp index a9c17b4..4616bef 100644 --- a/imaging/cpu/convolve.cpp +++ b/imaging/cpu/convolve.cpp @@ -181,16 +181,17 @@ static auto transform( return crop(image, kw, kh, pw, ph, planes); } -auto Imaging::Cpu::Convolve::apply(const Image& image, image_kernel kernel) - -> Utility::Result { - const Core::View::Vector weights(kernel.values, kernel.count); +auto Imaging::Cpu::Convolve::apply( + const Image& image, + const image_kernel* kernel) -> Utility::Result { + const Core::View::Vector weights(kernel->values, kernel->count); const auto invalid = Imaging::Contracts::Kernel::validate( - kernel.width, kernel.height, weights); + kernel->width, kernel->height, weights); if (!invalid.is_empty()) { return invalid; } - auto output = transform(image, kernel.width, kernel.height, weights); + auto output = transform(image, kernel->width, kernel->height, weights); return output.visit( [&](Memory::Dynamic::Bytes& pixels) -> Utility::Result { diff --git a/imaging/cpu/convolve.hpp b/imaging/cpu/convolve.hpp index 303a295..3d0b466 100644 --- a/imaging/cpu/convolve.hpp +++ b/imaging/cpu/convolve.hpp @@ -13,7 +13,7 @@ namespace Godot::Imaging::Cpu { // padding. class Convolve { public: - static auto apply(const Image& image, image_kernel kernel) + static auto apply(const Image& image, const image_kernel* kernel) -> Perimortem::Utility::Result; }; diff --git a/imaging/cpu/image.cpp b/imaging/cpu/image.cpp index 9703cc6..67ee8f4 100644 --- a/imaging/cpu/image.cpp +++ b/imaging/cpu/image.cpp @@ -101,7 +101,7 @@ auto Imaging::Cpu::Image::fulfill( if (contract == Imaging::Contracts::Convolve::contract_id) { static const image_convolve_operations table = { - [](const void* source, image_kernel kernel, + [](const void* source, const image_kernel* kernel, image_object* output) -> image_error { return Convolve::apply(*static_cast(source), kernel) .visit( diff --git a/imaging/cuda/convolve.cpp b/imaging/cuda/convolve.cpp index be6b3f7..c8bed43 100644 --- a/imaging/cuda/convolve.cpp +++ b/imaging/cuda/convolve.cpp @@ -98,18 +98,19 @@ static auto transform( }); } -auto Imaging::Cuda::Convolve::apply(const Image& image, image_kernel kernel) - -> Utility::Result { - const Core::View::Vector weights(kernel.values, kernel.count); +auto Imaging::Cuda::Convolve::apply( + const Image& image, + const image_kernel* kernel) -> Utility::Result { + const Core::View::Vector weights(kernel->values, kernel->count); const auto invalid = Imaging::Contracts::Kernel::validate( - kernel.width, kernel.height, weights); + kernel->width, kernel->height, weights); if (!invalid.is_empty()) { return invalid; } auto output = transform( image, Imaging::Contracts::Kernel::Coefficients{ - kernel.width, kernel.height, weights}); + kernel->width, kernel->height, weights}); return output.visit( [&](Allocation& pixels) -> Utility::Result { return image.derive(Core::Data::take(pixels)); diff --git a/imaging/cuda/convolve.hpp b/imaging/cuda/convolve.hpp index 3455744..5119fa2 100644 --- a/imaging/cuda/convolve.hpp +++ b/imaging/cuda/convolve.hpp @@ -12,7 +12,7 @@ namespace Godot::Imaging::Cuda { // The cropped result remains resident until its pixel contract is observed. class Convolve { public: - static auto apply(const Image& image, image_kernel kernel) + static auto apply(const Image& image, const image_kernel* kernel) -> Perimortem::Utility::Result; }; diff --git a/imaging/cuda/image.cpp b/imaging/cuda/image.cpp index 3143bfd..85e6997 100644 --- a/imaging/cuda/image.cpp +++ b/imaging/cuda/image.cpp @@ -107,7 +107,7 @@ auto Imaging::Cuda::Image::fulfill( if (contract == Imaging::Contracts::Convolve::contract_id) { static const image_convolve_operations table = { - [](const void* source, image_kernel kernel, + [](const void* source, const image_kernel* kernel, image_object* output) -> image_error { return Convolve::apply(*static_cast(source), kernel) .visit( diff --git a/imaging/graph/operation.hpp b/imaging/graph/operation.hpp index eef7e4a..6ad9ae8 100644 --- a/imaging/graph/operation.hpp +++ b/imaging/graph/operation.hpp @@ -108,7 +108,7 @@ class Operation { return "Image operation requires a kernel."_view; } - return contract.apply(*kernel); + return contract.apply(kernel); } else if constexpr (input == Input::Image) { if (!image) { return "Image operation requires a second image."_view; diff --git a/tests/image_check.cpp b/tests/image_check.cpp index c7872dc..72a6088 100644 --- a/tests/image_check.cpp +++ b/tests/image_check.cpp @@ -9,6 +9,7 @@ #include "perimortem/core/time.hpp" #include "perimortem/core/writer/textual.hpp" +#include "imaging/contracts/convolve.hpp" #include "imaging/contracts/invert.hpp" #include "imaging/graph/call.hpp" #include "imaging/graph/image.hpp" @@ -24,6 +25,27 @@ using namespace Godot; using namespace Perimortem; +// The retired call boundary has the same outer record size as Convolve. Its +// full callable form must be refused under the unchanged UUID before any +// consumer can treat the supplied table as an invocation agreement. +struct LegacyConvolveOperations { + image_error (*apply)(const void*, image_kernel, image_object*); +}; + +struct LegacyConvolveApi { + const void* source; + const LegacyConvolveOperations* operations; +}; + +TTX_DATA_RECORD( + LegacyConvolveOperations, + TTX_DATA_MEMBER(LegacyConvolveOperations, apply)); + +TTX_DATA_RECORD( + LegacyConvolveApi, + TTX_DATA_MEMBER(LegacyConvolveApi, source), + TTX_DATA_MEMBER(LegacyConvolveApi, operations)); + static void report(Core::View::Bytes text) { Core::Diagnostics::Log::info(text); } @@ -44,6 +66,49 @@ static auto accepted(Utility::Result result) }); } +static void convolution_binding(const Imaging::Graph::Image& image) { + using Ttx::Semantic::Negotiation::Binding::Status; + const auto query = image.get_query(); + const auto& legacy = Ttx::Data::Form::Compiled::reference>::get_representation(); + LegacyConvolveApi rejected{image.get_abi().source, nullptr}; + const auto status = query.bind( + Imaging::Contracts::Convolve::contract_id, + Ttx::Data::Form::Storage( + {&legacy, reinterpret_cast(&rejected), sizeof(rejected)})); + require( + status == Status::Rejected && rejected.source == image.get_abi().source && + rejected.operations == nullptr, + "Convolution supplied a table for the retired value signature"_view); + + // A successful call outlives both pieces of caller owned kernel storage. + // Read the output after ending that scope, so a delayed consumer of the + // borrowed record cannot satisfy the synchronous image result contract. + Core::Option retained; + { + image_convolve api = {}; + const auto& form = Imaging::Contracts::Convolve::get_representation(); + const auto bound = query.bind( + Imaging::Contracts::Convolve::contract_id, + Ttx::Data::Form::Storage( + {&form, reinterpret_cast(&api), sizeof(api)})); + require( + bound == Status::Satisfied, "Convolution pointer binding failed"_view); + const R32 weight = 1; + const image_kernel kernel{1, 1, &weight, 1}; + const auto output = + accepted(Imaging::Contracts::Convolve(api).apply(&kernel)); + retained = Imaging::Graph::Image(output, image.get_provider()); + output.operations->release(output.source); + } + + Memory::Allocator::Arena errors; + require( + accepted(retained->read_pixels(errors)).get_view() == + accepted(image.read_pixels(errors)).get_view(), + "Convolution output retained caller kernel storage"_view); +} + static auto definition( const Imaging::Graph::Image& image, Core::View::Bytes name) -> const Imaging::Graph::Operation& { @@ -145,6 +210,7 @@ static auto spatial( } static void small(Imaging::Graph::Image image, Core::View::Bytes original) { + convolution_binding(image); Memory::Allocator::Arena errors; require( image.get_width() == 19 && image.get_height() == 11,