diff --git a/CHANGELOG.md b/CHANGELOG.md index d829277bc..2664c7799 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,12 +6,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +* Support PV image generation along a polyline region ([#1341](https://github.com/CARTAvis/carta-backend/issues/1341)). + ### Fixed * Fixed crash when loading non-image HDU by URL ([#1365](https://github.com/CARTAvis/carta-backend/issues/1365)). * Fix crash when parsing FITS header long value ([#1366](https://github.com/CARTAvis/carta-backend/issues/1366)). * Fix incorrect parsing of SPECSYS value for ATCA FITS header ([#1375](https://github.com/CARTAvis/carta-backend/issues/1375)). * Fix hdf5 image distortion after animation stops ([#1368](https://github.com/CARTAvis/carta-backend/issues/1368)). -* Fix save image bug which could cause directory overwrite or deletion ([#1377](https://github.com/CARTAvis/carta-backend/issues/1377)). +* Fix matched polygon region approximation crash ([#1383](https://github.com/CARTAvis/carta-backend/issues/1383)). +* Fix save image/export regions bug which could cause directory overwrite or deletion ([#1377](https://github.com/CARTAvis/carta-backend/issues/1377)). ### Changed * Move the loader cache to separate files ([#1021](https://github.com/CARTAvis/carta-backend/issues/1021)). diff --git a/src/HttpServer/HttpServer.cc b/src/HttpServer/HttpServer.cc index b02ca62d9..cd5a95c7a 100644 --- a/src/HttpServer/HttpServer.cc +++ b/src/HttpServer/HttpServer.cc @@ -36,7 +36,7 @@ uint32_t HttpServer::_scripting_request_id = 0; HttpServer::HttpServer(std::shared_ptr session_manager, fs::path root_folder, fs::path user_directory, std::string auth_token, bool read_only_mode, bool enable_frontend, bool enable_database, bool enable_scripting, - bool enable_runtime_config) + bool enable_runtime_config, std::string url_prefix) : _session_manager(session_manager), _http_root_folder(root_folder), _auth_token(auth_token), @@ -45,7 +45,8 @@ HttpServer::HttpServer(std::shared_ptr session_manager, fs::path _enable_frontend(enable_frontend), _enable_database(enable_database), _enable_scripting(enable_scripting), - _enable_runtime_config(enable_runtime_config) { + _enable_runtime_config(enable_runtime_config), + _url_prefix(url_prefix) { if (_enable_frontend && !root_folder.empty()) { _frontend_found = IsValidFrontendFolder(root_folder); @@ -61,70 +62,74 @@ void HttpServer::RegisterRoutes() { uWS::App& app = _session_manager->App(); if (_enable_scripting) { - app.post("/api/scripting/action", [&](auto res, auto req) { HandleScriptingAction(res, req); }); + app.post(fmt::format("{}/api/scripting/action", _url_prefix), [&](auto res, auto req) { HandleScriptingAction(res, req); }); } else { - app.post("/api/scripting/action", [&](auto res, auto req) { NotImplemented(res, req); }); + app.post(fmt::format("{}/api/scripting/action", _url_prefix), [&](auto res, auto req) { NotImplemented(res, req); }); } if (_enable_database) { // Dynamic routes for preferences, layouts, snippets and workspaces - app.get("/api/database/preferences", [&](auto res, auto req) { HandleGetPreferences(res, req); }); - app.put("/api/database/preferences", [&](auto res, auto req) { HandleSetPreferences(res, req); }); - app.del("/api/database/preferences", [&](auto res, auto req) { HandleClearPreferences(res, req); }); + app.get(fmt::format("{}/api/database/preferences", _url_prefix), [&](auto res, auto req) { HandleGetPreferences(res, req); }); + app.put(fmt::format("{}/api/database/preferences", _url_prefix), [&](auto res, auto req) { HandleSetPreferences(res, req); }); + app.del(fmt::format("{}/api/database/preferences", _url_prefix), [&](auto res, auto req) { HandleClearPreferences(res, req); }); for (const auto& elem : SCHEMA_URLS) { const auto& object_type = elem.first; - app.get(fmt::format("/api/database/list/{}s", object_type), + app.get(fmt::format("{}/api/database/list/{}s", _url_prefix, object_type), [&](auto res, auto req) { HandleGetObjectList(object_type, res, req); }); - app.get(fmt::format("/api/database/{}s", object_type), [&](auto res, auto req) { HandleGetObjects(object_type, res, req); }); - app.get( - fmt::format("/api/database/{}/:name", object_type), [&](auto res, auto req) { HandleGetObject(object_type, res, req); }); - app.put(fmt::format("/api/database/{}", object_type), [&](auto res, auto req) { HandleSetObject(object_type, res, req); }); - app.del(fmt::format("/api/database/{}", object_type), [&](auto res, auto req) { HandleClearObject(object_type, res, req); }); + app.get(fmt::format("{}/api/database/{}s", _url_prefix, object_type), + [&](auto res, auto req) { HandleGetObjects(object_type, res, req); }); + app.get(fmt::format("{}/api/database/{}/:name", _url_prefix, object_type), + [&](auto res, auto req) { HandleGetObject(object_type, res, req); }); + app.put(fmt::format("{}/api/database/{}", _url_prefix, object_type), + [&](auto res, auto req) { HandleSetObject(object_type, res, req); }); + app.del(fmt::format("{}/api/database/{}", _url_prefix, object_type), + [&](auto res, auto req) { HandleClearObject(object_type, res, req); }); } } else { - app.get("/api/database/*", [&](auto res, auto req) { NotImplemented(res, req); }); - app.put("/api/database/*", [&](auto res, auto req) { NotImplemented(res, req); }); - app.del("/api/database/*", [&](auto res, auto req) { NotImplemented(res, req); }); + app.get(fmt::format("{}/api/database/*", _url_prefix), [&](auto res, auto req) { NotImplemented(res, req); }); + app.put(fmt::format("{}/api/database/*", _url_prefix), [&](auto res, auto req) { NotImplemented(res, req); }); + app.del(fmt::format("{}/api/database/*", _url_prefix), [&](auto res, auto req) { NotImplemented(res, req); }); } if (_enable_frontend) { if (_enable_runtime_config) { - app.get("/config", [&](auto res, auto req) { HandleGetConfig(res, req); }); + app.get(fmt::format("{}/config", _url_prefix), [&](auto res, auto req) { HandleGetConfig(res, req); }); } else { - app.get("/config", [&](auto res, auto req) { DefaultSuccess(res, req); }); + app.get(fmt::format("{}/config", _url_prefix), [&](auto res, auto req) { DefaultSuccess(res, req); }); } // Static routes for all other files - app.get("/*", [&](Res* res, Req* req) { HandleStaticRequest(res, req); }); + app.get(fmt::format("{}/*", _url_prefix), [&](Res* res, Req* req) { HandleStaticRequest(res, req); }); } else { - app.get("/*", [&](auto res, auto req) { NotImplemented(res, req); }); + app.get(fmt::format("{}/*", _url_prefix), [&](auto res, auto req) { NotImplemented(res, req); }); } // CORS support for the API - app.options("/api/*", [&](auto res, auto req) { + app.options(fmt::format("{}/api/*", _url_prefix), [&](auto res, auto req) { AddCorsHeaders(res); res->end(); }); } void HttpServer::HandleGetConfig(Res* res, Req* req) { - json runtime_config = {{"apiAddress", "/api"}}; + json runtime_config = {{"apiAddress", fmt::format("{}/api", _url_prefix)}}; res->writeStatus(HTTP_200); res->writeHeader("Content-Type", "application/json"); res->end(runtime_config.dump()); } void HttpServer::HandleStaticRequest(Res* res, Req* req) { - std::string_view url = req->getUrl(); + std::string url(req->getUrl()); fs::path path = _http_root_folder; - if (url.empty() || url == "/") { + + // Trim prefix and any number of slashes before and after it + std::regex prefix(fmt::format("^/*{}/*", _url_prefix)); + url = std::regex_replace(url, prefix, ""); + + if (url.empty()) { path /= "index.html"; } else { - // Trim all leading '/' - while (url.size() && url[0] == '/') { - url = url.substr(1); - } - path /= std::string(url); + path /= url; } std::error_code error_code; diff --git a/src/HttpServer/HttpServer.h b/src/HttpServer/HttpServer.h index 97acb7654..f40af7d32 100644 --- a/src/HttpServer/HttpServer.h +++ b/src/HttpServer/HttpServer.h @@ -40,7 +40,7 @@ class HttpServer { public: HttpServer(std::shared_ptr session_manager, fs::path root_folder, fs::path user_directory, std::string auth_token, bool read_only_mode = false, bool enable_frontend = true, bool enable_database = true, bool enable_scripting = false, - bool enable_runtime_config = true); + bool enable_runtime_config = true, std::string url_prefix = ""); bool CanServeFrontend() { return _frontend_found; } @@ -96,6 +96,7 @@ class HttpServer { bool _enable_database; bool _enable_scripting; bool _enable_runtime_config; + std::string _url_prefix; std::shared_ptr _session_manager; static uint32_t _scripting_request_id; }; diff --git a/src/ImageGenerators/PvGenerator.cc b/src/ImageGenerators/PvGenerator.cc index c0de88ed9..63310932e 100644 --- a/src/ImageGenerators/PvGenerator.cc +++ b/src/ImageGenerators/PvGenerator.cc @@ -23,7 +23,8 @@ void PvGenerator::SetFileName(int index, const std::string& filename, bool is_pr } bool PvGenerator::GetPvImage(const std::shared_ptr& frame, const casacore::Matrix& pv_data, casacore::IPosition& pv_shape, - const casacore::Quantity& offset_increment, int start_chan, int stokes, bool reverse, GeneratedImage& pv_image, std::string& message) { + PositionAxisType axis_type, const casacore::Quantity& position_increment, int start_chan, int stokes, bool reverse, + GeneratedImage& pv_image, std::string& message) { // Create PV image with input data. auto input_csys = frame->CoordinateSystem(); if (!input_csys->hasSpectralAxis()) { @@ -32,12 +33,26 @@ bool PvGenerator::GetPvImage(const std::shared_ptr& frame, const casacore return false; } - // Convert spectral reference pixel value to world coordinates + // Position axis + int linear_axis = (reverse ? 1 : 0); + std::string position_name; + double position_refval(0.0), position_refpix(0.0); + if (axis_type == OFFSET) { + // refval 0 at center pixel + position_name = "Offset"; + position_refpix = (pv_shape[linear_axis] - 1) / 2; + } else { + // refval 0 at first pixel + position_name = "Distance"; + } + + // Spectral axis: Convert spectral reference pixel value to world coordinates double spectral_refval, spectral_pixval(start_chan); input_csys->spectralCoordinate().toWorld(spectral_refval, spectral_pixval); // Create casacore::TempImage coordinate system and other image info - auto image = SetupPvImage(frame->GetImage(), input_csys, pv_shape, stokes, offset_increment, spectral_refval, reverse, message); + auto image = SetupPvImage(frame->GetImage(), input_csys, pv_shape, position_name, position_increment, position_refval, position_refpix, + spectral_refval, stokes, reverse, message); if (!image) { message = "PV image setup failed."; return false; @@ -73,10 +88,12 @@ void PvGenerator::SetPvImageName(const std::string& filename, int index, bool is std::shared_ptr> PvGenerator::SetupPvImage( const std::shared_ptr>& input_image, const std::shared_ptr& input_csys, - casacore::IPosition& pv_shape, int stokes, const casacore::Quantity& offset_increment, double spectral_refval, bool reverse, - std::string& message) { + casacore::IPosition& pv_shape, const std::string& position_name, const casacore::Quantity& position_increment, double position_refval, + double position_refpix, double spectral_refval, int stokes, bool reverse, std::string& message) { // Create temporary image (no data) using input image. Return casacore::TempImage. - casacore::CoordinateSystem pv_csys = GetPvCoordinateSystem(input_csys, pv_shape, stokes, offset_increment, spectral_refval, reverse); + // Position axis + casacore::CoordinateSystem pv_csys = GetPvCoordinateSystem( + input_csys, pv_shape, position_name, position_increment, position_refval, position_refpix, spectral_refval, stokes, reverse); std::shared_ptr> image( new casacore::TempImage(casacore::TiledShape(pv_shape), pv_csys)); @@ -96,35 +113,37 @@ std::shared_ptr> PvGenerator::SetupPvI } casacore::CoordinateSystem PvGenerator::GetPvCoordinateSystem(const std::shared_ptr& input_csys, - casacore::IPosition& pv_shape, int stokes, const casacore::Quantity& offset_increment, double spectral_refval, bool reverse) { + casacore::IPosition& pv_shape, const std::string& position_name, const casacore::Quantity& position_increment, double position_refval, + double position_refpix, double spectral_refval, int stokes, bool reverse) { // Set PV coordinate system with LinearCoordinate and input coordinates for spectral and stokes casacore::CoordinateSystem pv_csys; - int offset_axis = reverse ? 1 : 0; - // Add linear coordinate (offset); needs to have 2 axes or pc matrix will fail in wcslib. + // Add linear coordinate (offset or distance); needs to have 2 axes or pc matrix will fail in wcslib. // Will remove degenerate linear axis below - casacore::Vector name(2, "Offset"); - casacore::Vector unit(2, offset_increment.getUnit()); - casacore::Vector crval(2, 0.0); // center offset is 0 - casacore::Vector inc(2, offset_increment.getValue()); + casacore::Vector name(2, position_name); + casacore::Vector unit(2, position_increment.getUnit()); + casacore::Vector refval(2, position_refval); + casacore::Vector inc(2, position_increment.getValue()); casacore::Matrix pc(2, 2, 1); pc(0, 1) = 0.0; pc(1, 0) = 0.0; - casacore::Vector crpix(2, (pv_shape[offset_axis] - 1) / 2); - casacore::LinearCoordinate linear_coord(name, unit, crval, inc, pc, crpix); + casacore::Vector refpix(2, position_refpix); + casacore::LinearCoordinate linear_coord(name, unit, refval, inc, pc, refpix); // Set spectral coordinate auto& input_spectral_coord = input_csys->spectralCoordinate(); auto freq_type = input_spectral_coord.frequencySystem(); auto freq_inc = input_spectral_coord.increment()(0); auto rest_freq = input_spectral_coord.restFrequency(); - casacore::Double refpix(0.0); - casacore::SpectralCoordinate spectral_coord(freq_type, spectral_refval, freq_inc, refpix, rest_freq); + casacore::Double spectral_refpix(0.0); + casacore::SpectralCoordinate spectral_coord(freq_type, spectral_refval, freq_inc, spectral_refpix, rest_freq); - // Add offset and spectral coordinates + // Add offset and spectral coordinates in axis order + int linear_axis(0); if (reverse) { pv_csys.addCoordinate(spectral_coord); pv_csys.addCoordinate(linear_coord); + linear_axis = 1; } else { pv_csys.addCoordinate(linear_coord); pv_csys.addCoordinate(spectral_coord); @@ -140,7 +159,7 @@ casacore::CoordinateSystem PvGenerator::GetPvCoordinateSystem(const std::shared_ } // Remove second linear axis - pv_csys.removeWorldAxis(offset_axis + 1, 0.0); + pv_csys.removeWorldAxis(linear_axis + 1, 0.0); pv_csys.setObsInfo(input_csys->obsInfo()); diff --git a/src/ImageGenerators/PvGenerator.h b/src/ImageGenerators/PvGenerator.h index d746cdc93..2755d9b13 100644 --- a/src/ImageGenerators/PvGenerator.h +++ b/src/ImageGenerators/PvGenerator.h @@ -20,27 +20,32 @@ namespace carta { class PvGenerator { public: + enum PositionAxisType { OFFSET, DISTANCE }; + PvGenerator(); ~PvGenerator() = default; // For PV generator (not preview) void SetFileName(int index, const std::string& filename, bool is_preview = false); - // Create generated PV image from input data. If reverse, [spectral, offset] instead of normal [offset, spectral]. + // Create generated PV image from input data. + // cut_region_type determines position axis as offset from center (line) or distance from beginning (polyline). + // reverse determines order of position and spectral axes. // Returns generated image and success, with message if failure. bool GetPvImage(const std::shared_ptr& frame, const casacore::Matrix& pv_data, casacore::IPosition& pv_shape, - const casacore::Quantity& offset_increment, int start_chan, int stokes, bool reverse, GeneratedImage& pv_image, - std::string& message); + PositionAxisType axis_type, const casacore::Quantity& position_increment, int start_chan, int stokes, bool reverse, + GeneratedImage& pv_image, std::string& message); private: void SetPvImageName(const std::string& filename, int index, bool is_preview); std::shared_ptr> SetupPvImage( const std::shared_ptr>& input_image, const std::shared_ptr& input_csys, - casacore::IPosition& pv_shape, int stokes, const casacore::Quantity& offset_increment, double spectral_refval, bool reverse, - std::string& message); + casacore::IPosition& pv_shape, const std::string& position_name, const casacore::Quantity& position_increment, + double position_refval, double position_refpix, double spectral_refval, int stokes, bool reverse, std::string& message); casacore::CoordinateSystem GetPvCoordinateSystem(const std::shared_ptr& input_csys, - casacore::IPosition& pv_shape, int stokes, const casacore::Quantity& offset_increment, double spectral_refval, bool reverse); + casacore::IPosition& pv_shape, const std::string& position_name, const casacore::Quantity& position_increment, + double position_refval, double position_refpix, double spectral_refval, int stokes, bool reverse); // GeneratedImage parameters int _file_id; diff --git a/src/Main/Main.cc b/src/Main/Main.cc index 4cf4e43fb..58e28f26a 100644 --- a/src/Main/Main.cc +++ b/src/Main/Main.cc @@ -127,6 +127,8 @@ int main(int argc, char* argv[]) { carta::OnMessageTask::SetSessionManager(session_manager); // HTTP server + + std::string url_prefix(settings.http_url_prefix); if (!settings.no_frontend || !settings.no_database || settings.enable_scripting) { fs::path frontend_path; @@ -143,9 +145,25 @@ int main(int argc, char* argv[]) { } } + if (!url_prefix.empty()) { + std::regex allowed_prefix("[0-9A-Za-z+/@_-]*"); + if (!std::regex_match(url_prefix, allowed_prefix)) { + spdlog::warn( + "Ignoring invalid custom HTTP URL prefix '{}'. The prefix may only contain the following characters: [0-9], [A-Z], " + "[a-z], +, -, /, @, _.", + url_prefix); + url_prefix = ""; + } else { + // normalize prefix by stripping leading and trailing slashes, and adding a single leading slash + std::regex normalized_prefix("^/*(.*?)/*$"); + url_prefix = std::regex_replace(url_prefix, normalized_prefix, "/$1"); + spdlog::info("Using custom HTTP URL prefix '{}'.", url_prefix); + } + } + http_server = std::make_unique(session_manager, frontend_path, settings.user_directory, auth_token, settings.read_only_mode, - !settings.no_frontend, !settings.no_database, settings.enable_scripting, !settings.no_runtime_config); + !settings.no_frontend, !settings.no_database, settings.enable_scripting, !settings.no_runtime_config, url_prefix); http_server->RegisterRoutes(); if (!settings.no_frontend && !http_server->CanServeFrontend()) { @@ -177,24 +195,24 @@ int main(int argc, char* argv[]) { } } - string base_url = fmt::format("http://{}:{}", default_host_string, port); + string base_url = fmt::format("http://{}:{}{}", default_host_string, port, url_prefix); if (!settings.no_frontend && http_server->CanServeFrontend()) { string frontend_url = base_url; - string query_url; + string query_url("/"); + if (!auth_token.empty()) { - query_url += fmt::format("/?token={}", auth_token); + query_url += fmt::format("?token={}", auth_token); } auto file_query_url = HttpServer::GetFileUrlString(settings.files); + if (!file_query_url.empty()) { - query_url += (query_url.empty() ? "/?" : "&") + file_query_url; + query_url += (auth_token.empty() ? "?" : "&") + file_query_url; } - if (!query_url.empty()) { - frontend_url += query_url; - } + frontend_url += query_url; if (!settings.no_browser) { WebBrowser wb(frontend_url, settings.browser); diff --git a/src/Main/ProgramSettings.cc b/src/Main/ProgramSettings.cc index 71f8a1cf4..9e1009d60 100644 --- a/src/Main/ProgramSettings.cc +++ b/src/Main/ProgramSettings.cc @@ -178,6 +178,7 @@ void ProgramSettings::ApplyCommandLineSettings(int argc, char** argv) { ("log_protocol_messages", "enable protocol message debug logs", cxxopts::value()) ("no_frontend", "disable built-in HTTP frontend interface", cxxopts::value()) ("no_database", "disable built-in HTTP database interface", cxxopts::value()) + ("http_url_prefix", "custom URL prefix for HTTP server", cxxopts::value(), "") ("no_browser", "don't open the frontend URL in a browser on startup", cxxopts::value()) ("browser", "custom browser command", cxxopts::value(), "") ("host", "only listen on the specified interface (IP address or hostname)", cxxopts::value(), "") @@ -228,7 +229,9 @@ provides an interface to the CARTA database. These features can be disabled with 'no_frontend' and 'no_database', for example if the CARTA backend is being invoked by the CARTA controller, which manages access to the frontend and database independently. The HTTP server also provides a scripting interface, but -this must be enabled explicitly with 'enable_scripting'. +this must be enabled explicitly with 'enable_scripting'. A custom prefix for all +HTTP server URLs may be set with 'http_url_prefix' (leading and trailing slashes +will be stripped, except for a single leading slash). Frontend files are served from '{}' (relative to the location of the backend executable). A custom frontend location may be specified with 'frontend_folder'. @@ -314,6 +317,8 @@ global configuration files, respectively. applyOptionalArgument(host, "host", result); applyOptionalArgument(port, "port", result); + applyOptionalArgument(http_url_prefix, "http_url_prefix", result); + applyOptionalArgument(omp_thread_count, "omp_threads", result); applyOptionalArgument(wait_time, "exit_timeout", result); applyOptionalArgument(init_wait_time, "initial_timeout", result); diff --git a/src/Main/ProgramSettings.h b/src/Main/ProgramSettings.h index 313ef1c0d..8147405ff 100644 --- a/src/Main/ProgramSettings.h +++ b/src/Main/ProgramSettings.h @@ -55,6 +55,7 @@ struct ProgramSettings { bool no_frontend = false; bool no_database = false; bool no_runtime_config = false; + std::string http_url_prefix = ""; bool debug_no_auth = false; bool no_browser = false; bool no_log = false; @@ -107,7 +108,8 @@ struct ProgramSettings { {"top_level_folder", &top_level_folder}, {"starting_folder", &starting_folder}, {"frontend_folder", &frontend_folder}, - {"browser", &browser} + {"browser", &browser}, + {"http_url_prefix", &http_url_prefix} }; std::unordered_map*> vector_int_keys_map { diff --git a/src/Region/RegionConverter.cc b/src/Region/RegionConverter.cc index 2e3771851..8a57f1722 100644 --- a/src/Region/RegionConverter.cc +++ b/src/Region/RegionConverter.cc @@ -440,8 +440,9 @@ std::shared_ptr RegionConverter::GetAppliedPolygonRegion( return lc_region; } - if (has_distortion) { - // if ~horizontal then remove intermediate points to fix "kinks" in mask + // If short segment with only starting point, do not fix. + if (has_distortion && segment_x.size() > 1) { + // If ~horizontal segment, remove intermediate points to fix "kinks" in mask. RemoveHorizontalPolygonPoints(segment_x, segment_y); } diff --git a/src/Region/RegionHandler.cc b/src/Region/RegionHandler.cc index df703b28c..6cb9b2d5f 100644 --- a/src/Region/RegionHandler.cc +++ b/src/Region/RegionHandler.cc @@ -986,7 +986,7 @@ bool RegionHandler::CalculatePvImage(const CARTA::PvRequest& pv_request, std::sh } // 2. Region is line - if (GetRegion(region_id)->GetRegionState().type != CARTA::RegionType::LINE) { + if (!IsLineRegion(region_id)) { pv_response.set_message("Region type not supported for PV cut."); return false; } @@ -1294,13 +1294,15 @@ bool RegionHandler::CalculatePvPreviewImage(int frame_id, int preview_id, bool q RemoveRegion(preview_cut_id); // Use PvGenerator to set PV image for headers only + PvGenerator::PositionAxisType pos_axis_type = (preview_cut_state.type == CARTA::LINE ? PvGenerator::OFFSET : PvGenerator::DISTANCE); casacore::Matrix no_preview_data; // do not copy actual preview data into image int start_channel(0); // spectral range applied in preview image int stokes(preview_cube->GetStokes()); PvGenerator pv_generator; pv_generator.SetFileName(preview_id, preview_cube->GetSourceFileName(), true); - if (pv_generator.GetPvImage(preview_frame, no_preview_data, data_shape, increment, start_channel, stokes, reverse, pv_image, error)) { + if (pv_generator.GetPvImage( + preview_frame, no_preview_data, data_shape, pos_axis_type, increment, start_channel, stokes, reverse, pv_image, error)) { int width = data_shape(0); int height = data_shape(1); @@ -1343,13 +1345,20 @@ bool RegionHandler::CalculatePvPreviewImage(int frame_id, int preview_id, bool q bool RegionHandler::CalculatePvImage(int file_id, int region_id, int width, AxisRange& spectral_range, bool reverse, bool keep, std::shared_ptr& frame, GeneratorProgressCallback progress_callback, CARTA::PvResponse& pv_response, GeneratedImage& pv_image) { - // Generate PV image by approximating line as box regions and getting spectral profile for each. + // Generate PV image by approximating line/polyline as box regions and getting spectral profile for each. // Sends updates via progress callback. // Return parameters: PvResponse, GeneratedImage, and preview data if is preview. // Returns whether PV image was generated. pv_response.set_success(false); pv_response.set_cancel(false); + auto region = GetRegion(region_id); + if (!region) { + pv_response.set_message("PV cut region not set"); + return false; + } + auto cut_region_type = region->GetRegionState().type; + // Reset stop flag _stop_pv[file_id] = false; @@ -1366,6 +1375,7 @@ bool RegionHandler::CalculatePvImage(int file_id, int region_id, int width, Axis if (GetLineProfiles(file_id, region_id, width, spectral_range, per_z, stokes_index, "", progress_callback, pv_data, offset_increment, cancelled, message, reverse)) { auto pv_shape = pv_data.shape(); + PvGenerator::PositionAxisType pos_axis_type = (cut_region_type == CARTA::LINE ? PvGenerator::OFFSET : PvGenerator::DISTANCE); int start_chan(spectral_range.from); // Used for reference value in returned PV image // Set PV index suffix (_pv1, _pv2, etc) to keep previously opened PV image @@ -1381,8 +1391,8 @@ bool RegionHandler::CalculatePvImage(int file_id, int region_id, int width, Axis // Create GeneratedImage in PvGenerator PvGenerator pv_generator; pv_generator.SetFileName(name_index, source_filename); - pv_success = - pv_generator.GetPvImage(frame, pv_data, pv_shape, offset_increment, start_chan, stokes_index, reverse, pv_image, message); + pv_success = pv_generator.GetPvImage( + frame, pv_data, pv_shape, pos_axis_type, offset_increment, start_chan, stokes_index, reverse, pv_image, message); cancelled &= _stop_pv[file_id]; // Cleanup @@ -2449,11 +2459,6 @@ bool RegionHandler::GetLineProfiles(int file_id, int region_id, int width, const auto line_coord_sys = line_region->CoordinateSystem(); region_lock.unlock(); - if (per_z && (line_region_state.type == CARTA::RegionType::POLYLINE)) { - message = "Polyline region not supported for PV images."; - return false; - } - if (CancelLineProfiles(region_id, file_id, line_region_state)) { cancelled = true; return false; diff --git a/test/TestPvGenerator.cc b/test/TestPvGenerator.cc index 0e7bcb9fb..aa8f1138b 100644 --- a/test/TestPvGenerator.cc +++ b/test/TestPvGenerator.cc @@ -25,9 +25,17 @@ class PvGeneratorTest : public ::testing::Test, public ImageGenerator { std::shared_ptr csys, bool is_annotation = false) { // Define RegionState for line region std::vector control_points; - control_points.push_back(Message::Point(endpoints[0], endpoints[1])); - control_points.push_back(Message::Point(endpoints[2], endpoints[3])); - CARTA::RegionType type = is_annotation ? CARTA::RegionType::ANNLINE : CARTA::RegionType::LINE; + for (size_t i = 0; i < endpoints.size(); i += 2) { + control_points.push_back(Message::Point(endpoints[i], endpoints[i + 1])); + } + + CARTA::RegionType type(CARTA::RegionType::LINE); + if (endpoints.size() > 4) { + type = is_annotation ? CARTA::RegionType::ANNPOLYLINE : CARTA::RegionType::POLYLINE; + } else if (is_annotation) { + type = CARTA::RegionType::ANNLINE; + } + RegionState region_state(file_id, type, control_points, 0.0); // Set region @@ -96,15 +104,15 @@ TEST_F(PvGeneratorTest, FitsPvImage) { carta::GeneratedImage pv_image; region_handler.CalculatePvImage(pv_request, frame, progress_callback, pv_response, pv_image); - EXPECT_EQ(pv_response.success(), true); - EXPECT_EQ(pv_response.cancel(), false); + EXPECT_TRUE(pv_response.success()); + EXPECT_FALSE(pv_response.cancel()); EXPECT_NE(pv_image.image.get(), nullptr); // Check PV coordinate system auto pv_coord_sys = pv_image.image->coordinates(); EXPECT_EQ(pv_coord_sys.nCoordinates(), 2); - EXPECT_EQ(pv_coord_sys.hasLinearCoordinate(), true); - EXPECT_EQ(pv_coord_sys.hasSpectralAxis(), true); + EXPECT_TRUE(pv_coord_sys.hasLinearCoordinate()); + EXPECT_TRUE(pv_coord_sys.hasSpectralAxis()); auto pv_linear_axes = pv_coord_sys.linearAxesNumbers(); int pv_spectral_axis(pv_coord_sys.spectralAxisNumber()); @@ -166,15 +174,15 @@ TEST_F(PvGeneratorTest, FitsPvImageHorizontalCut) { carta::GeneratedImage pv_image; region_handler.CalculatePvImage(pv_request, frame, progress_callback, pv_response, pv_image); - EXPECT_EQ(pv_response.success(), true); - EXPECT_EQ(pv_response.cancel(), false); + EXPECT_TRUE(pv_response.success()); + EXPECT_FALSE(pv_response.cancel()); EXPECT_NE(pv_image.image.get(), nullptr); // Check PV coordinate system auto pv_coord_sys = pv_image.image->coordinates(); EXPECT_EQ(pv_coord_sys.nCoordinates(), 2); - EXPECT_EQ(pv_coord_sys.hasLinearCoordinate(), true); - EXPECT_EQ(pv_coord_sys.hasSpectralAxis(), true); + EXPECT_TRUE(pv_coord_sys.hasLinearCoordinate()); + EXPECT_TRUE(pv_coord_sys.hasSpectralAxis()); auto pv_linear_axes = pv_coord_sys.linearAxesNumbers(); int pv_spectral_axis(pv_coord_sys.spectralAxisNumber()); @@ -243,15 +251,15 @@ TEST_F(PvGeneratorTest, FitsPvImageVerticalCut) { carta::GeneratedImage pv_image; region_handler.CalculatePvImage(pv_request, frame, progress_callback, pv_response, pv_image); - EXPECT_EQ(pv_response.success(), true); - EXPECT_EQ(pv_response.cancel(), false); + EXPECT_TRUE(pv_response.success()); + EXPECT_FALSE(pv_response.cancel()); EXPECT_NE(pv_image.image.get(), nullptr); // Check PV coordinate system auto pv_coord_sys = pv_image.image->coordinates(); EXPECT_EQ(pv_coord_sys.nCoordinates(), 2); EXPECT_EQ(pv_coord_sys.hasLinearCoordinate(), true); - EXPECT_EQ(pv_coord_sys.hasSpectralAxis(), true); + EXPECT_TRUE(pv_coord_sys.hasSpectralAxis()); auto pv_linear_axes = pv_coord_sys.linearAxesNumbers(); int pv_spectral_axis(pv_coord_sys.spectralAxisNumber()); @@ -312,8 +320,8 @@ TEST_F(PvGeneratorTest, TestNoSpectralAxis) { carta::GeneratedImage pv_image; region_handler.CalculatePvImage(pv_request, frame, progress_callback, pv_response, pv_image); - EXPECT_EQ(pv_response.success(), false); - EXPECT_EQ(pv_response.cancel(), false); + EXPECT_FALSE(pv_response.success()); + EXPECT_FALSE(pv_response.cancel()); EXPECT_EQ(pv_image.image.get(), nullptr); } @@ -325,7 +333,6 @@ TEST_F(PvGeneratorTest, AveragingWidthRange) { } TEST_F(PvGeneratorTest, PvImageSpectralRange) { - // FITS auto image_path = FileFinder::FitsImagePath("noise_3d.fits"); // 10x10x10 image std::shared_ptr loader(carta::FileLoader::GetLoader(image_path)); std::shared_ptr frame(new Frame(0, loader, "0")); @@ -345,8 +352,8 @@ TEST_F(PvGeneratorTest, PvImageSpectralRange) { carta::GeneratedImage pv_image; region_handler.CalculatePvImage(pv_request, frame, progress_callback, pv_response, pv_image); - EXPECT_EQ(pv_response.success(), true); - EXPECT_EQ(pv_response.cancel(), false); + EXPECT_TRUE(pv_response.success()); + EXPECT_FALSE(pv_response.cancel()); EXPECT_NE(pv_image.image.get(), nullptr); // Check shape @@ -375,7 +382,7 @@ TEST_F(PvGeneratorTest, PvImageReversedAxes) { CARTA::PvResponse pv_response; carta::GeneratedImage pv_image; region_handler.CalculatePvImage(pv_request, frame, progress_callback, pv_response, pv_image); - EXPECT_EQ(pv_response.success(), true); + EXPECT_TRUE(pv_response.success()); EXPECT_NE(pv_image.image.get(), nullptr); auto pv_image_shape = pv_image.image->shape(); @@ -385,7 +392,7 @@ TEST_F(PvGeneratorTest, PvImageReversedAxes) { CARTA::PvResponse rev_pv_response; carta::GeneratedImage rev_pv_image; region_handler.CalculatePvImage(pv_request, frame, progress_callback, rev_pv_response, rev_pv_image); - EXPECT_EQ(rev_pv_response.success(), true); + EXPECT_TRUE(rev_pv_response.success()); EXPECT_NE(rev_pv_image.image.get(), nullptr); auto rev_pv_image_shape = rev_pv_image.image->shape(); @@ -417,7 +424,7 @@ TEST_F(PvGeneratorTest, PvImageKeep) { region_handler.CalculatePvImage(pv_request, frame, progress_callback, pv_response, pv_image); // Check PV image file_id and name int index(0); - EXPECT_EQ(pv_response.success(), true); + EXPECT_TRUE(pv_response.success()); EXPECT_TRUE(pv_image.name.find("pv.fits") != std::string::npos); // Request PV image, keeping the first @@ -428,7 +435,7 @@ TEST_F(PvGeneratorTest, PvImageKeep) { region_handler.CalculatePvImage(pv_request, frame, progress_callback, pv_response2, pv_image2); // Check PV image file_id and name index++; - EXPECT_EQ(pv_response2.success(), true); + EXPECT_TRUE(pv_response2.success()); EXPECT_TRUE(pv_image2.name.find("pv1.fits") != std::string::npos); // Request PV image, replace all and reset index @@ -439,7 +446,7 @@ TEST_F(PvGeneratorTest, PvImageKeep) { region_handler.CalculatePvImage(pv_request, frame, progress_callback, pv_response3, pv_image3); // Check PV image file_id and name index = 0; - EXPECT_EQ(pv_response3.success(), true); + EXPECT_TRUE(pv_response3.success()); EXPECT_TRUE(pv_image3.name.find("pv.fits") != std::string::npos); } @@ -465,7 +472,52 @@ TEST_F(PvGeneratorTest, FitsPvAnnotationLine) { carta::GeneratedImage pv_image; region_handler.CalculatePvImage(pv_request, frame, progress_callback, pv_response, pv_image); - EXPECT_EQ(pv_response.success(), false); - EXPECT_EQ(pv_response.cancel(), false); + EXPECT_FALSE(pv_response.success()); + EXPECT_FALSE(pv_response.cancel()); EXPECT_EQ(pv_image.image.get(), nullptr); } + +TEST_F(PvGeneratorTest, FitsPvPolyLine) { + auto image_path = FileFinder::FitsImagePath("noise_3d.fits"); // 10x10x10 image + std::shared_ptr loader(carta::FileLoader::GetLoader(image_path)); + std::shared_ptr frame(new Frame(0, loader, "0")); + carta::RegionHandler region_handler; + + int file_id(0), region_id(-1); + std::vector endpoints = {0.0, 0.0, 3.0, 6.0, 9.0, 9.0}; + auto csys = frame->CoordinateSystem(); + bool is_annotation(false); + SetPvCut(region_handler, file_id, region_id, endpoints, csys, is_annotation); + + // Request PV image + int width(3), z_min(0), z_max(9); // all channels + bool reverse(false), keep(false); + auto pv_request = Message::PvRequest(file_id, region_id, width, z_min, z_max, reverse, keep); + auto progress_callback = [&](float progress) {}; + CARTA::PvResponse pv_response; + carta::GeneratedImage pv_image; + region_handler.CalculatePvImage(pv_request, frame, progress_callback, pv_response, pv_image); + + EXPECT_TRUE(pv_response.success()); + EXPECT_FALSE(pv_response.cancel()); + EXPECT_NE(pv_image.image.get(), nullptr); + // Check data + casacore::Array pv_data; + pv_image.image->get(pv_data); + EXPECT_EQ(pv_data.shape().size(), 2); + EXPECT_EQ(pv_data.shape()(0), 15); + EXPECT_EQ(pv_data.shape()(1), frame->Depth()); + + // AnnPolyline fails + region_id = -1; + is_annotation = true; + SetPvCut(region_handler, file_id, region_id, endpoints, csys, is_annotation); + pv_request = Message::PvRequest(file_id, region_id, width, z_min, z_max, reverse, keep); + CARTA::PvResponse pv_response2; + carta::GeneratedImage pv_image2; + region_handler.CalculatePvImage(pv_request, frame, progress_callback, pv_response2, pv_image2); + + EXPECT_FALSE(pv_response2.success()); + EXPECT_FALSE(pv_response2.cancel()); + EXPECT_EQ(pv_image2.image.get(), nullptr); +} diff --git a/test/TestRegionMatched.cc b/test/TestRegionMatched.cc index 5da734e3e..4d5315fd5 100644 --- a/test/TestRegionMatched.cc +++ b/test/TestRegionMatched.cc @@ -147,27 +147,38 @@ TEST_F(RegionMatchedTest, TestMatchedImagePolygonLCRegion) { // Set region in frame0 carta::RegionHandler region_handler; - int file_id(0), region_id(-1); + int file_id0(0), region_id(-1); CARTA::RegionType region_type = CARTA::RegionType::POLYGON; std::vector points = {5.0, 5.0, 4.0, 3.0, 1.0, 6.0, 3.0, 8.0}; float rotation(0.0); - auto csys = frame0->CoordinateSystem(); - bool ok = SetRegion(region_handler, file_id, region_id, region_type, points, rotation, csys); + auto csys0 = frame0->CoordinateSystem(); + bool ok = SetRegion(region_handler, file_id0, region_id, region_type, points, rotation, csys0); ASSERT_TRUE(ok); auto region = region_handler.GetRegion(region_id); ASSERT_TRUE(region); // shared_ptr // Get Region as 2D LCRegion in frame1 - file_id = 1; - csys = frame1->CoordinateSystem(); + int file_id1(1); + auto csys1 = frame1->CoordinateSystem(); auto image_shape = frame1->ImageShape(); - auto lc_region = region->GetImageRegion(file_id, csys, image_shape); + auto lc_region = region->GetImageRegion(file_id1, csys1, image_shape); // Check LCRegion ASSERT_TRUE(lc_region); // shared_ptr ASSERT_EQ(lc_region->ndim(), image_shape.size()); ASSERT_EQ(lc_region->latticeShape(), image_shape); ASSERT_EQ(lc_region->shape(), casacore::IPosition(2, 5, 6)); + + // Set polygon with very short segment which is does not have points added in matched region approximation. + region_id = -1; + std::vector points2 = {5.0, 5.0, 4.0, 3.0, 4.0, 3.01, 1.0, 6.0, 3.0, 8.0}; + ok = SetRegion(region_handler, file_id0, region_id, region_type, points2, rotation, csys0); + ASSERT_TRUE(ok); + region = region_handler.GetRegion(region_id); + ASSERT_TRUE(region); // shared_ptr + // Get Region as 2D LCRegion in frame1 + lc_region = region->GetImageRegion(file_id1, csys1, image_shape); + ASSERT_TRUE(lc_region); // shared_ptr } TEST_F(RegionMatchedTest, TestMatchedImagePointRecord) {