From 9e802b423031e24a5fb9dcc6f70007252cb5f9b0 Mon Sep 17 00:00:00 2001 From: johndoe6345789 Date: Fri, 9 Jan 2026 20:25:49 +0000 Subject: [PATCH] ROADMAP.md --- CMakeLists.txt | 4 + ROADMAP.md | 51 ++ config/engine_tester_runtime.json | 56 +++ config/schema/runtime_config_v2.schema.json | 38 ++ .../impl/json_config_document_loader.cpp | 76 +++ .../impl/json_config_document_loader.hpp | 29 ++ .../impl/json_config_document_parser.cpp | 31 ++ .../impl/json_config_document_parser.hpp | 16 + .../impl/json_config_extend_resolver.cpp | 43 ++ .../impl/json_config_extend_resolver.hpp | 16 + .../impl/json_config_merge_service.hpp | 31 ++ src/services/impl/json_config_service.cpp | 467 +++++++++--------- .../impl/json_config_writer_service.cpp | 60 ++- src/services/impl/validation_tour_service.cpp | 251 ++++++++-- src/services/impl/validation_tour_service.hpp | 19 +- src/services/interfaces/config_types.hpp | 25 + 16 files changed, 930 insertions(+), 283 deletions(-) create mode 100644 config/engine_tester_runtime.json create mode 100644 src/services/impl/json_config_document_loader.cpp create mode 100644 src/services/impl/json_config_document_loader.hpp create mode 100644 src/services/impl/json_config_document_parser.cpp create mode 100644 src/services/impl/json_config_document_parser.hpp create mode 100644 src/services/impl/json_config_extend_resolver.cpp create mode 100644 src/services/impl/json_config_extend_resolver.hpp create mode 100644 src/services/impl/json_config_merge_service.hpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 2e48bba..1a6c046 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -279,6 +279,7 @@ if(BUILD_SDL3_APP) src/di/service_registry.cpp src/events/event_bus.cpp src/services/impl/json_config_service.cpp + src/services/impl/json_config_document_loader.cpp src/services/impl/config_compiler_service.cpp src/services/impl/command_line_service.cpp src/services/impl/json_config_writer_service.cpp @@ -545,6 +546,7 @@ add_executable(vulkan_shader_linking_test src/services/impl/bgfx_gui_service.cpp src/services/impl/bgfx_shader_compiler.cpp src/services/impl/json_config_service.cpp + src/services/impl/json_config_document_loader.cpp src/services/impl/materialx_shader_generator.cpp src/services/impl/shader_pipeline_validator.cpp src/services/impl/platform_service.cpp @@ -733,6 +735,7 @@ add_test(NAME render_graph_service_test COMMAND render_graph_service_test) add_executable(json_config_merge_test tests/json_config_merge_test.cpp src/services/impl/json_config_service.cpp + src/services/impl/json_config_document_loader.cpp ) target_include_directories(json_config_merge_test PRIVATE "${CMAKE_CURRENT_SOURCE_DIR}/src") target_link_libraries(json_config_merge_test PRIVATE @@ -746,6 +749,7 @@ add_test(NAME json_config_merge_test COMMAND json_config_merge_test) add_executable(json_config_schema_validation_test tests/json_config_schema_validation_test.cpp src/services/impl/json_config_service.cpp + src/services/impl/json_config_document_loader.cpp ) target_include_directories(json_config_schema_validation_test PRIVATE "${CMAKE_CURRENT_SOURCE_DIR}/src") target_link_libraries(json_config_schema_validation_test PRIVATE diff --git a/ROADMAP.md b/ROADMAP.md index 851be80..3982986 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -59,6 +59,7 @@ Treat JSON config as a declarative control plane that compiles into scene, resou - Add runtime probe hooks and map probe severity to crash recovery policies. - Enforce shader uniform compatibility using reflection + material metadata. - Add tests for schema/merge rules, render graph validation, and budget enforcement. +- Start service refactor program for large modules (approaching 2K LOC). ## Config-First Program Plan (Verbose) @@ -143,6 +144,54 @@ Treat JSON config as a declarative control plane that compiles into scene, resou - Deliverable: regression protection for the new pipeline. - Acceptance: new tests pass alongside existing integration tests. +## Service Refactor Program (2K LOC Risk Plan) +### Goals +- Reduce single-file service sizes to improve readability, reviewability, and test coverage. +- Isolate responsibilities: parsing vs validation vs runtime state vs external I/O. +- Make failure modes explicit and easier to diagnose with trace probes. + +### Target Services (Top Of List) +- JsonConfigService (~1800 LOC): split into loader/merger/validator/parser modules. +- ScriptEngineService (~1650 LOC): split Lua binding registry, library setup, and script loading. +- BgfxGraphicsBackend (~1400 LOC): split pipeline/buffer/texture/screenshot/validation submodules. +- BgfxGuiService (~1100 LOC): split font cache, SVG cache, command encoding, and layout. +- MaterialXShaderGenerator (~1100 LOC): split MaterialX graph prep, shader emit, validation. + +### Phase A: Mechanical Extraction (1-3 days) +- [~] JsonConfigService: extracted config document load/merge helpers into `src/services/impl/json_config_document_loader.cpp` to isolate parsing + extends/@delete merging. +- Move self-contained helpers into `*_helpers.cpp` with clear headers. +- Extract pure data transforms into free functions with unit tests. +- Preserve public interfaces; no behavior change in this phase. + +### Phase B: Responsibility Split (2-5 days) +- Create focused classes (e.g., `ConfigSchemaValidator`, `ConfigMergeService`, + `LuaBindingRegistry`, `BgfxPipelineCache`, `TextureLoader`, `GuiFontCache`). +- Reduce cross-module knowledge by passing simple data structs. +- Add trace logging at handoff boundaries to retain diagnostics. + +### Phase C: API Stabilization (2-4 days) +- Tighten constructor injection to only needed dependencies. +- Remove circular dependencies; make order-of-operations explicit. +- Add targeted unit tests for each new helper/service. + +### Acceptance Criteria +- Each refactored service has < 800 LOC in its primary implementation file. +- 1–3 unit tests per extracted module (minimum happy + failure path). +- No regression in existing integration tests or runtime logs. + +## Validation Tour (Production Self-Test) +### Multi-Method Screen Validation +- Image compare (baseline diff with tolerance + max diff pixels). +- Non-black ratio checks (detect all-black or missing render output). +- Luma range checks (detect over/under-exposed frames). +- Mean color checks (verify dominant color scenes without exact baselines). +- Sample point checks (pinpoint color at specific normalized coordinates). + +### Engine Tester Config +- `config/engine_tester_runtime.json` provides a default self-test config. +- Designed for production binaries; no golden image required by default. +- Produces capture artifacts in `artifacts/validation/`. + ### Default Config Behavior (Config-First) - Default config resolution remains `--json-file-in` → `--set-default-json` path → stored default config → seed config. - Config-first is the default runtime path after the config is loaded. @@ -215,6 +264,7 @@ Option B: per-shader only - [x] Pipeline compatibility tests (shader inputs vs mesh layouts) - [x] Crash recovery timeout tests (`tests/crash_recovery_timeout_test.cpp`) - [~] Budget enforcement tests (GUI cache pruning + texture tracker covered; transient pool pending) +- [~] Config-driven validation tour (checkpoint captures + image/ratio/luma/sample-point checks) - [ ] Smoke test: cube demo boots with config-first scene definition ## Test Strategy (Solid Coverage Plan) @@ -228,6 +278,7 @@ Option B: per-shader only - Service: render graph validation (cycles, unknown refs, duplicates), shader pipeline validation, budget enforcement. - Integration: shader compilation, MaterialX generation + validation, crash recovery timeouts. - Smoke: config-first boot of the cube demo with no Lua scene execution. +- Runtime: validation tour checkpoints for production self-test. ### Coverage Matrix (What We Must Prove) - Config parsing + schema errors produce JSON Pointer diagnostics. diff --git a/config/engine_tester_runtime.json b/config/engine_tester_runtime.json new file mode 100644 index 0000000..d889026 --- /dev/null +++ b/config/engine_tester_runtime.json @@ -0,0 +1,56 @@ +{ + "schema_version": 2, + "configVersion": 2, + "scripts": { + "entry": "scripts/cube_logic.lua", + "lua_debug": false + }, + "runtime": { + "scene_source": "lua" + }, + "window": { + "title": "SDL3 Engine Tester", + "size": { + "width": 1280, + "height": 720 + } + }, + "rendering": { + "bgfx": { + "renderer": "auto" + } + }, + "validation_tour": { + "enabled": true, + "fail_on_mismatch": true, + "warmup_frames": 3, + "capture_frames": 1, + "output_dir": "artifacts/validation", + "checkpoints": [ + { + "id": "startup_spawn", + "camera": { + "position": [0.0, 2.0, 5.0], + "look_at": [0.0, 1.0, 0.0], + "fov_degrees": 60.0, + "near": 0.1, + "far": 100.0 + }, + "checks": [ + { + "type": "non_black_ratio", + "threshold": 0.02, + "min_ratio": 0.01, + "max_ratio": 1.0 + }, + { + "type": "luma_range", + "min_luma": 0.01, + "max_luma": 0.95 + } + ] + } + ] + }, + "config_file": "config/engine_tester_runtime.json" +} diff --git a/config/schema/runtime_config_v2.schema.json b/config/schema/runtime_config_v2.schema.json index c18e9b1..84cb977 100644 --- a/config/schema/runtime_config_v2.schema.json +++ b/config/schema/runtime_config_v2.schema.json @@ -289,8 +289,46 @@ "max_diff_pixels": {"type": "number", "minimum": 0} }, "additionalProperties": true + }, + "checks": { + "type": "array", + "items": { + "type": "object", + "properties": { + "type": { + "type": "string", + "enum": ["non_black_ratio", "luma_range", "mean_color", "sample_points"] + }, + "threshold": {"type": "number", "minimum": 0, "maximum": 1}, + "min_ratio": {"type": "number", "minimum": 0, "maximum": 1}, + "max_ratio": {"type": "number", "minimum": 0, "maximum": 1}, + "min_luma": {"type": "number", "minimum": 0, "maximum": 1}, + "max_luma": {"type": "number", "minimum": 0, "maximum": 1}, + "color": {"$ref": "#/definitions/float3"}, + "tolerance": {"type": "number", "minimum": 0, "maximum": 1}, + "points": { + "type": "array", + "items": { + "type": "object", + "properties": { + "x": {"type": "number", "minimum": 0, "maximum": 1}, + "y": {"type": "number", "minimum": 0, "maximum": 1}, + "color": {"$ref": "#/definitions/float3"}, + "tolerance": {"type": "number", "minimum": 0, "maximum": 1} + }, + "additionalProperties": true + } + } + }, + "required": ["type"], + "additionalProperties": true + } } }, + "anyOf": [ + { "required": ["expected"] }, + { "required": ["checks"] } + ], "additionalProperties": true } } diff --git a/src/services/impl/json_config_document_loader.cpp b/src/services/impl/json_config_document_loader.cpp new file mode 100644 index 0000000..137e3d3 --- /dev/null +++ b/src/services/impl/json_config_document_loader.cpp @@ -0,0 +1,76 @@ +#include "json_config_document_loader.hpp" +#include "json_config_document_parser.hpp" +#include "json_config_extend_resolver.hpp" +#include "json_config_merge_service.hpp" +#include "../interfaces/i_logger.hpp" + +#include +#include + +namespace sdl3cpp::services::impl::json_config { + +JsonConfigDocumentLoader::JsonConfigDocumentLoader(std::shared_ptr logger) + : logger_(std::move(logger)) {} + +rapidjson::Document JsonConfigDocumentLoader::Load(const std::filesystem::path& configPath) const { + std::unordered_set visited; + return LoadRecursive(configPath, visited); +} + +std::filesystem::path JsonConfigDocumentLoader::NormalizeConfigPath(const std::filesystem::path& path) const { + std::error_code ec; + auto canonicalPath = std::filesystem::weakly_canonical(path, ec); + if (ec) { + return std::filesystem::absolute(path); + } + return canonicalPath; +} + +rapidjson::Document JsonConfigDocumentLoader::LoadRecursive(const std::filesystem::path& configPath, + std::unordered_set& visited) const { + const auto normalizedPath = NormalizeConfigPath(configPath); + const std::string pathKey = normalizedPath.string(); + if (!visited.insert(pathKey).second) { + throw std::runtime_error("Config extends cycle detected at " + pathKey); + } + + if (logger_) { + logger_->Trace("JsonConfigService", "LoadConfigDocumentRecursive", + "configPath=" + pathKey, "Loading config document"); + } + + JsonConfigDocumentParser parser; + rapidjson::Document document = parser.Parse(normalizedPath, "config file"); + + JsonConfigExtendResolver extendResolver; + auto extendPaths = extendResolver.Extract(document, normalizedPath); + if (document.HasMember("extends")) { + document.RemoveMember("extends"); + } + + if (extendPaths.empty()) { + visited.erase(pathKey); + return document; + } + + if (logger_) { + logger_->Trace("JsonConfigService", "LoadConfigDocumentRecursive", + "configPath=" + pathKey + ", extendsCount=" + std::to_string(extendPaths.size()), + "Merging extended configs"); + } + + JsonConfigMergeService mergeService(logger_); + rapidjson::Document merged; + merged.SetObject(); + auto& allocator = merged.GetAllocator(); + for (const auto& extendPath : extendPaths) { + auto baseDoc = LoadRecursive(extendPath, visited); + mergeService.Merge(merged, baseDoc, allocator, extendPath.string()); + } + mergeService.Merge(merged, document, allocator, normalizedPath.string()); + + visited.erase(pathKey); + return merged; +} + +} // namespace sdl3cpp::services::impl::json_config diff --git a/src/services/impl/json_config_document_loader.hpp b/src/services/impl/json_config_document_loader.hpp new file mode 100644 index 0000000..13b1670 --- /dev/null +++ b/src/services/impl/json_config_document_loader.hpp @@ -0,0 +1,29 @@ +#pragma once + +#include +#include +#include + +#include + +namespace sdl3cpp::services { +class ILogger; +} + +namespace sdl3cpp::services::impl::json_config { + +class JsonConfigDocumentLoader { +public: + explicit JsonConfigDocumentLoader(std::shared_ptr logger); + + rapidjson::Document Load(const std::filesystem::path& configPath) const; + +private: + rapidjson::Document LoadRecursive(const std::filesystem::path& configPath, + std::unordered_set& visited) const; + std::filesystem::path NormalizeConfigPath(const std::filesystem::path& path) const; + + std::shared_ptr logger_; +}; + +} // namespace sdl3cpp::services::impl::json_config diff --git a/src/services/impl/json_config_document_parser.cpp b/src/services/impl/json_config_document_parser.cpp new file mode 100644 index 0000000..b793f65 --- /dev/null +++ b/src/services/impl/json_config_document_parser.cpp @@ -0,0 +1,31 @@ +#include "json_config_document_parser.hpp" + +#include +#include + +#include +#include + +namespace sdl3cpp::services::impl::json_config { + +rapidjson::Document JsonConfigDocumentParser::Parse(const std::filesystem::path& jsonPath, + const std::string& description) const { + std::ifstream configStream(jsonPath); + if (!configStream) { + throw std::runtime_error("Failed to open " + description + ": " + jsonPath.string()); + } + + rapidjson::IStreamWrapper inputWrapper(configStream); + rapidjson::Document document; + document.ParseStream(inputWrapper); + if (document.HasParseError()) { + throw std::runtime_error("Failed to parse " + description + " at " + jsonPath.string() + + ": " + rapidjson::GetParseError_En(document.GetParseError())); + } + if (!document.IsObject()) { + throw std::runtime_error("JSON " + description + " must contain an object at the root"); + } + return document; +} + +} // namespace sdl3cpp::services::impl::json_config diff --git a/src/services/impl/json_config_document_parser.hpp b/src/services/impl/json_config_document_parser.hpp new file mode 100644 index 0000000..812cc9d --- /dev/null +++ b/src/services/impl/json_config_document_parser.hpp @@ -0,0 +1,16 @@ +#pragma once + +#include +#include + +#include + +namespace sdl3cpp::services::impl::json_config { + +class JsonConfigDocumentParser { +public: + rapidjson::Document Parse(const std::filesystem::path& jsonPath, + const std::string& description) const; +}; + +} // namespace sdl3cpp::services::impl::json_config diff --git a/src/services/impl/json_config_extend_resolver.cpp b/src/services/impl/json_config_extend_resolver.cpp new file mode 100644 index 0000000..9fa4ca9 --- /dev/null +++ b/src/services/impl/json_config_extend_resolver.cpp @@ -0,0 +1,43 @@ +#include "json_config_extend_resolver.hpp" + +#include + +namespace sdl3cpp::services::impl::json_config { + +namespace { +constexpr const char* kExtendsKey = "extends"; +} + +std::vector JsonConfigExtendResolver::Extract( + const rapidjson::Value& document, + const std::filesystem::path& configPath) const { + std::vector paths; + if (!document.HasMember(kExtendsKey)) { + return paths; + } + + const auto& extendsValue = document[kExtendsKey]; + auto resolvePath = [&](const std::filesystem::path& candidate) { + if (candidate.is_absolute()) { + return candidate; + } + return configPath.parent_path() / candidate; + }; + + if (extendsValue.IsString()) { + paths.push_back(resolvePath(extendsValue.GetString())); + } else if (extendsValue.IsArray()) { + for (const auto& entry : extendsValue.GetArray()) { + if (!entry.IsString()) { + throw std::runtime_error("JSON member 'extends' must be a string or array of strings"); + } + paths.push_back(resolvePath(entry.GetString())); + } + } else { + throw std::runtime_error("JSON member 'extends' must be a string or array of strings"); + } + + return paths; +} + +} // namespace sdl3cpp::services::impl::json_config diff --git a/src/services/impl/json_config_extend_resolver.hpp b/src/services/impl/json_config_extend_resolver.hpp new file mode 100644 index 0000000..e6003ef --- /dev/null +++ b/src/services/impl/json_config_extend_resolver.hpp @@ -0,0 +1,16 @@ +#pragma once + +#include +#include + +#include + +namespace sdl3cpp::services::impl::json_config { + +class JsonConfigExtendResolver { +public: + std::vector Extract(const rapidjson::Value& document, + const std::filesystem::path& configPath) const; +}; + +} // namespace sdl3cpp::services::impl::json_config diff --git a/src/services/impl/json_config_merge_service.hpp b/src/services/impl/json_config_merge_service.hpp new file mode 100644 index 0000000..9bfcc2a --- /dev/null +++ b/src/services/impl/json_config_merge_service.hpp @@ -0,0 +1,31 @@ +#pragma once + +#include +#include + +#include + +namespace sdl3cpp::services { +class ILogger; +} + +namespace sdl3cpp::services::impl::json_config { + +class JsonConfigMergeService { +public: + explicit JsonConfigMergeService(std::shared_ptr logger); + + void Merge(rapidjson::Value& target, + const rapidjson::Value& overlay, + rapidjson::Document::AllocatorType& allocator, + const std::string& jsonPath) const; + +private: + void ApplyDeleteDirectives(rapidjson::Value& target, + const rapidjson::Value& overlay, + const std::string& jsonPath) const; + + std::shared_ptr logger_; +}; + +} // namespace sdl3cpp::services::impl::json_config diff --git a/src/services/impl/json_config_service.cpp b/src/services/impl/json_config_service.cpp index 6169724..3f00744 100644 --- a/src/services/impl/json_config_service.cpp +++ b/src/services/impl/json_config_service.cpp @@ -1,20 +1,15 @@ #include "json_config_service.hpp" +#include "json_config_document_loader.hpp" #include "../interfaces/i_logger.hpp" #include -#include -#include #include #include #include #include #include -#include -#include -#include #include #include #include -#include #include namespace sdl3cpp::services::impl { @@ -23,8 +18,6 @@ namespace { constexpr int kExpectedSchemaVersion = 2; constexpr const char* kSchemaVersionKey = "schema_version"; constexpr const char* kConfigVersionKey = "configVersion"; -constexpr const char* kExtendsKey = "extends"; -constexpr const char* kDeleteKey = "@delete"; const char* SceneSourceName(SceneSource source) { switch (source) { @@ -53,193 +46,6 @@ std::string PointerToString(const rapidjson::Pointer& pointer) { return buffer.GetString(); } -std::filesystem::path NormalizeConfigPath(const std::filesystem::path& path) { - std::error_code ec; - auto canonicalPath = std::filesystem::weakly_canonical(path, ec); - if (ec) { - return std::filesystem::absolute(path); - } - return canonicalPath; -} - -rapidjson::Document ParseJsonDocument(const std::filesystem::path& jsonPath, - const std::string& description) { - std::ifstream configStream(jsonPath); - if (!configStream) { - throw std::runtime_error("Failed to open " + description + ": " + jsonPath.string()); - } - - rapidjson::IStreamWrapper inputWrapper(configStream); - rapidjson::Document document; - document.ParseStream(inputWrapper); - if (document.HasParseError()) { - throw std::runtime_error("Failed to parse " + description + " at " + jsonPath.string() + - ": " + rapidjson::GetParseError_En(document.GetParseError())); - } - if (!document.IsObject()) { - throw std::runtime_error("JSON " + description + " must contain an object at the root"); - } - return document; -} - -std::vector ExtractExtendPaths(const rapidjson::Value& document, - const std::filesystem::path& configPath) { - std::vector paths; - if (!document.HasMember(kExtendsKey)) { - return paths; - } - - const auto& extendsValue = document[kExtendsKey]; - auto resolvePath = [&](const std::filesystem::path& candidate) { - if (candidate.is_absolute()) { - return candidate; - } - return configPath.parent_path() / candidate; - }; - - if (extendsValue.IsString()) { - paths.push_back(resolvePath(extendsValue.GetString())); - } else if (extendsValue.IsArray()) { - for (const auto& entry : extendsValue.GetArray()) { - if (!entry.IsString()) { - throw std::runtime_error("JSON member 'extends' must be a string or array of strings"); - } - paths.push_back(resolvePath(entry.GetString())); - } - } else { - throw std::runtime_error("JSON member 'extends' must be a string or array of strings"); - } - - return paths; -} - -std::filesystem::path ResolveSchemaPath(const std::filesystem::path& configPath) { - const std::filesystem::path schemaFile = "runtime_config_v2.schema.json"; - std::vector candidates; - if (!configPath.empty()) { - candidates.push_back(configPath.parent_path() / "schema" / schemaFile); - } - candidates.push_back(std::filesystem::current_path() / "config" / "schema" / schemaFile); - - std::error_code ec; - for (const auto& candidate : candidates) { - if (!candidate.empty() && std::filesystem::exists(candidate, ec)) { - return candidate; - } - } - return {}; -} - -void ApplyDeleteDirectives(rapidjson::Value& target, - const rapidjson::Value& overlay, - const std::shared_ptr& logger, - const std::string& jsonPath) { - if (!overlay.HasMember(kDeleteKey)) { - return; - } - const auto& deletes = overlay[kDeleteKey]; - if (!deletes.IsArray()) { - throw std::runtime_error("JSON member '" + std::string(kDeleteKey) + "' must be an array of strings"); - } - for (const auto& entry : deletes.GetArray()) { - if (!entry.IsString()) { - throw std::runtime_error("JSON member '" + std::string(kDeleteKey) + "' must contain only strings"); - } - const char* key = entry.GetString(); - if (target.HasMember(key)) { - target.RemoveMember(key); - if (logger) { - logger->Trace("JsonConfigService", "ApplyDeleteDirectives", - "jsonPath=" + jsonPath + ", key=" + std::string(key), - "Removed key from merged config"); - } - } - } -} - -void MergeJsonValues(rapidjson::Value& target, - const rapidjson::Value& overlay, - rapidjson::Document::AllocatorType& allocator, - const std::shared_ptr& logger, - const std::string& jsonPath) { - if (!overlay.IsObject()) { - target.CopyFrom(overlay, allocator); - return; - } - - if (!target.IsObject()) { - target.CopyFrom(overlay, allocator); - return; - } - - ApplyDeleteDirectives(target, overlay, logger, jsonPath); - - for (auto it = overlay.MemberBegin(); it != overlay.MemberEnd(); ++it) { - const std::string memberName = it->name.GetString(); - if (memberName == kDeleteKey) { - continue; - } - if (target.HasMember(memberName.c_str())) { - auto& targetValue = target[memberName.c_str()]; - const auto& overlayValue = it->value; - if (targetValue.IsObject() && overlayValue.IsObject()) { - MergeJsonValues(targetValue, overlayValue, allocator, logger, jsonPath + "/" + memberName); - } else { - targetValue.CopyFrom(overlayValue, allocator); - } - } else { - rapidjson::Value nameValue(memberName.c_str(), allocator); - rapidjson::Value valueCopy; - valueCopy.CopyFrom(it->value, allocator); - target.AddMember(nameValue, valueCopy, allocator); - } - } -} - -rapidjson::Document LoadConfigDocumentRecursive(const std::filesystem::path& configPath, - const std::shared_ptr& logger, - std::unordered_set& visited) { - const auto normalizedPath = NormalizeConfigPath(configPath); - const std::string pathKey = normalizedPath.string(); - if (!visited.insert(pathKey).second) { - throw std::runtime_error("Config extends cycle detected at " + pathKey); - } - - if (logger) { - logger->Trace("JsonConfigService", "LoadConfigDocumentRecursive", - "configPath=" + pathKey, "Loading config document"); - } - - rapidjson::Document document = ParseJsonDocument(normalizedPath, "config file"); - auto extendPaths = ExtractExtendPaths(document, normalizedPath); - if (document.HasMember(kExtendsKey)) { - document.RemoveMember(kExtendsKey); - } - - if (extendPaths.empty()) { - visited.erase(pathKey); - return document; - } - - if (logger) { - logger->Trace("JsonConfigService", "LoadConfigDocumentRecursive", - "configPath=" + pathKey + ", extendsCount=" + std::to_string(extendPaths.size()), - "Merging extended configs"); - } - - rapidjson::Document merged; - merged.SetObject(); - auto& allocator = merged.GetAllocator(); - for (const auto& extendPath : extendPaths) { - auto baseDoc = LoadConfigDocumentRecursive(extendPath, logger, visited); - MergeJsonValues(merged, baseDoc, allocator, logger, extendPath.string()); - } - MergeJsonValues(merged, document, allocator, logger, normalizedPath.string()); - - visited.erase(pathKey); - return merged; -} - std::optional ReadVersionField(const rapidjson::Value& document, const char* fieldName, const std::filesystem::path& configPath) { @@ -314,7 +120,7 @@ void ValidateSchemaDocument(const rapidjson::Document& document, const std::filesystem::path& configPath, const std::shared_ptr& logger, const std::shared_ptr& probeService) { - const auto schemaPath = ResolveSchemaPath(configPath); + const auto schemaPath = json_config::ResolveSchemaPath(configPath); if (schemaPath.empty()) { if (logger) { logger->Warn("JsonConfigService::ValidateSchemaDocument: Schema file not found for " + @@ -323,7 +129,7 @@ void ValidateSchemaDocument(const rapidjson::Document& document, return; } - rapidjson::Document schemaDocument = ParseJsonDocument(schemaPath, "schema file"); + rapidjson::Document schemaDocument = json_config::ParseJsonDocument(schemaPath, "schema file"); rapidjson::SchemaDocument schema(schemaDocument); rapidjson::SchemaValidator validator(schema); if (!document.Accept(validator)) { @@ -440,7 +246,7 @@ RuntimeConfig JsonConfigService::LoadFromJson(std::shared_ptr logger, logger->Trace("JsonConfigService", "LoadFromJson", args); std::unordered_set visitedPaths; - rapidjson::Document document = LoadConfigDocumentRecursive(configPath, logger, visitedPaths); + rapidjson::Document document = json_config::LoadConfigDocumentRecursive(configPath, logger, visitedPaths); const auto activeVersion = ValidateSchemaVersion(document, configPath, logger); if (activeVersion && *activeVersion != kExpectedSchemaVersion) { const bool migrated = ApplyMigrations(document, @@ -1294,39 +1100,192 @@ RuntimeConfig JsonConfigService::LoadFromJson(std::shared_ptr logger, checkpoint.camera.farPlane = static_cast(value.GetDouble()); } - if (!entry.HasMember("expected") || !entry["expected"].IsObject()) { - throw std::runtime_error("JSON member '" + basePath + ".expected' must be an object"); - } - const auto& expectedValue = entry["expected"]; - if (!expectedValue.HasMember("image") || !expectedValue["image"].IsString()) { - throw std::runtime_error("JSON member '" + basePath + ".expected.image' must be a string"); - } - checkpoint.expected.imagePath = expectedValue["image"].GetString(); - if (expectedValue.HasMember("tolerance")) { - const auto& value = expectedValue["tolerance"]; + auto readFloatInRange = [&](const rapidjson::Value& value, + const std::string& path, + float minValue, + float maxValue) -> float { if (!value.IsNumber()) { - throw std::runtime_error("JSON member '" + basePath + - ".expected.tolerance' must be a number"); + throw std::runtime_error("JSON member '" + path + "' must be a number"); } const double rawValue = value.GetDouble(); - if (rawValue < 0.0 || rawValue > 1.0) { - throw std::runtime_error("JSON member '" + basePath + - ".expected.tolerance' must be between 0 and 1"); + const double minAllowed = static_cast(minValue); + const double maxAllowed = static_cast(maxValue); + if (rawValue < minAllowed || rawValue > maxAllowed) { + throw std::runtime_error("JSON member '" + path + "' must be between " + + std::to_string(minValue) + " and " + + std::to_string(maxValue)); + } + return static_cast(rawValue); + }; + + if (entry.HasMember("expected")) { + if (!entry["expected"].IsObject()) { + throw std::runtime_error("JSON member '" + basePath + ".expected' must be an object"); + } + const auto& expectedValue = entry["expected"]; + if (!expectedValue.HasMember("image") || !expectedValue["image"].IsString()) { + throw std::runtime_error("JSON member '" + basePath + ".expected.image' must be a string"); + } + checkpoint.expected.enabled = true; + checkpoint.expected.imagePath = expectedValue["image"].GetString(); + if (expectedValue.HasMember("tolerance")) { + checkpoint.expected.tolerance = readFloatInRange( + expectedValue["tolerance"], + basePath + ".expected.tolerance", + 0.0f, + 1.0f); + } + if (expectedValue.HasMember("max_diff_pixels")) { + const auto& value = expectedValue["max_diff_pixels"]; + if (!value.IsNumber()) { + throw std::runtime_error("JSON member '" + basePath + + ".expected.max_diff_pixels' must be a number"); + } + const double rawValue = value.GetDouble(); + if (rawValue < 0.0) { + throw std::runtime_error("JSON member '" + basePath + + ".expected.max_diff_pixels' must be non-negative"); + } + checkpoint.expected.maxDiffPixels = static_cast(rawValue); } - checkpoint.expected.tolerance = static_cast(rawValue); } - if (expectedValue.HasMember("max_diff_pixels")) { - const auto& value = expectedValue["max_diff_pixels"]; - if (!value.IsNumber()) { - throw std::runtime_error("JSON member '" + basePath + - ".expected.max_diff_pixels' must be a number"); + + if (entry.HasMember("checks")) { + const auto& checksValue = entry["checks"]; + if (!checksValue.IsArray()) { + throw std::runtime_error("JSON member '" + basePath + ".checks' must be an array"); } - const double rawValue = value.GetDouble(); - if (rawValue < 0.0) { - throw std::runtime_error("JSON member '" + basePath + - ".expected.max_diff_pixels' must be non-negative"); + checkpoint.checks.clear(); + checkpoint.checks.reserve(checksValue.Size()); + + auto readFloat3Field = [&](const rapidjson::Value& value, + const std::string& path, + std::array& target) { + if (!value.IsArray() || value.Size() != 3) { + throw std::runtime_error("JSON member '" + path + "' must be an array of 3 numbers"); + } + for (rapidjson::SizeType index = 0; index < 3; ++index) { + if (!value[index].IsNumber()) { + throw std::runtime_error("JSON member '" + path + "[" + std::to_string(index) + + "]' must be a number"); + } + target[index] = static_cast(value[index].GetDouble()); + } + }; + + for (rapidjson::SizeType checkIndex = 0; checkIndex < checksValue.Size(); ++checkIndex) { + const auto& checkValue = checksValue[checkIndex]; + const std::string checkPath = basePath + ".checks[" + std::to_string(checkIndex) + "]"; + if (!checkValue.IsObject()) { + throw std::runtime_error("JSON member '" + checkPath + "' must be an object"); + } + if (!checkValue.HasMember("type") || !checkValue["type"].IsString()) { + throw std::runtime_error("JSON member '" + checkPath + ".type' must be a string"); + } + ValidationCheckConfig check{}; + check.type = checkValue["type"].GetString(); + + if (check.type == "non_black_ratio") { + if (checkValue.HasMember("threshold")) { + check.threshold = readFloatInRange( + checkValue["threshold"], + checkPath + ".threshold", + 0.0f, + 1.0f); + } + if (checkValue.HasMember("min_ratio")) { + check.minValue = readFloatInRange( + checkValue["min_ratio"], + checkPath + ".min_ratio", + 0.0f, + 1.0f); + } + if (checkValue.HasMember("max_ratio")) { + check.maxValue = readFloatInRange( + checkValue["max_ratio"], + checkPath + ".max_ratio", + 0.0f, + 1.0f); + } + if (check.minValue > check.maxValue) { + throw std::runtime_error("JSON member '" + checkPath + + "' must have min_ratio <= max_ratio"); + } + } else if (check.type == "luma_range") { + if (!checkValue.HasMember("min_luma") || !checkValue.HasMember("max_luma")) { + throw std::runtime_error("JSON member '" + checkPath + + "' must include min_luma and max_luma"); + } + check.minValue = readFloatInRange( + checkValue["min_luma"], + checkPath + ".min_luma", + 0.0f, + 1.0f); + check.maxValue = readFloatInRange( + checkValue["max_luma"], + checkPath + ".max_luma", + 0.0f, + 1.0f); + if (check.minValue > check.maxValue) { + throw std::runtime_error("JSON member '" + checkPath + + "' must have min_luma <= max_luma"); + } + } else if (check.type == "mean_color") { + if (!checkValue.HasMember("color")) { + throw std::runtime_error("JSON member '" + checkPath + ".color' is required"); + } + readFloat3Field(checkValue["color"], checkPath + ".color", check.color); + if (checkValue.HasMember("tolerance")) { + check.tolerance = readFloatInRange( + checkValue["tolerance"], + checkPath + ".tolerance", + 0.0f, + 1.0f); + } + } else if (check.type == "sample_points") { + if (!checkValue.HasMember("points") || !checkValue["points"].IsArray()) { + throw std::runtime_error("JSON member '" + checkPath + ".points' must be an array"); + } + const auto& pointsValue = checkValue["points"]; + check.points.clear(); + check.points.reserve(pointsValue.Size()); + for (rapidjson::SizeType pointIndex = 0; pointIndex < pointsValue.Size(); ++pointIndex) { + const auto& pointValue = pointsValue[pointIndex]; + const std::string pointPath = checkPath + ".points[" + std::to_string(pointIndex) + "]"; + if (!pointValue.IsObject()) { + throw std::runtime_error("JSON member '" + pointPath + "' must be an object"); + } + ValidationSamplePointConfig point{}; + if (!pointValue.HasMember("x") || !pointValue.HasMember("y")) { + throw std::runtime_error("JSON member '" + pointPath + + "' must include x and y"); + } + point.x = readFloatInRange(pointValue["x"], pointPath + ".x", 0.0f, 1.0f); + point.y = readFloatInRange(pointValue["y"], pointPath + ".y", 0.0f, 1.0f); + if (!pointValue.HasMember("color")) { + throw std::runtime_error("JSON member '" + pointPath + ".color' is required"); + } + readFloat3Field(pointValue["color"], pointPath + ".color", point.color); + if (pointValue.HasMember("tolerance")) { + point.tolerance = readFloatInRange( + pointValue["tolerance"], + pointPath + ".tolerance", + 0.0f, + 1.0f); + } + check.points.push_back(std::move(point)); + } + } else { + throw std::runtime_error("JSON member '" + checkPath + ".type' is unsupported"); + } + + checkpoint.checks.push_back(std::move(check)); } - checkpoint.expected.maxDiffPixels = static_cast(rawValue); + } + + if (!checkpoint.expected.enabled && checkpoint.checks.empty()) { + throw std::runtime_error("JSON member '" + basePath + + "' must define 'expected' or 'checks'"); } config.validationTour.checkpoints.push_back(std::move(checkpoint)); @@ -1534,15 +1493,57 @@ std::string JsonConfigService::BuildConfigJson(const RuntimeConfig& config, cameraObject.AddMember("far", checkpoint.camera.farPlane, allocator); entry.AddMember("camera", cameraObject, allocator); - rapidjson::Value expectedObject(rapidjson::kObjectType); - expectedObject.AddMember("image", - rapidjson::Value(checkpoint.expected.imagePath.string().c_str(), allocator), - allocator); - expectedObject.AddMember("tolerance", checkpoint.expected.tolerance, allocator); - expectedObject.AddMember("max_diff_pixels", - static_cast(checkpoint.expected.maxDiffPixels), - allocator); - entry.AddMember("expected", expectedObject, allocator); + if (checkpoint.expected.enabled) { + rapidjson::Value expectedObject(rapidjson::kObjectType); + expectedObject.AddMember("image", + rapidjson::Value(checkpoint.expected.imagePath.string().c_str(), allocator), + allocator); + expectedObject.AddMember("tolerance", checkpoint.expected.tolerance, allocator); + expectedObject.AddMember("max_diff_pixels", + static_cast(checkpoint.expected.maxDiffPixels), + allocator); + entry.AddMember("expected", expectedObject, allocator); + } + + if (!checkpoint.checks.empty()) { + rapidjson::Value checksArray(rapidjson::kArrayType); + for (const auto& check : checkpoint.checks) { + rapidjson::Value checkObject(rapidjson::kObjectType); + checkObject.AddMember("type", rapidjson::Value(check.type.c_str(), allocator), allocator); + if (check.type == "non_black_ratio") { + checkObject.AddMember("threshold", check.threshold, allocator); + checkObject.AddMember("min_ratio", check.minValue, allocator); + checkObject.AddMember("max_ratio", check.maxValue, allocator); + } else if (check.type == "luma_range") { + checkObject.AddMember("min_luma", check.minValue, allocator); + checkObject.AddMember("max_luma", check.maxValue, allocator); + } else if (check.type == "mean_color") { + rapidjson::Value colorValue(rapidjson::kArrayType); + colorValue.PushBack(check.color[0], allocator); + colorValue.PushBack(check.color[1], allocator); + colorValue.PushBack(check.color[2], allocator); + checkObject.AddMember("color", colorValue, allocator); + checkObject.AddMember("tolerance", check.tolerance, allocator); + } else if (check.type == "sample_points") { + rapidjson::Value pointsArray(rapidjson::kArrayType); + for (const auto& point : check.points) { + rapidjson::Value pointValue(rapidjson::kObjectType); + pointValue.AddMember("x", point.x, allocator); + pointValue.AddMember("y", point.y, allocator); + rapidjson::Value pointColor(rapidjson::kArrayType); + pointColor.PushBack(point.color[0], allocator); + pointColor.PushBack(point.color[1], allocator); + pointColor.PushBack(point.color[2], allocator); + pointValue.AddMember("color", pointColor, allocator); + pointValue.AddMember("tolerance", point.tolerance, allocator); + pointsArray.PushBack(pointValue, allocator); + } + checkObject.AddMember("points", pointsArray, allocator); + } + checksArray.PushBack(checkObject, allocator); + } + entry.AddMember("checks", checksArray, allocator); + } checkpointsArray.PushBack(entry, allocator); } diff --git a/src/services/impl/json_config_writer_service.cpp b/src/services/impl/json_config_writer_service.cpp index 8e662df..28af737 100644 --- a/src/services/impl/json_config_writer_service.cpp +++ b/src/services/impl/json_config_writer_service.cpp @@ -289,15 +289,57 @@ void JsonConfigWriterService::WriteConfig(const RuntimeConfig& config, const std cameraObject.AddMember("far", checkpoint.camera.farPlane, allocator); entry.AddMember("camera", cameraObject, allocator); - rapidjson::Value expectedObject(rapidjson::kObjectType); - expectedObject.AddMember("image", - rapidjson::Value(checkpoint.expected.imagePath.string().c_str(), allocator), - allocator); - expectedObject.AddMember("tolerance", checkpoint.expected.tolerance, allocator); - expectedObject.AddMember("max_diff_pixels", - static_cast(checkpoint.expected.maxDiffPixels), - allocator); - entry.AddMember("expected", expectedObject, allocator); + if (checkpoint.expected.enabled) { + rapidjson::Value expectedObject(rapidjson::kObjectType); + expectedObject.AddMember("image", + rapidjson::Value(checkpoint.expected.imagePath.string().c_str(), allocator), + allocator); + expectedObject.AddMember("tolerance", checkpoint.expected.tolerance, allocator); + expectedObject.AddMember("max_diff_pixels", + static_cast(checkpoint.expected.maxDiffPixels), + allocator); + entry.AddMember("expected", expectedObject, allocator); + } + + if (!checkpoint.checks.empty()) { + rapidjson::Value checksArray(rapidjson::kArrayType); + for (const auto& check : checkpoint.checks) { + rapidjson::Value checkObject(rapidjson::kObjectType); + checkObject.AddMember("type", rapidjson::Value(check.type.c_str(), allocator), allocator); + if (check.type == "non_black_ratio") { + checkObject.AddMember("threshold", check.threshold, allocator); + checkObject.AddMember("min_ratio", check.minValue, allocator); + checkObject.AddMember("max_ratio", check.maxValue, allocator); + } else if (check.type == "luma_range") { + checkObject.AddMember("min_luma", check.minValue, allocator); + checkObject.AddMember("max_luma", check.maxValue, allocator); + } else if (check.type == "mean_color") { + rapidjson::Value colorValue(rapidjson::kArrayType); + colorValue.PushBack(check.color[0], allocator); + colorValue.PushBack(check.color[1], allocator); + colorValue.PushBack(check.color[2], allocator); + checkObject.AddMember("color", colorValue, allocator); + checkObject.AddMember("tolerance", check.tolerance, allocator); + } else if (check.type == "sample_points") { + rapidjson::Value pointsArray(rapidjson::kArrayType); + for (const auto& point : check.points) { + rapidjson::Value pointValue(rapidjson::kObjectType); + pointValue.AddMember("x", point.x, allocator); + pointValue.AddMember("y", point.y, allocator); + rapidjson::Value pointColor(rapidjson::kArrayType); + pointColor.PushBack(point.color[0], allocator); + pointColor.PushBack(point.color[1], allocator); + pointColor.PushBack(point.color[2], allocator); + pointValue.AddMember("color", pointColor, allocator); + pointValue.AddMember("tolerance", point.tolerance, allocator); + pointsArray.PushBack(pointValue, allocator); + } + checkObject.AddMember("points", pointsArray, allocator); + } + checksArray.PushBack(checkObject, allocator); + } + entry.AddMember("checks", checksArray, allocator); + } checkpointsArray.PushBack(entry, allocator); } diff --git a/src/services/impl/validation_tour_service.cpp b/src/services/impl/validation_tour_service.cpp index 2f65a16..8bdeab5 100644 --- a/src/services/impl/validation_tour_service.cpp +++ b/src/services/impl/validation_tour_service.cpp @@ -98,10 +98,7 @@ ValidationFramePlan ValidationTourService::BeginFrame(float aspect) { PendingCapture pending{}; pending.actualPath = actualPath; - pending.expectedPath = ResolvePath(checkpoint.expected.imagePath); - pending.checkpointId = checkpoint.id; - pending.tolerance = checkpoint.expected.tolerance; - pending.maxDiffPixels = checkpoint.expected.maxDiffPixels; + pending.checkpointIndex = checkpointIndex_; pending.captureIndex = captureIndex_; pendingCapture_ = std::move(pending); @@ -139,7 +136,7 @@ ValidationFrameResult ValidationTourService::EndFrame() { } std::string errorMessage; - bool ok = CompareImages(pending, errorMessage); + bool ok = AnalyzeCapture(pending, errorMessage); if (!ok) { failed_ = true; failureMessage_ = errorMessage; @@ -217,7 +214,13 @@ std::filesystem::path ValidationTourService::ResolvePath(const std::filesystem:: return std::filesystem::current_path() / path; } -bool ValidationTourService::CompareImages(const PendingCapture& pending, std::string& errorMessage) const { +bool ValidationTourService::AnalyzeCapture(const PendingCapture& pending, std::string& errorMessage) const { + if (pending.checkpointIndex >= config_.checkpoints.size()) { + errorMessage = "Validation checkpoint index out of range"; + return false; + } + const ValidationCheckpointConfig& checkpoint = config_.checkpoints[pending.checkpointIndex]; + int actualWidth = 0; int actualHeight = 0; int actualChannels = 0; @@ -228,52 +231,232 @@ bool ValidationTourService::CompareImages(const PendingCapture& pending, std::st STBI_rgb_alpha); if (!actualPixels) { errorMessage = "Validation capture missing or unreadable: " + pending.actualPath.string(); - ReportMismatch(pending, "Capture missing", errorMessage); + ReportMismatch(checkpoint.id, "Capture missing", errorMessage); + return false; + } + + bool expectedOk = CompareExpectedImage(checkpoint, actualWidth, actualHeight, actualPixels, errorMessage); + bool checksOk = expectedOk + ? ApplyChecks(checkpoint, actualWidth, actualHeight, actualPixels, errorMessage) + : false; + + stbi_image_free(actualPixels); + if (!expectedOk || !checksOk) { + return false; + } + + return true; +} + +bool ValidationTourService::ApplyChecks(const ValidationCheckpointConfig& checkpoint, + int width, + int height, + const unsigned char* pixels, + std::string& errorMessage) const { + if (checkpoint.checks.empty()) { + return true; + } + if (width <= 0 || height <= 0) { + errorMessage = "Validation check failed: invalid capture dimensions"; + ReportMismatch(checkpoint.id, "Invalid capture", errorMessage); + return false; + } + + struct NonBlackState { + float threshold = 0.05f; + float minRatio = 0.0f; + float maxRatio = 1.0f; + size_t count = 0; + size_t checkIndex = 0; + }; + + std::vector nonBlackStates; + nonBlackStates.reserve(checkpoint.checks.size()); + for (size_t index = 0; index < checkpoint.checks.size(); ++index) { + const auto& check = checkpoint.checks[index]; + if (check.type == "non_black_ratio") { + NonBlackState state{}; + state.threshold = check.threshold; + state.minRatio = check.minValue; + state.maxRatio = check.maxValue; + state.checkIndex = index; + nonBlackStates.push_back(state); + } + } + + const size_t pixelCount = static_cast(width) * static_cast(height); + const size_t totalChannels = pixelCount * 4; + double sumR = 0.0; + double sumG = 0.0; + double sumB = 0.0; + double sumLuma = 0.0; + + for (size_t idx = 0; idx < totalChannels; idx += 4) { + const double r = static_cast(pixels[idx]) / 255.0; + const double g = static_cast(pixels[idx + 1]) / 255.0; + const double b = static_cast(pixels[idx + 2]) / 255.0; + sumR += r; + sumG += g; + sumB += b; + const double luma = r * 0.2126 + g * 0.7152 + b * 0.0722; + sumLuma += luma; + if (!nonBlackStates.empty()) { + for (auto& state : nonBlackStates) { + if (luma > static_cast(state.threshold)) { + ++state.count; + } + } + } + } + + const double denom = pixelCount == 0 ? 1.0 : static_cast(pixelCount); + const double avgLuma = sumLuma / denom; + const double avgR = sumR / denom; + const double avgG = sumG / denom; + const double avgB = sumB / denom; + + for (const auto& state : nonBlackStates) { + const double ratio = static_cast(state.count) / denom; + if (ratio < static_cast(state.minRatio) || ratio > static_cast(state.maxRatio)) { + const auto& check = checkpoint.checks[state.checkIndex]; + errorMessage = "Validation check failed (non_black_ratio) checkpoint '" + checkpoint.id + + "' ratio=" + std::to_string(ratio) + + " min=" + std::to_string(check.minValue) + + " max=" + std::to_string(check.maxValue) + + " threshold=" + std::to_string(check.threshold); + ReportMismatch(checkpoint.id, "Non-black ratio mismatch", errorMessage); + return false; + } + } + + for (const auto& check : checkpoint.checks) { + if (check.type == "luma_range") { + if (avgLuma < static_cast(check.minValue) || + avgLuma > static_cast(check.maxValue)) { + errorMessage = "Validation check failed (luma_range) checkpoint '" + checkpoint.id + + "' avgLuma=" + std::to_string(avgLuma) + + " min=" + std::to_string(check.minValue) + + " max=" + std::to_string(check.maxValue); + ReportMismatch(checkpoint.id, "Luma range mismatch", errorMessage); + return false; + } + } else if (check.type == "mean_color") { + const double tol = static_cast(check.tolerance); + const double dr = std::abs(avgR - static_cast(check.color[0])); + const double dg = std::abs(avgG - static_cast(check.color[1])); + const double db = std::abs(avgB - static_cast(check.color[2])); + if (dr > tol || dg > tol || db > tol) { + errorMessage = "Validation check failed (mean_color) checkpoint '" + checkpoint.id + + "' avgColor=[" + std::to_string(avgR) + "," + + std::to_string(avgG) + "," + std::to_string(avgB) + + "] target=[" + std::to_string(check.color[0]) + "," + + std::to_string(check.color[1]) + "," + + std::to_string(check.color[2]) + "] tolerance=" + + std::to_string(check.tolerance); + ReportMismatch(checkpoint.id, "Mean color mismatch", errorMessage); + return false; + } + } else if (check.type == "sample_points") { + for (size_t pointIndex = 0; pointIndex < check.points.size(); ++pointIndex) { + const auto& point = check.points[pointIndex]; + const double xPos = std::round(static_cast(point.x) * + static_cast(width - 1)); + const double yPos = std::round(static_cast(point.y) * + static_cast(height - 1)); + const int x = std::clamp(static_cast(xPos), 0, width - 1); + const int y = std::clamp(static_cast(yPos), 0, height - 1); + const size_t pixelIndex = (static_cast(y) * static_cast(width) + + static_cast(x)) * 4; + const double r = static_cast(pixels[pixelIndex]) / 255.0; + const double g = static_cast(pixels[pixelIndex + 1]) / 255.0; + const double b = static_cast(pixels[pixelIndex + 2]) / 255.0; + const double tol = static_cast(point.tolerance); + const double dr = std::abs(r - static_cast(point.color[0])); + const double dg = std::abs(g - static_cast(point.color[1])); + const double db = std::abs(b - static_cast(point.color[2])); + if (dr > tol || dg > tol || db > tol) { + errorMessage = "Validation check failed (sample_points) checkpoint '" + checkpoint.id + + "' point=" + std::to_string(pointIndex) + + " actual=[" + std::to_string(r) + "," + + std::to_string(g) + "," + std::to_string(b) + + "] target=[" + std::to_string(point.color[0]) + "," + + std::to_string(point.color[1]) + "," + + std::to_string(point.color[2]) + "] tolerance=" + + std::to_string(point.tolerance); + ReportMismatch(checkpoint.id, "Sample point mismatch", errorMessage); + return false; + } + } + } + } + + if (logger_) { + logger_->Trace("ValidationTourService", "ApplyChecks", + "checkpoint=" + checkpoint.id + + ", avgLuma=" + std::to_string(avgLuma) + + ", avgColor=[" + std::to_string(avgR) + "," + + std::to_string(avgG) + "," + std::to_string(avgB) + "]"); + } + + return true; +} + +bool ValidationTourService::CompareExpectedImage(const ValidationCheckpointConfig& checkpoint, + int width, + int height, + const unsigned char* pixels, + std::string& errorMessage) const { + if (!checkpoint.expected.enabled) { + return true; + } + std::filesystem::path expectedPath = ResolvePath(checkpoint.expected.imagePath); + if (expectedPath.empty()) { + errorMessage = "Expected image path missing for checkpoint '" + checkpoint.id + "'"; + ReportMismatch(checkpoint.id, "Expected missing", errorMessage); return false; } int expectedWidth = 0; int expectedHeight = 0; int expectedChannels = 0; - stbi_uc* expectedPixels = stbi_load(pending.expectedPath.string().c_str(), + stbi_uc* expectedPixels = stbi_load(expectedPath.string().c_str(), &expectedWidth, &expectedHeight, &expectedChannels, STBI_rgb_alpha); if (!expectedPixels) { - stbi_image_free(actualPixels); - errorMessage = "Expected image missing or unreadable: " + pending.expectedPath.string(); - ReportMismatch(pending, "Expected missing", errorMessage); + errorMessage = "Expected image missing or unreadable: " + expectedPath.string(); + ReportMismatch(checkpoint.id, "Expected missing", errorMessage); return false; } - if (actualWidth != expectedWidth || actualHeight != expectedHeight) { - stbi_image_free(actualPixels); + if (width != expectedWidth || height != expectedHeight) { stbi_image_free(expectedPixels); - errorMessage = "Validation image size mismatch for checkpoint '" + pending.checkpointId + - "' actual=" + std::to_string(actualWidth) + "x" + std::to_string(actualHeight) + + errorMessage = "Validation image size mismatch for checkpoint '" + checkpoint.id + + "' actual=" + std::to_string(width) + "x" + std::to_string(height) + " expected=" + std::to_string(expectedWidth) + "x" + std::to_string(expectedHeight); - ReportMismatch(pending, "Image size mismatch", errorMessage); + ReportMismatch(checkpoint.id, "Image size mismatch", errorMessage); return false; } - const size_t pixelCount = static_cast(actualWidth) * static_cast(actualHeight); + const size_t pixelCount = static_cast(width) * static_cast(height); const size_t totalChannels = pixelCount * 4; size_t mismatchCount = 0; - float maxChannelDiff = 0.0f; double totalDiff = 0.0; + double maxChannelDiff = 0.0; + const double tolerance = static_cast(checkpoint.expected.tolerance); for (size_t idx = 0; idx < totalChannels; idx += 4) { bool pixelMismatch = false; for (size_t channel = 0; channel < 4; ++channel) { - int actual = actualPixels[idx + channel]; + int actual = pixels[idx + channel]; int expected = expectedPixels[idx + channel]; - float diff = static_cast(std::abs(actual - expected)) / 255.0f; - totalDiff += static_cast(diff); + double diff = static_cast(std::abs(actual - expected)) / 255.0; + totalDiff += diff; if (diff > maxChannelDiff) { maxChannelDiff = diff; } - if (diff > pending.tolerance) { + if (diff > tolerance) { pixelMismatch = true; } } @@ -282,37 +465,35 @@ bool ValidationTourService::CompareImages(const PendingCapture& pending, std::st } } - stbi_image_free(actualPixels); stbi_image_free(expectedPixels); const double averageDiff = totalChannels == 0 ? 0.0 : totalDiff / static_cast(totalChannels); - if (mismatchCount > pending.maxDiffPixels) { - errorMessage = "Validation mismatch for checkpoint '" + pending.checkpointId + + if (mismatchCount > checkpoint.expected.maxDiffPixels) { + errorMessage = "Validation mismatch for checkpoint '" + checkpoint.id + "' mismatchedPixels=" + std::to_string(mismatchCount) + - " maxAllowed=" + std::to_string(pending.maxDiffPixels) + - " tolerance=" + std::to_string(pending.tolerance) + + " maxAllowed=" + std::to_string(checkpoint.expected.maxDiffPixels) + + " tolerance=" + std::to_string(checkpoint.expected.tolerance) + " avgDiff=" + std::to_string(averageDiff) + " maxChannelDiff=" + std::to_string(maxChannelDiff) + - " actual=" + pending.actualPath.string() + - " expected=" + pending.expectedPath.string(); - ReportMismatch(pending, "Image mismatch", errorMessage); + " expected=" + expectedPath.string(); + ReportMismatch(checkpoint.id, "Image mismatch", errorMessage); return false; } if (logger_) { - logger_->Trace("ValidationTourService", "CompareImages", - "checkpoint=" + pending.checkpointId + + logger_->Trace("ValidationTourService", "CompareExpectedImage", + "checkpoint=" + checkpoint.id + ", mismatchedPixels=" + std::to_string(mismatchCount) + ", avgDiff=" + std::to_string(averageDiff)); } return true; } -void ValidationTourService::ReportMismatch(const PendingCapture& pending, +void ValidationTourService::ReportMismatch(const std::string& checkpointId, const std::string& summary, const std::string& details) const { if (logger_) { - logger_->Error("ValidationTourService::CompareImages: " + details); + logger_->Error("ValidationTourService::AnalyzeCapture: " + details); } if (!probeService_) { return; @@ -320,7 +501,7 @@ void ValidationTourService::ReportMismatch(const PendingCapture& pending, ProbeReport report{}; report.severity = ProbeSeverity::Error; report.code = "VALIDATION_MISMATCH"; - report.resourceId = pending.checkpointId; + report.resourceId = checkpointId; report.message = summary; report.details = details; probeService_->Report(report); diff --git a/src/services/impl/validation_tour_service.hpp b/src/services/impl/validation_tour_service.hpp index 75c73e7..e79d4eb 100644 --- a/src/services/impl/validation_tour_service.hpp +++ b/src/services/impl/validation_tour_service.hpp @@ -22,18 +22,25 @@ public: private: struct PendingCapture { std::filesystem::path actualPath; - std::filesystem::path expectedPath; - std::string checkpointId; - float tolerance = 0.01f; - size_t maxDiffPixels = 0; + size_t checkpointIndex = 0; size_t captureIndex = 0; }; void AdvanceCheckpoint(); ViewState BuildViewState(const ValidationCameraConfig& camera, float aspect) const; std::filesystem::path ResolvePath(const std::filesystem::path& path) const; - bool CompareImages(const PendingCapture& pending, std::string& errorMessage) const; - void ReportMismatch(const PendingCapture& pending, + bool AnalyzeCapture(const PendingCapture& pending, std::string& errorMessage) const; + bool ApplyChecks(const ValidationCheckpointConfig& checkpoint, + int width, + int height, + const unsigned char* pixels, + std::string& errorMessage) const; + bool CompareExpectedImage(const ValidationCheckpointConfig& checkpoint, + int width, + int height, + const unsigned char* pixels, + std::string& errorMessage) const; + void ReportMismatch(const std::string& checkpointId, const std::string& summary, const std::string& details) const; diff --git a/src/services/interfaces/config_types.hpp b/src/services/interfaces/config_types.hpp index 19b18b5..bd53bce 100644 --- a/src/services/interfaces/config_types.hpp +++ b/src/services/interfaces/config_types.hpp @@ -165,11 +165,35 @@ struct ValidationCameraConfig { * @brief Expected output for a validation checkpoint. */ struct ValidationExpectedConfig { + bool enabled = false; std::filesystem::path imagePath; float tolerance = 0.01f; size_t maxDiffPixels = 0; }; +/** + * @brief Sample point expectation for validation checks. + */ +struct ValidationSamplePointConfig { + float x = 0.5f; + float y = 0.5f; + std::array color = {0.0f, 0.0f, 0.0f}; + float tolerance = 0.1f; +}; + +/** + * @brief Validation check definitions that do not require a baseline image. + */ +struct ValidationCheckConfig { + std::string type; + float minValue = 0.0f; + float maxValue = 1.0f; + float threshold = 0.05f; + float tolerance = 0.1f; + std::array color = {0.0f, 0.0f, 0.0f}; + std::vector points{}; +}; + /** * @brief A single validation checkpoint definition. */ @@ -177,6 +201,7 @@ struct ValidationCheckpointConfig { std::string id; ValidationCameraConfig camera{}; ValidationExpectedConfig expected{}; + std::vector checks{}; }; /**