Fixed inconcistencies - only processes MAP_ files now
This commit is contained in:
96
ProjectNotes/MAP_Prefix_Enforcement_Plan.md
Normal file
96
ProjectNotes/MAP_Prefix_Enforcement_Plan.md
Normal file
@@ -0,0 +1,96 @@
|
||||
# Plan: Enforcing "MAP_" Prefix for Internal Processing and Standard Type for Output Naming
|
||||
|
||||
**Date:** 2025-05-13
|
||||
|
||||
**I. Goal:**
|
||||
The primary goal is to ensure that for all internal processing, the system *exclusively* uses `FileRule.item_type` values that start with the "MAP_" prefix (e.g., "MAP_COL", "MAP_NRM"). The "standard type" (e.g., "COL", "NRM") associated with these "MAP_" types (as defined in `config/app_settings.json`) should *only* be used during the file saving stages for output naming. Any `FileRule` whose `item_type` does not start with "MAP_" (and isn't a special type like "EXTRA" or "MODEL") should be skipped by the relevant map processing stages.
|
||||
|
||||
**II. Current State Analysis Summary:**
|
||||
|
||||
* **Output Naming:** The use of "standard type" for output filenames via the `get_filename_friendly_map_type` utility in `SaveVariantsStage` and `OutputOrganizationStage` is **correct** and already meets the requirement.
|
||||
* **Internal "MAP_" Prefix Usage:**
|
||||
* Some stages like `GlossToRoughConversionStage` correctly check for "MAP_" prefixes (e.g., `processing_map_type.startswith("MAP_GLOSS")`).
|
||||
* Other stages like `RegularMapProcessorStage` and `MergedTaskProcessorStage` (and its helpers) implicitly expect "MAP_" prefixed types for their internal regex-based logic but lack explicit checks to skip items if the prefix is missing.
|
||||
* Stages like `AlphaExtractionToMaskStage` and `NormalMapGreenChannelStage` currently use non-"MAP_" prefixed "standard types" (e.g., "NORMAL", "ALBEDO") when reading from `context.processed_maps_details` for their decision-making logic.
|
||||
* The `PrepareProcessingItemsStage` adds `FileRule`s to the processing queue without filtering based on the "MAP_" prefix in `item_type`.
|
||||
* **Data Consistency in `AssetProcessingContext`:**
|
||||
* `FileRule.item_type` is the field that should hold the "MAP_" prefixed type from the initial rule generation.
|
||||
* `context.processed_maps_details` entries can contain various map type representations:
|
||||
* `map_type`: Often stores the "standard type" (e.g., "Roughness", "MASK", "NORMAL").
|
||||
* `processing_map_type` / `internal_map_type`: Generally seem to store the "MAP_" prefixed type. This needs to be consistent.
|
||||
* **Configuration (`config/app_settings.json`):**
|
||||
* `FILE_TYPE_DEFINITIONS` correctly use "MAP_" prefixed keys.
|
||||
* `MAP_MERGE_RULES` need to be reviewed to ensure their `output_map_type` and input map types are "MAP_" prefixed.
|
||||
|
||||
**III. Proposed Changes (Code Identification & Recommendations):**
|
||||
|
||||
**A. Enforce "MAP_" Prefix for Processing Items (Skipping Logic):**
|
||||
The core requirement is that processing stages should skip `FileRule` items if their `item_type` doesn't start with "MAP_".
|
||||
|
||||
1. **`RegularMapProcessorStage` (`processing/pipeline/stages/regular_map_processor.py`):**
|
||||
* **Identify:** In the `execute` method, `initial_internal_map_type` is derived from `file_rule.item_type_override` or `file_rule.item_type`.
|
||||
* **Recommend:** Add an explicit check after determining `initial_internal_map_type`. If `initial_internal_map_type` does not start with `"MAP_"`, the stage should log a warning, set the `result.status` to "Skipped (Invalid Type)" or similar, and return `result` early, effectively skipping processing for this item.
|
||||
|
||||
2. **`MergedTaskProcessorStage` (`processing/pipeline/stages/merged_task_processor.py`):**
|
||||
* **Identify:** This stage processes `MergeTaskDefinition`s. The definitions for these tasks (input types, output type) come from `MAP_MERGE_RULES` in `config/app_settings.json`. The stage uses `required_map_type_from_rule` for its inputs.
|
||||
* **Recommend:**
|
||||
* **Configuration First:** Review all entries in `MAP_MERGE_RULES` in `config/app_settings.json`.
|
||||
* Ensure the `output_map_type` for each rule (e.g., "MAP_NRMRGH") starts with "MAP_".
|
||||
* Ensure all map type values within the `inputs` dictionary (e.g., `"R": "MAP_NRM"`) start with "MAP_".
|
||||
* **Stage Logic:** In the `execute` method, when iterating through `merge_inputs_config.items()`, check if `required_map_type_from_rule` starts with `"MAP_"`. If not, log a warning and either:
|
||||
* Skip loading/processing this specific input channel (potentially using its fallback if the overall merge can still proceed).
|
||||
* Or, if a non-"MAP_" input is critical, fail the entire merge task for this asset.
|
||||
* The helper `_apply_in_memory_transformations` already uses regex expecting "MAP_" prefixes; this will naturally fail or misbehave if inputs are not "MAP_" prefixed, reinforcing the need for the check above.
|
||||
|
||||
**B. Standardize Map Type Fields and Usage in `context.processed_maps_details`:**
|
||||
Ensure consistency in how "MAP_" prefixed types are stored and accessed within `context.processed_maps_details` for internal logic (not naming).
|
||||
|
||||
1. **Recommendation:** Establish a single, consistent field name within `context.processed_maps_details` to store the definitive "MAP_" prefixed internal map type (e.g., `internal_map_type` or `processing_map_type`). All stages that perform logic based on the specific *kind* of map (e.g., transformations, source selection) should read from this standardized field. The `map_type` field can continue to store the "standard type" (e.g., "Roughness") primarily for informational/metadata purposes if needed, but not for core processing logic.
|
||||
|
||||
2. **`AlphaExtractionToMaskStage` (`processing/pipeline/stages/alpha_extraction_to_mask.py`):**
|
||||
* **Identify:**
|
||||
* Checks for existing MASK map using `file_rule.map_type == "MASK"`. (Discrepancy: `FileRule` uses `item_type`).
|
||||
* Searches for suitable source maps using `details.get('map_type') in self.SUITABLE_SOURCE_MAP_TYPES` where `SUITABLE_SOURCE_MAP_TYPES` are standard types like "ALBEDO".
|
||||
* When adding new details, it sets `map_type: "MASK"` and the new `FileRule` gets `item_type="MAP_MASK"`.
|
||||
* **Recommend:**
|
||||
* Change the check for an existing MASK map to `file_rule.item_type == "MAP_MASK"`.
|
||||
* Modify the source map search to use the standardized "MAP_" prefixed field from `details` (e.g., `details.get('internal_map_type')`) and update `SUITABLE_SOURCE_MAP_TYPES` to be "MAP_" prefixed (e.g., "MAP_COL", "MAP_ALBEDO").
|
||||
* When adding new details for the created MASK map to `context.processed_maps_details`, ensure the standardized "MAP_" prefixed field is set to "MAP_MASK", and `map_type` (if kept) is "MASK".
|
||||
|
||||
3. **`NormalMapGreenChannelStage` (`processing/pipeline/stages/normal_map_green_channel.py`):**
|
||||
* **Identify:** Checks `map_details.get('map_type') == "NORMAL"`.
|
||||
* **Recommend:** Change this check to use the standardized "MAP_" prefixed field from `map_details` (e.g., `map_details.get('internal_map_type')`) and verify if it `startswith("MAP_NRM")`.
|
||||
|
||||
4. **`GlossToRoughConversionStage` (`processing/pipeline/stages/gloss_to_rough_conversion.py`):**
|
||||
* **Identify:** This stage already uses `processing_map_type.startswith("MAP_GLOSS")` and updates `processing_map_type` to "MAP_ROUGH" in `map_details`. It also updates the `FileRule.item_type` correctly.
|
||||
* **Recommend:** This stage is largely consistent. Ensure the field it reads/writes (`processing_map_type`) aligns with the chosen standardized "MAP_" prefixed field for `processed_maps_details`.
|
||||
|
||||
**C. Review Orchestration Logic (Conceptual):**
|
||||
* When the orchestrator populates `context.processed_maps_details` after stages like `SaveVariantsStage`, ensure it stores the "MAP_" prefixed `internal_map_type` (from `SaveVariantsInput`) into the chosen standardized field in `processed_maps_details`.
|
||||
|
||||
**IV. Testing Recommendations:**
|
||||
|
||||
* Create test cases with `AssetRule`s containing `FileRule`s where `item_type` is intentionally set to a non-"MAP_" prefixed value (e.g., "COLOR_MAP", "TEXTURE_ROUGH"). Verify that `RegularMapProcessorStage` skips these.
|
||||
* Modify `MAP_MERGE_RULES` in a test configuration:
|
||||
* Set an `output_map_type` to a non-"MAP_" value.
|
||||
* Set an input map type (e.g., for channel "R") to a non-"MAP_" value.
|
||||
* Verify that `MergedTaskProcessorStage` correctly handles these (e.g., fails the task, skips the input, logs warnings).
|
||||
* Test `AlphaExtractionToMaskStage`:
|
||||
* With an existing `FileRule` having `item_type="MAP_MASK"` to ensure extraction is skipped.
|
||||
* With source maps having "MAP_COL" (with alpha) as their `internal_map_type` in `processed_maps_details` to ensure they are correctly identified as sources.
|
||||
* Test `NormalMapGreenChannelStage` with a normal map having "MAP_NRM" as its `internal_map_type` in `processed_maps_details` to ensure it's processed.
|
||||
* Verify that output filenames continue to use the "standard type" (e.g., "COL", "ROUGH", "NRM") correctly.
|
||||
|
||||
**V. Mermaid Diagram (Illustrative Flow for `FileRule` Processing):**
|
||||
|
||||
```mermaid
|
||||
graph TD
|
||||
A[AssetRule with FileRules] --> B{FileRuleFilterStage};
|
||||
B -- files_to_process --> C{PrepareProcessingItemsStage};
|
||||
C -- processing_items (FileRule) --> D{PipelineOrchestrator};
|
||||
D -- FileRule --> E(RegularMapProcessorStage);
|
||||
E --> F{Check FileRule.item_type};
|
||||
F -- Starts with "MAP_"? --> G[Process Map];
|
||||
F -- No --> H[Skip Map / Log Warning];
|
||||
G --> I[...subsequent stages...];
|
||||
H --> I;
|
||||
Reference in New Issue
Block a user