diff --git a/CLAUDE.md b/CLAUDE.md index 8441ecc..400ca92 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1115,6 +1115,239 @@ catch a Dart `@JsonValue` drifting from the Rust serde name. > spellings genuinely differ between plugins: mvtools takes `thsad`, mClean's > wrapper takes `thSAD`. Don't "normalise" one to the other. +### `visibleWhen` only works inside `ui`, and 8 schemas had it outside + +Reviewed 2026-08-18 after Chroma Fixes was reported as unintuitive. The panel was +showing every tuning slider for every repair at once, and the cause was not the +grouping — it was that **none of the conditions were being read**. + +`ParameterDefinition` declares no `visibleWhen`; only `ParameterUiConfig` does. +A schema that writes it one level up therefore has it **dropped at parse time**, +and the control is always visible. Measured across the shipped schemas: **35 +parameters in 8 files**, plus **12 sections** — and `UiSection` has no +`visibleWhen` either, so those did nothing at all. The pattern is a clean split by +age: everything written before the 2026-08-17 build-out put it in `ui` correctly, +everything added during it did not. + +What that shipped, beyond Chroma Fixes: Noise Reduction showed its mClean and +TemporalDegrain2 knobs under SMDegrain, SpotLess showed RemoveDirt's, Chroma +Denoise showed CCD's threshold and sampling sliders under Cnr4, Deflicker showed +both methods' at once. All fixed by moving the key; section conditions were pushed +down onto their member parameters, which is the only place they work. + +> **Both directions are now linted** in +> `app/test/filter_schema_curation_test.dart`: nothing may declare `visibleWhen` +> outside a parameter's `ui`, and every condition must name a parameter that +> exists — a `method` condition naming a method the filter doesn't offer can +> *never* be satisfied, so the control it guards would be invisible forever. +> That is the worse of the two silent failures, and nothing else would catch it. + +> **Sections were not visible groups, and now they are.** +> `DynamicFilterPanelCompact` used to render a heading **only** for an +> `advancedOnly` section, and only in advanced mode — so every ordinary section's +> parameters ran together as one undifferentiated list and a `title` was +> effectively a comment. It now prints the title for every section, in both +> modes, **when the schema declares more than one**; a single-section schema +> still gets none, because there is nothing to tell it apart from and most of +> them call it "Settings". `advancedOnly` headings keep the accent colour. +> Pinned by `dynamic_filter_panel_advanced_test.dart`. + +### Chroma Fixes: five repairs, one switch each, tuning behind advanced mode + +The restructure that came out of the same review. The pass holds five unrelated +repairs — alignment, bleeding, dot crawl, rainbowing, chroma combing — and it now +reads as five: a switch, the one or two controls you would actually reach for, and +an `advancedOnly` tuning section for the thresholds, per filter, in that order. +(Per *filter*, not per repair — dot crawl and rainbowing each offer two, and +several of their labels repeat, so a shared tuning section would give no way to +tell which slider belonged to which.) +Default state is eight checkboxes instead of ~25 always-on controls. + +Three things worth keeping: + +> **Automatic and manual alignment are alternatives, and the script has to agree +> with the panel.** `_auto_chroma_fix` measures the misalignment and corrects it, +> and its block runs **before** the manual Y/C shift — so with both set the +> picture was shifted by a measured amount and then again by a number the user +> guessed, silently. The schema hides the manual controls +> (`visibleWhen: {"applyAutoChroma": false}`) and +> `ChromaFixParameters::effective_apply_chroma_shift` (Rust) / +> `effectiveApplyChromaShift` (Dart) is the single derivation both the generator +> and the pass summary ask. It deliberately does **not** clear +> `apply_chroma_shift`, so turning automatic off restores what the user set by +> hand. `test_150` and the Dart twin in `integration_filter_parameters_test.dart` +> pin all four combinations. + +> **`optional: true` was a lie on every one of these parameters.** It promises +> "unticked → omitted → the plugin's own default applies", but +> `script_generator.rs` passes `Some(...)` for all of them unconditionally, so +> unticking silently substituted *our* default instead (havsfunc's `thr` default +> is 4.0; ours is 0.7). Combined with each repair's own switch it also meant two +> checkboxes per slider. All dropped, and the curation test now asserts nothing +> in this schema is `optional`. + +> **`copyWith` silently reset two repairs.** It never gained parameters for +> `applyAutoChroma`/`applyDedot` or their tuning, so it reconstructed them at +> their defaults — and `ProcessingPipeline.togglePass` calls +> `chromaFixes.copyWith(enabled: …)`. Switching the pass off and on again +> therefore discarded the DeDot that **VHS Cleanup** turns on and the automatic +> alignment that **DV Camcorder Tape** turns on, with no error. When a field is +> added to a parameter model, `copyWith` is the third place to change, and it +> fails quietly rather than not compiling. + +`bifrostInterlaced`, `deRainbowUseLuma` and `deRainbowLinkUv` are still +deliberately UI-less — they are sent at their defaults and nothing exposes them. +Note `bifrostInterlaced` defaults **true**, which is a question about progressive +sources rather than about the panel. + +### The panel audit (2026-08-18): what the newly-visible headings exposed + +Making section titles render (above) put every schema's grouping on screen for +the first time, so all 21 were audited together. Three defects, each in more than +one schema, and each invisible until then: + +> **A method dropdown that gates nothing.** `color_correction` declared +> `tweak` / `white_balance` and `crop_resize` declared +> `standard` / `nnedi3_2x` / `eedi3_2x`. In both, **no parameter carried a +> `method` condition**, neither model has a `method` field, and the converter +> either hardcoded one value or emitted none — so the dropdown rendered, +> responded, and changed nothing. Crop & Resize is the sharper case: its real +> upscaler choice is the `upscaleMethod` *parameter*, which does gate its tuning, +> so the dropdown was an inert duplicate of a working control. Both are single- +> method now, which suppresses the dropdown entirely. Remember that a method's +> `parameters` list does **not** drive the panel when `ui.sections` exists — +> sections win — so listing parameters under a method is not gating them. + +> **A parameter only some methods use, shown for all of them.** `deblock`'s +> `quant1` was ungated while DCTFilter — which does not take it — was one of the +> three methods on offer. Now gated to the two that use it. + +> **Sections whose title was written as a comment, not a heading.** Three +> schemas held a section containing only `method`, which renders nothing because +> the panel draws the dropdown itself and always skips that parameter. And +> `chroma_denoise` / `spotless` / `noise_reduction` mixed "Settings" with +> siblings named after a filter ("Cnr4 settings", "RemoveDirt settings"), so the +> generic one read as if it applied to everything. Titles now name the filter +> whose controls they hold ("CCD settings", "SpotLess settings", "SMDegrain"), +> and the dead sections are gone. + +All three are linted in `filter_schema_curation_test.dart`: at least one +parameter conditional on `method` wherever a schema declares two or more, a +method condition matching exactly the methods that list the parameter, and no +section consisting solely of `method`. + +Deliberately **not** changed: `deinterlace`'s fourteen section titles (QTGMC's +grouping is established, documented and heavily referenced), and the mixed +Title Case / sentence case of parameter labels across schemas — now more visible +side by side under headings, but a cosmetic sweep of several hundred strings that +should be its own change. + +### Presets are the other way a hidden setting arrives + +A preset is the main route by which settings appear without anyone touching a +control — so it is the main route by which a setting the user *cannot see* +appears. The panel hides a parameter whose `visibleWhen` is unsatisfied and skips +one no section lists, silently in both cases, so a preset can enable a filter that +has no control on screen, cannot be adjusted and cannot be switched off. + +Audited 2026-08-18 against the panel changes above: **the nine built-ins map +cleanly.** Worth recording why, because most of it is luck rather than design: + +- **No built-in uses Colour Correction at all**, so neither the `apply_levels` + fix nor the automatic-levels precedence can change what any of them render. + (That gap is itself noted under "Audit the presets whenever a pass ships".) +- The three that use Chroma Fixes — VHS Cleanup (DeDot), DV Camcorder Tape + (bleeding + automatic alignment), Anime DVD (DeRainbow + DeDot) — set **no + manual chroma shift**, so nothing collides with the automatic pass. +- PAL DVD's deblock uses Deblock_QED, so `quant1` is still on screen under its + new method gating. +- Removing the inert methods from `color_correction` and `crop_resize` can't + affect a preset, built-in or user-saved: neither model has a `method` field, so + no stored pipeline ever names one. + +> **`builtin-fast` sets a QTGMC preset while selecting Bwdif, and that is +> deliberate** — the source says so: "kept so that switching method in the UI +> lands somewhere sensible". The panel hides it (a `method` condition), nothing +> applies it, and `preset_visibility_test.dart` therefore exempts values hidden +> **only** by a method condition. Don't "fix" it by clearing the value. + +`app/test/preset_visibility_test.dart` walks every built-in's enabled passes +through the real converter and fails if a value differing from the schema default +is not reachable in that preset's own state. Advanced-only values are allowed +through a **named allowlist** — currently three, all QTGMC's, in a pass whose own +control and summary say it is doing something expert — and a companion test fails +if that list gains a stale entry, so it cannot quietly become a rubber stamp. + +### Color Correction: automatic and manual belong in the same group + +The pass reported as unintuitive (2026-08-18) — Chroma Fixes was reviewed first +by mistake, and the same three faults turned out to be in both. + +It offers two adjustments that can be made **automatically or by hand**, levels +and white balance, and the automatic halves used to sit together in an +"Automatic" section at the top, three groups above the manual halves they +supersede. Each pair now shares a section, automatic first, so the relationship +is visible without reading the descriptions. + +Four things were wrong underneath, all silent: + +> **The Method dropdown changed nothing.** The schema declared `tweak` and +> `white_balance` as methods, but **no parameter was conditional on the choice**, +> neither model has a `method` field, and `fromColorCorrection` hardcodes +> `'method': 'tweak'`. Both groups of controls rendered whatever was selected, and +> the worker never saw the value. It is one method now, so no dropdown renders. +> A method that gates nothing is worse than no method: it teaches the user the +> panel responds to it. + +> **`apply_levels` was a UI-only flag.** `script_generator.rs` decided by value — +> `has_levels` was true whenever any level differed from its default — so +> unticking "Levels" left the adjustment running with nothing on screen able to +> stop it (the sliders hide with the switch). `effective_apply_levels()` is the +> switch now, and it still requires something to do, so an identity mapping emits +> nothing. `test_151` pins both directions. + +> **Automatic levels supersedes the manual points, but not gamma.** The automatic +> block runs first and places black and white itself, so manual input/output +> points on top grade an already-graded picture with numbers measured against the +> original. `effective_levels_points()` drops them and the panel hides them +> (`visibleWhen: {"applyAutoLevels": false}`). **Gamma is deliberately kept** — +> automatic levels never touches the midtones, and the Levels group is the only +> place in the app to reach them. That asymmetry is the whole design; don't +> "simplify" it into hiding the group. + +> **White balance is the opposite case and must stay composable.** Automatic +> white balance neutralises the cast; temperature and tint then offset the +> result deliberately ("neutral, but a little warmer"). Same shape as levels, +> different answer, because an offset still means what its label says after a +> correction while a mapping does not. `pass_advice.dart` says which order they +> happen in rather than hiding anything. + +`copyWith` had dropped all six automatic fields, which is the systemic bug below. + +### `copyWith` silently resets what it forgets, in six models + +`ProcessingPipeline.togglePass` rebuilds a pass through `copyWith`, so a field the +method never gained is a field that reverts to its default when the user flicks +the pass switch. It compiles, it runs, and the pass then does something other than +what the panel says. + +Audited 2026-08-18 across every parameter model: **26 fields in 6 models**, all +added during the 2026-08-17 build-out — Colour Correction's six automatic fields, +Chroma Fixes' automatic alignment and DeDot (which VHS Cleanup and DV Camcorder +Tape turn on, so those presets lost settings to one click), thirteen in Noise +Reduction including every mClean and TemporalDegrain2 value, SpotLess's `method` +(so RemoveDirt silently reverted to SpotLess), QTGMC's `bwdifEdeint` and +Subtitles' `burnInPath`. + +> **`app/test/parameter_copy_with_test.dart` checks it two ways, and it needs +> both.** The behavioural pass fills every field through `fromJson`, calls +> `copyWith()` and compares — but it cannot perturb a **string or enum** value +> (an invented one would not decode), which is exactly the shape of the SpotLess +> and Subtitles bugs. So a second pass reads the model source and asserts every +> `final` field of the parameter class appears in the `copyWith` body. Deleting +> either pass loses a real class of bug; both were verified against the actual +> defects. + ### Suggestions and advice are hints, and must stay hints Two small pure-function models sit beside the pass list, and both are diff --git a/README.md b/README.md index 4e5aa46..657f776 100644 --- a/README.md +++ b/README.md @@ -111,8 +111,8 @@ Twenty-one filters, each switchable independently, applied in a fixed order. Mos | **Film Grain** | Grain added back after denoising, so the picture is not left plastic — and to hide banding in skies and fades. | | **Rotate / Flip** | Footage shot sideways, mirrored captures, scans that came off the scanner the wrong way round. | | **Sharpen** | Soft sources needing edge and fine detail recovery. aWarpSharp2 sharpens by warping edges instead of raising contrast, so it adds no halos. | -| **Chroma Fixes** | Color bleeding past edges, rainbowing and dot crawl — including the shimmering kind that only shows when the picture moves — and residual combing. | -| **Color Correction** | Brightness, contrast, saturation, hue, levels, white balance (warm/cool, green/magenta), and lifting detail out of the shadows of underexposed footage. | +| **Chroma Fixes** | Colour that sits sideways from the picture (corrected automatically or by hand), bleeding past edges, rainbowing and dot crawl — including the shimmering kind that only shows when the picture moves — and residual combing. Each repair has its own switch, and its settings appear only once it is on. | +| **Color Correction** | Brightness, contrast, saturation, hue, levels, white balance (warm/cool, green/magenta), and lifting detail out of the shadows of underexposed footage. Levels and white balance can each be measured automatically or set by hand. | | **Crop & Resize** | Trimming overscan, scaling, and edge-directed upscaling. | | **Frame Rate** | Converting between PAL and NTSC rates, for a tape that was already converted once and now plays at the wrong speed. | | **Subtitles** | Whisper AI speech-to-text, to `.srt`, embedded, or both. | diff --git a/app/assets/filters/core/chroma_denoise.json b/app/assets/filters/core/chroma_denoise.json index 15b3cb1..a1fbdc8 100644 --- a/app/assets/filters/core/chroma_denoise.json +++ b/app/assets/filters/core/chroma_denoise.json @@ -77,7 +77,12 @@ "label": "Threshold", "description": "How different a neighbouring pixel's colour may be and still be averaged in. Higher cleans more and can bleed colour across edges", "widget": "slider", - "precision": 1 + "precision": 1, + "visibleWhen": { + "method": [ + "ccd" + ] + } } }, "temporalRadius": { @@ -93,7 +98,12 @@ "ui": { "label": "Temporal Radius", "description": "Frames on each side to average with. 0 works on each frame alone; higher cleans more but can smear fast colour movement", - "widget": "slider" + "widget": "slider", + "visibleWhen": { + "method": [ + "ccd" + ] + } } }, "pointsLow": { @@ -102,7 +112,12 @@ "optional": true, "ui": { "label": "Near Samples", - "description": "Include the closest ring of sample points" + "description": "Include the closest ring of sample points", + "visibleWhen": { + "method": [ + "ccd" + ] + } } }, "pointsMedium": { @@ -111,7 +126,12 @@ "optional": true, "ui": { "label": "Mid Samples", - "description": "Include the mid-distance ring of sample points" + "description": "Include the mid-distance ring of sample points", + "visibleWhen": { + "method": [ + "ccd" + ] + } } }, "pointsHigh": { @@ -120,7 +140,12 @@ "optional": true, "ui": { "label": "Far Samples", - "description": "Include the outermost ring of sample points. Cleans large blotches at the cost of speed and some colour bleed" + "description": "Include the outermost ring of sample points. Cleans large blotches at the cost of speed and some colour bleed", + "visibleWhen": { + "method": [ + "ccd" + ] + } } }, "scale": { @@ -137,7 +162,12 @@ "label": "Sample Scale", "description": "How far apart the sample points sit. Left off, this is derived from the frame height (CCD was written for 240p), which is almost always what you want", "widget": "slider", - "precision": 1 + "precision": 1, + "visibleWhen": { + "method": [ + "ccd" + ] + } } }, "cnr4Strength": { @@ -146,15 +176,15 @@ "min": 0, "max": 255, "step": 8, - "visibleWhen": { - "method": [ - "cnr4" - ] - }, "ui": { "label": "Strength", "description": "How far colour is pulled toward the surrounding frames. The default already sits near the top of the range, so there is far more room downward than up.", - "widget": "slider" + "widget": "slider", + "visibleWhen": { + "method": [ + "cnr4" + ] + } } }, "cnr4Sense": { @@ -163,15 +193,15 @@ "min": 0, "max": 255, "step": 5, - "visibleWhen": { - "method": [ - "cnr4" - ] - }, "ui": { "label": "Motion sensitivity", "description": "How much movement is tolerated before the filter stops correcting. Raise it if noise survives in moving areas; lower it if moving colour smears.", - "widget": "slider" + "widget": "slider", + "visibleWhen": { + "method": [ + "cnr4" + ] + } } }, "cnr4Radius": { @@ -180,15 +210,15 @@ "min": 1, "max": 8, "step": 1, - "visibleWhen": { - "method": [ - "cnr4" - ] - }, "ui": { "label": "Temporal radius", "description": "How many frames either side are considered. Higher is stronger and slower.", - "widget": "slider" + "widget": "slider", + "visibleWhen": { + "method": [ + "cnr4" + ] + } } }, "cnr4Tmode": { @@ -197,15 +227,15 @@ "min": 0, "max": 3, "step": 1, - "visibleWhen": { - "method": [ - "cnr4" - ] - }, "ui": { "label": "Detail retention", "description": "Higher settings protect fine chroma detail at the cost of some smoothing.", - "widget": "slider" + "widget": "slider", + "visibleWhen": { + "method": [ + "cnr4" + ] + } } }, "cnr4Wmode": { @@ -214,42 +244,30 @@ "min": 0, "max": 2, "step": 1, - "visibleWhen": { - "method": [ - "cnr4" - ] - }, "ui": { "label": "Weighting", "description": "How neighbouring frames are weighted against the current one.", - "widget": "slider" + "widget": "slider", + "visibleWhen": { + "method": [ + "cnr4" + ] + } } } }, "ui": { "sections": [ { - "title": "Method", - "parameters": [ - "method" - ], - "expanded": true - }, - { - "title": "Settings", + "title": "CCD settings", "parameters": [ "threshold", "temporalRadius" ], - "expanded": true, - "visibleWhen": { - "method": [ - "ccd" - ] - } + "expanded": true }, { - "title": "Sampling", + "title": "CCD sampling", "parameters": [ "pointsLow", "pointsMedium", @@ -257,12 +275,7 @@ "scale" ], "expanded": false, - "advancedOnly": true, - "visibleWhen": { - "method": [ - "ccd" - ] - } + "advancedOnly": true }, { "title": "Cnr4 settings", @@ -271,12 +284,7 @@ "cnr4Sense", "cnr4Radius" ], - "expanded": true, - "visibleWhen": { - "method": [ - "cnr4" - ] - } + "expanded": true }, { "title": "Cnr4 tuning", @@ -285,12 +293,7 @@ "cnr4Wmode" ], "expanded": false, - "advancedOnly": true, - "visibleWhen": { - "method": [ - "cnr4" - ] - } + "advancedOnly": true } ] }, diff --git a/app/assets/filters/core/chroma_fixes.json b/app/assets/filters/core/chroma_fixes.json index 02c9388..543b17b 100644 --- a/app/assets/filters/core/chroma_fixes.json +++ b/app/assets/filters/core/chroma_fixes.json @@ -1,10 +1,10 @@ { "$schema": "https://vapourbox.app/schemas/filter-v1.json", "id": "chroma_fixes", - "version": "1.0.0", + "version": "1.1.0", "name": "Chroma Fixes", - "description": "Fix chroma bleeding, rainbows, and dot crawl", - "longDescription": "Repairs colour-specific damage from analog and composite video: chroma shifted sideways from the luma it belongs to, colour bleeding past edges, rainbow shimmer over fine patterns, and dots crawling along edges.\n\nUse it for VHS, Video8 and other composite captures. Each fix is enabled separately, so turn on only the ones matching what you can actually see in the preview \u2014 every one of them costs some colour detail.", + "description": "Fix colour alignment, bleeding, rainbows and dot crawl", + "longDescription": "Repairs colour-specific damage from analog and composite video. It covers five separate problems, each with its own switch, in the order they appear below:\n\n**Alignment** \u2014 the colour sits sideways or up/down from the picture it belongs to. Correct it automatically or by hand; these are alternatives, not a pair.\n\n**Bleeding** \u2014 colour smears past the edge it belongs to, most obviously on strong reds.\n\n**Dot crawl** \u2014 dots crawling along sharp colour edges.\n\n**Rainbowing** \u2014 shimmering colour bands over fine detail.\n\n**Chroma combing** \u2014 leftover comb teeth in the colour planes.\n\nDot crawl and rainbowing each offer a second removal that compares neighbouring frames rather than working inside one. They reach different patterns, so they are worth combining rather than choosing between.\n\nTurn on only what you can actually see in the preview \u2014 every one of these costs some colour detail.", "category": "cleanup", "icon": "palette", "order": 8, @@ -20,21 +20,36 @@ { "id": "chroma_fixes", "name": "Chroma Fixes", - "description": "Fix chroma bleeding, dot crawl, and combing artifacts (enable individually below)", + "description": "Alignment, bleeding, dot crawl, rainbowing and chroma combing, each switched on separately below", "function": "custom", "parameters": [ + "applyAutoChroma", + "autoChromaMaxShift", + "autoChromaAccuracy", + "autoChromaReferenceFrame", "applyChromaShift", "chromaShiftH", "chromaShiftV", "applyChromaBleedingFix", + "chromaBleedStrength", + "chromaBleedCBlur", "chromaBleedCx", "chromaBleedCy", - "chromaBleedCBlur", - "chromaBleedStrength", "applyDeCrawl", "deCrawlYThresh", "deCrawlCThresh", "deCrawlMaxDiff", + "applyDedot", + "dedotLuma2d", + "dedotLumaT", + "dedotChromaT1", + "dedotChromaT2", + "applyDeRainbow", + "deRainbowCThresh", + "deRainbowYThresh", + "applyBifrost", + "bifrostLumaThresh", + "bifrostVariation", "applyVinverse", "vinverseSstr", "vinverseAmnt" @@ -49,12 +64,71 @@ "hidden": true } }, + "applyAutoChroma": { + "type": "boolean", + "default": false, + "ui": { + "label": "Correct colour alignment automatically", + "description": "Measures how far the colour has slipped from the picture and shifts it back, so you do not have to guess it on a slider. If it cannot measure the source reliably it leaves the picture alone rather than guessing. This replaces the manual sliders \u2014 with it on, they are not used.", + "widget": "checkbox" + } + }, + "autoChromaMaxShift": { + "type": "integer", + "default": 2, + "min": 1, + "max": 8, + "step": 1, + "ui": { + "label": "Search range", + "description": "The largest misalignment to look for, in pixels.", + "widget": "slider", + "visibleWhen": { + "applyAutoChroma": true + } + } + }, + "autoChromaAccuracy": { + "type": "number", + "default": 0.25, + "min": 0.05, + "max": 1.0, + "step": 0.05, + "ui": { + "label": "Precision", + "description": "Smaller finds sub-pixel shifts, at some cost.", + "widget": "slider", + "precision": 2, + "visibleWhen": { + "applyAutoChroma": true + } + } + }, + "autoChromaReferenceFrame": { + "type": "integer", + "default": 0, + "min": -1, + "max": 100000, + "step": 1, + "ui": { + "label": "Reference frame", + "description": "Measure once on this frame. -1 measures every frame, which is about 23 times slower and rarely worth it.", + "widget": "number", + "visibleWhen": { + "applyAutoChroma": true + } + } + }, "applyChromaShift": { "type": "boolean", "default": false, "ui": { - "label": "Adjust Y/C Delay", - "description": "Shift chroma planes to correct Y/C timing delay (common on VHS and analog sources)" + "label": "Correct colour alignment by hand (Y/C delay)", + "description": "Shift the colour back into place yourself. Use this when the automatic measurement finds nothing \u2014 very soft VHS colour is sometimes not measurable.", + "widget": "checkbox", + "visibleWhen": { + "applyAutoChroma": false + } } }, "chromaShiftH": { @@ -67,11 +141,12 @@ "name": "src_left" }, "ui": { - "label": "Horizontal Shift", - "description": "Shift chroma left/right in pixels (negative = left, positive = right)", + "label": "Horizontal shift", + "description": "Negative moves the colour left, positive moves it right, in pixels.", "widget": "slider", "precision": 2, "visibleWhen": { + "applyAutoChroma": false, "applyChromaShift": true } } @@ -86,11 +161,12 @@ "name": "src_top" }, "ui": { - "label": "Vertical Shift", - "description": "Shift chroma up/down in pixels (negative = up, positive = down)", + "label": "Vertical shift", + "description": "Negative moves the colour up, positive moves it down, in pixels.", "widget": "slider", "precision": 2, "visibleWhen": { + "applyAutoChroma": false, "applyChromaShift": true } } @@ -99,83 +175,80 @@ "type": "boolean", "default": false, "ui": { - "label": "Fix Chroma Bleeding", - "description": "Apply chroma bleeding fix (common on VHS)" + "label": "Fix colour bleeding past edges", + "description": "Colour smeared beyond the edge it belongs to, most obvious on strong reds. Common on VHS and other composite captures.", + "widget": "checkbox" } }, - "chromaBleedCx": { - "type": "integer", - "default": 4, - "min": 0, - "max": 16, - "step": 1, - "optional": true, + "chromaBleedStrength": { + "type": "number", + "default": 0.8, + "min": 0.0, + "max": 1.0, + "step": 0.1, "vapoursynth": { - "name": "cx" + "name": "strength" }, "ui": { - "label": "Horizontal Offset", - "description": "Chroma X offset correction", + "label": "Strength", + "description": "How much of the correction to blend in.", "widget": "slider", + "precision": 1, "visibleWhen": { "applyChromaBleedingFix": true } } }, - "chromaBleedCy": { - "type": "integer", - "default": 4, - "min": 0, - "max": 16, - "step": 1, - "optional": true, + "chromaBleedCBlur": { + "type": "number", + "default": 0.7, + "min": 0.0, + "max": 2.0, + "step": 0.1, "vapoursynth": { - "name": "cy" + "name": "thr" }, "ui": { - "label": "Vertical Offset", - "description": "Chroma Y offset correction", + "label": "Colour blur", + "description": "Softens the colour before the correction. Higher hides more bleeding and loses more colour detail.", "widget": "slider", + "precision": 1, "visibleWhen": { "applyChromaBleedingFix": true } } }, - "chromaBleedCBlur": { - "type": "number", - "default": 0.7, - "min": 0.0, - "max": 2.0, - "step": 0.1, - "optional": true, + "chromaBleedCx": { + "type": "integer", + "default": 4, + "min": 0, + "max": 16, + "step": 1, "vapoursynth": { - "name": "thr" + "name": "cx" }, "ui": { - "label": "Chroma Blur", - "description": "Chroma blur strength", + "label": "Horizontal offset", + "description": "Whole-pixel colour offset the fix works from. Leave it alone unless the colour is misaligned as well as bleeding \u2014 the alignment controls above are the better tool for that.", "widget": "slider", - "precision": 1, "visibleWhen": { "applyChromaBleedingFix": true } } }, - "chromaBleedStrength": { - "type": "number", - "default": 0.8, - "min": 0.0, - "max": 1.0, - "step": 0.1, - "optional": true, + "chromaBleedCy": { + "type": "integer", + "default": 4, + "min": 0, + "max": 16, + "step": 1, "vapoursynth": { - "name": "strength" + "name": "cy" }, "ui": { - "label": "Strength", - "description": "Fix strength", + "label": "Vertical offset", + "description": "Whole-pixel colour offset the fix works from, vertically.", "widget": "slider", - "precision": 1, "visibleWhen": { "applyChromaBleedingFix": true } @@ -185,8 +258,9 @@ "type": "boolean", "default": false, "ui": { - "label": "Apply DeCrawl", - "description": "Remove dot crawl artifacts (common on composite video)" + "label": "Remove dot crawl", + "description": "The pattern of dots crawling along sharp colour edges on composite captures. Decided within each frame.", + "widget": "checkbox" } }, "deCrawlYThresh": { @@ -195,13 +269,12 @@ "min": 0, "max": 50, "step": 1, - "optional": true, "vapoursynth": { "name": "ythresh" }, "ui": { - "label": "Luma Threshold", - "description": "Luma threshold for de-crawl", + "label": "Brightness threshold", + "description": "How much brightness variation still counts as dot crawl. Lower catches more.", "widget": "slider", "visibleWhen": { "applyDeCrawl": true @@ -214,13 +287,12 @@ "min": 0, "max": 50, "step": 1, - "optional": true, "vapoursynth": { "name": "cthresh" }, "ui": { - "label": "Chroma Threshold", - "description": "Chroma threshold for de-crawl", + "label": "Colour threshold", + "description": "How much colour variation still counts as dot crawl. Lower catches more.", "widget": "slider", "visibleWhen": { "applyDeCrawl": true @@ -233,63 +305,84 @@ "min": 0, "max": 255, "step": 5, - "optional": true, "vapoursynth": { "name": "maxdiff" }, "ui": { - "label": "Max Difference", - "description": "Maximum difference allowed", + "label": "Maximum change", + "description": "The most any pixel may be altered. Lower is safer on detailed pictures.", "widget": "slider", "visibleWhen": { "applyDeCrawl": true } } }, - "applyVinverse": { + "applyDedot": { "type": "boolean", "default": false, "ui": { - "label": "Apply Vinverse", - "description": "Remove chroma combing artifacts" + "label": "Remove dot crawl across frames (DeDot)", + "description": "A second removal that compares neighbouring frames instead of working inside one, and skips anything that is moving. It reaches crawl patterns the one above leaves alone, so the two are worth combining. 8-bit only, so a deeper source is converted down for this step and restored after.", + "widget": "checkbox" } }, - "vinverseSstr": { - "type": "number", - "default": 2.7, - "min": 0.0, - "max": 10.0, - "step": 0.1, - "optional": true, - "vapoursynth": { - "name": "sstr" - }, + "dedotLuma2d": { + "type": "integer", + "default": 20, + "min": 0, + "max": 510, + "step": 1, "ui": { - "label": "Strength", - "description": "Spatial strength", + "label": "Brightness (within a frame)", + "description": "How different a pixel must be from its neighbours to count as dot crawl.", "widget": "slider", - "precision": 1, "visibleWhen": { - "applyVinverse": true + "applyDedot": true } } }, - "vinverseAmnt": { + "dedotLumaT": { "type": "integer", - "default": 255, + "default": 20, "min": 0, "max": 255, - "step": 5, - "optional": true, - "vapoursynth": { - "name": "amnt" - }, + "step": 1, "ui": { - "label": "Amount", - "description": "Blend amount (0-255)", + "label": "Brightness (between frames)", + "description": "How much a pixel may change between frames and still be treated as dot crawl rather than motion.", "widget": "slider", "visibleWhen": { - "applyVinverse": true + "applyDedot": true + } + } + }, + "dedotChromaT1": { + "type": "integer", + "default": 15, + "min": 0, + "max": 255, + "step": 1, + "ui": { + "label": "Colour threshold", + "description": "How strongly colour is corrected.", + "widget": "slider", + "visibleWhen": { + "applyDedot": true + } + } + }, + "dedotChromaT2": { + "type": "integer", + "default": 5, + "min": 0, + "max": 255, + "step": 1, + "ui": { + "label": "Colour motion limit", + "description": "Raise towards 255 to leave colour alone entirely.", + "widget": "slider", + "visibleWhen": { + "applyDedot": true } } }, @@ -297,8 +390,9 @@ "type": "boolean", "default": false, "ui": { - "label": "Remove Rainbowing", - "description": "Shimmering colour bands over fine detail on composite captures. The companion to dot crawl removal, which targets the dots along colour edges" + "label": "Remove rainbow shimmer", + "description": "Shimmering colour bands over fine detail on composite captures \u2014 the companion to dot crawl, which sits along edges instead. Decided within each frame.", + "widget": "checkbox" } }, "deRainbowCThresh": { @@ -308,8 +402,8 @@ "max": 255, "step": 1, "ui": { - "label": "Rainbow Chroma Threshold", - "description": "How different chroma must be before it is treated as rainbowing. Lower catches more", + "label": "Colour threshold", + "description": "How different colour must be before it is treated as rainbowing. Lower catches more.", "widget": "slider", "visibleWhen": { "applyDeRainbow": true @@ -323,8 +417,8 @@ "max": 255, "step": 1, "ui": { - "label": "Rainbow Luma Threshold", - "description": "Areas moving more than this are left alone, which protects real motion", + "label": "Motion threshold", + "description": "Areas whose brightness moves more than this are left alone, which protects real motion.", "widget": "slider", "visibleWhen": { "applyDeRainbow": true @@ -335,8 +429,9 @@ "type": "boolean", "default": false, "ui": { - "label": "Remove Shimmering Rainbows", - "description": "Compares across frames rather than within one, so it catches rainbowing that shimmers as the picture moves. Works alongside the other rainbow removal, not instead of it" + "label": "Remove rainbow shimmer across frames (Bifrost)", + "description": "Compares across frames rather than within one, so it catches rainbowing that shimmers as the picture moves. Works alongside the removal above, not instead of it. 8-bit only, so a deeper source is converted down for this step and restored after.", + "widget": "checkbox" } }, "bifrostLumaThresh": { @@ -346,8 +441,8 @@ "max": 64.0, "step": 1.0, "ui": { - "label": "Motion Threshold", - "description": "Areas changing more than this between frames are treated as movement and left alone", + "label": "Motion threshold", + "description": "Areas changing more than this between frames are treated as movement and left alone.", "widget": "slider", "precision": 1, "visibleWhen": { @@ -363,243 +458,170 @@ "step": 1, "ui": { "label": "Caution", - "description": "How much agreement is needed before a pixel is changed. Higher is safer and removes less", + "description": "How much agreement is needed before a pixel is changed. Higher is safer and removes less.", "widget": "slider", "visibleWhen": { "applyBifrost": true } } }, - "applyDedot": { + "applyVinverse": { "type": "boolean", "default": false, "ui": { - "label": "Remove dot crawl (DeDot)", - "description": "Removes the crawling dotted pattern along sharp colour edges on composite captures. Works on brightness and colour together, and skips anything that is moving.", + "label": "Remove chroma combing", + "description": "Leftover comb teeth in the colour planes, usually from a source that was deinterlaced or field-blended badly before it reached you.", "widget": "checkbox" } }, - "dedotLuma2d": { - "type": "integer", - "default": 20, - "min": 0, - "max": 510, - "step": 1, - "visibleWhen": { - "applyDedot": [ - true - ] - }, - "ui": { - "label": "Brightness (within a frame)", - "description": "How different a pixel must be from its neighbours to count as dot crawl.", - "widget": "slider" - } - }, - "dedotLumaT": { - "type": "integer", - "default": 20, - "min": 0, - "max": 255, - "step": 1, - "visibleWhen": { - "applyDedot": [ - true - ] - }, - "ui": { - "label": "Brightness (between frames)", - "description": "How much a pixel may change between frames and still be treated as dot crawl rather than motion.", - "widget": "slider" - } - }, - "dedotChromaT1": { - "type": "integer", - "default": 15, - "min": 0, - "max": 255, - "step": 1, - "visibleWhen": { - "applyDedot": [ - true - ] + "vinverseSstr": { + "type": "number", + "default": 2.7, + "min": 0.0, + "max": 10.0, + "step": 0.1, + "vapoursynth": { + "name": "sstr" }, "ui": { - "label": "Colour threshold", - "description": "How strongly colour is corrected.", - "widget": "slider" + "label": "Strength", + "description": "How hard to work on the combing. Higher softens more of the picture with it.", + "widget": "slider", + "precision": 1, + "visibleWhen": { + "applyVinverse": true + } } }, - "dedotChromaT2": { + "vinverseAmnt": { "type": "integer", - "default": 5, + "default": 255, "min": 0, "max": 255, - "step": 1, - "visibleWhen": { - "applyDedot": [ - true - ] - }, - "ui": { - "label": "Colour motion limit", - "description": "Raise towards 255 to leave colour alone entirely.", - "widget": "slider" - } - }, - "applyAutoChroma": { - "type": "boolean", - "default": false, - "ui": { - "label": "Detect colour alignment automatically", - "description": "Measures how far the colour has slipped from the picture and corrects it, instead of you guessing on the sliders below. If it cannot measure the source reliably it leaves it alone rather than guessing.", - "widget": "checkbox" - } - }, - "autoChromaMaxShift": { - "type": "integer", - "default": 2, - "min": 1, - "max": 8, - "step": 1, - "visibleWhen": { - "applyAutoChroma": [ - true - ] - }, - "ui": { - "label": "Search range", - "description": "The largest misalignment to look for, in pixels.", - "widget": "slider" - } - }, - "autoChromaAccuracy": { - "type": "number", - "default": 0.25, - "min": 0.05, - "max": 1.0, - "step": 0.05, - "visibleWhen": { - "applyAutoChroma": [ - true - ] + "step": 5, + "vapoursynth": { + "name": "amnt" }, "ui": { - "label": "Precision", - "description": "Smaller finds sub-pixel shifts, at some cost.", + "label": "Maximum change", + "description": "The most any pixel may be altered. 255 places no limit.", "widget": "slider", - "precision": 2 - } - }, - "autoChromaReferenceFrame": { - "type": "integer", - "default": 0, - "min": -1, - "max": 100000, - "step": 1, - "visibleWhen": { - "applyAutoChroma": [ - true - ] - }, - "ui": { - "label": "Reference frame", - "description": "Measure once on this frame. -1 measures every frame, which is about 23 times slower and rarely worth it.", - "widget": "number" + "visibleWhen": { + "applyVinverse": true + } } } }, "ui": { "sections": [ { - "title": "Automatic alignment", + "title": "Colour alignment", "parameters": [ "applyAutoChroma", - "autoChromaMaxShift" + "applyChromaShift", + "chromaShiftH", + "chromaShiftV" ], "expanded": true }, { - "title": "Y/C Delay (Chroma Shift)", + "title": "Automatic alignment tuning", "parameters": [ - "applyChromaShift", - "chromaShiftH", - "chromaShiftV" + "autoChromaMaxShift", + "autoChromaAccuracy", + "autoChromaReferenceFrame" ], - "expanded": true + "expanded": false, + "advancedOnly": true }, { - "title": "Chroma Bleeding Fix", + "title": "Colour bleeding", "parameters": [ "applyChromaBleedingFix", - "chromaBleedCx", - "chromaBleedCy", - "chromaBleedCBlur", - "chromaBleedStrength" + "chromaBleedStrength", + "chromaBleedCBlur" ], "expanded": true }, { - "title": "DeCrawl (Dot Crawl)", + "title": "Colour bleeding tuning", + "parameters": [ + "chromaBleedCx", + "chromaBleedCy" + ], + "expanded": false, + "advancedOnly": true + }, + { + "title": "Dot crawl", "parameters": [ "applyDeCrawl", - "deCrawlYThresh", - "deCrawlCThresh", - "deCrawlMaxDiff", - "applyDeRainbow", - "deRainbowCThresh", - "deRainbowYThresh", - "applyBifrost", - "bifrostLumaThresh", - "bifrostVariation" + "applyDedot" ], "expanded": true }, { - "title": "Vinverse (Combing Fix)", + "title": "Dot crawl tuning", "parameters": [ - "applyVinverse", - "vinverseSstr", - "vinverseAmnt" + "deCrawlYThresh", + "deCrawlCThresh", + "deCrawlMaxDiff" ], - "expanded": true + "expanded": false, + "advancedOnly": true }, { - "title": "Dot crawl (DeDot)", + "title": "Dot crawl (across frames) tuning", "parameters": [ - "applyDedot", "dedotLuma2d", - "dedotLumaT" + "dedotLumaT", + "dedotChromaT1", + "dedotChromaT2" + ], + "expanded": false, + "advancedOnly": true + }, + { + "title": "Rainbowing", + "parameters": [ + "applyDeRainbow", + "applyBifrost" ], "expanded": true }, { - "title": "Dot crawl tuning", + "title": "Rainbow tuning", "parameters": [ - "dedotChromaT1", - "dedotChromaT2" + "deRainbowCThresh", + "deRainbowYThresh" ], "expanded": false, - "advancedOnly": true, - "visibleWhen": { - "applyDedot": [ - true - ] - } + "advancedOnly": true }, { - "title": "Automatic alignment tuning", + "title": "Rainbow (across frames) tuning", "parameters": [ - "autoChromaAccuracy", - "autoChromaReferenceFrame" + "bifrostLumaThresh", + "bifrostVariation" ], "expanded": false, - "advancedOnly": true, - "visibleWhen": { - "applyAutoChroma": [ - true - ] - } + "advancedOnly": true + }, + { + "title": "Chroma combing", + "parameters": [ + "applyVinverse", + "vinverseSstr" + ], + "expanded": true + }, + { + "title": "Chroma combing tuning", + "parameters": [ + "vinverseAmnt" + ], + "expanded": false, + "advancedOnly": true } ] }, diff --git a/app/assets/filters/core/color_correction.json b/app/assets/filters/core/color_correction.json index 4561440..20b2cd6 100644 --- a/app/assets/filters/core/color_correction.json +++ b/app/assets/filters/core/color_correction.json @@ -3,8 +3,8 @@ "id": "color_correction", "version": "1.1.0", "name": "Color Correction", - "description": "Adjust brightness, contrast, saturation and levels", - "longDescription": "Adjusts brightness, contrast, saturation and the black and white levels.\n\nUse it to rescue washed-out or crushed transfers, to fix a capture made at the wrong levels (limited 16-235 read as full 0-255, or the reverse), or to lift colour from faded film. Small moves go a long way \u2014 check the before/after preview rather than judging by numbers.", + "description": "Adjust brightness, contrast, saturation, levels and white balance", + "longDescription": "Adjusts brightness, contrast, saturation, the black and white levels, and the colour cast.\n\nUse it to rescue washed-out or crushed transfers, to fix a capture made at the wrong levels (limited 16-235 read as full 0-255, or the reverse), or to lift colour from faded film. Small moves go a long way \u2014 check the before/after preview rather than judging by numbers.\n\nLevels and white balance can each be set **automatically** or **by hand**, and the two sit together in the same group. The automatic pass measures the picture and runs first; anything you then set by hand is applied on top of the corrected picture, not the original.", "category": "color", "icon": "tune", "order": 7, @@ -18,25 +18,33 @@ }, "methods": [ { - "id": "tweak", - "name": "Tweak", - "description": "Basic color adjustments - brightness, contrast, saturation, hue", - "function": "haf.Tweak", + "id": "color_correction", + "name": "Color Correction", + "description": "Brightness and colour, levels, white balance and shadow detail, each set separately below", + "function": "custom", "parameters": [ "brightness", "contrast", "saturation", - "hue" - ] - }, - { - "id": "white_balance", - "name": "White Balance", - "description": "Temperature and tint, applied as a chroma offset", - "function": "core.std.Expr", - "parameters": [ + "hue", + "coring", + "applyAutoLevels", + "autoLevelsStrength", + "autoLevelsBlack", + "autoLevelsWhite", + "applyLevels", + "inputLow", + "inputHigh", + "outputLow", + "outputHigh", + "gamma", + "smoothLevels", + "applyAutoWhiteBalance", + "autoWhiteBalanceStrength", "temperature", - "tint" + "tint", + "applyShadowDetail", + "shadowSigma" ] } ], @@ -54,13 +62,12 @@ "min": -50.0, "max": 50.0, "step": 1.0, - "optional": true, "vapoursynth": { "name": "bright" }, "ui": { "label": "Brightness", - "description": "Brightness adjustment (-50 to +50)", + "description": "Raises or lowers the whole picture. 0 leaves it alone.", "widget": "slider", "precision": 0 } @@ -71,13 +78,12 @@ "min": 0.5, "max": 2.0, "step": 0.05, - "optional": true, "vapoursynth": { "name": "cont" }, "ui": { "label": "Contrast", - "description": "Contrast multiplier (1.0 = no change)", + "description": "Multiplier. 1.00 leaves it alone.", "widget": "slider", "precision": 2 } @@ -88,13 +94,12 @@ "min": 0.0, "max": 3.0, "step": 0.05, - "optional": true, "vapoursynth": { "name": "sat" }, "ui": { "label": "Saturation", - "description": "Saturation multiplier (1.0 = no change)", + "description": "Colour intensity. 1.00 leaves it alone, 0 is black and white.", "widget": "slider", "precision": 2 } @@ -105,13 +110,12 @@ "min": -180.0, "max": 180.0, "step": 5.0, - "optional": true, "vapoursynth": { "name": "hue" }, "ui": { "label": "Hue", - "description": "Hue rotation in degrees", + "description": "Rotates every colour, in degrees. For a colour cast use white balance below instead.", "widget": "slider", "precision": 0 } @@ -119,21 +123,94 @@ "coring": { "type": "boolean", "default": false, - "optional": true, "vapoursynth": { "name": "coring" }, "ui": { - "label": "Coring", - "description": "Clamp output to TV range (16-235)" + "label": "Clamp to broadcast range (16-235)", + "description": "Keeps the result inside the range broadcast equipment expects, at the cost of clipping anything beyond it." + } + }, + "applyAutoLevels": { + "type": "boolean", + "default": false, + "ui": { + "label": "Set levels automatically", + "description": "Measures the picture and stretches it so its darkest and brightest parts reach the target black and white. The usual fix for a washed-out capture, and it replaces the input and output sliders below.", + "widget": "checkbox" + } + }, + "autoLevelsStrength": { + "type": "number", + "default": 1.0, + "min": 0.0, + "max": 1.0, + "step": 0.05, + "ui": { + "label": "Strength", + "description": "Lower this if the correction moves too much between shots.", + "widget": "slider", + "precision": 2, + "visibleWhen": { + "applyAutoLevels": true + } + } + }, + "autoLevelsBlack": { + "type": "integer", + "default": 16, + "min": 0, + "max": 64, + "step": 1, + "ui": { + "label": "Target black", + "description": "Where the darkest part of the picture should land. 16 is broadcast black.", + "widget": "slider", + "visibleWhen": { + "applyAutoLevels": true + } + } + }, + "autoLevelsWhite": { + "type": "integer", + "default": 235, + "min": 192, + "max": 255, + "step": 1, + "ui": { + "label": "Target white", + "description": "Where the brightest part should land. 235 is broadcast white.", + "widget": "slider", + "visibleWhen": { + "applyAutoLevels": true + } } }, "applyLevels": { "type": "boolean", "default": false, "ui": { - "label": "Enable Levels", - "description": "Enable input/output level adjustments" + "label": "Set levels by hand", + "description": "Map the input range onto the output range yourself, and adjust gamma. With automatic levels on, only gamma is left to set \u2014 the automatic pass has already placed black and white." + } + }, + "gamma": { + "type": "number", + "default": 1.0, + "min": 0.1, + "max": 4.0, + "step": 0.1, + "vapoursynth": { + "name": "gamma" + }, + "ui": { + "label": "Gamma", + "description": "Brightens or darkens the midtones without moving black or white. Above 1 brightens. Automatic levels does not touch this.", + "widget": "slider", + "precision": 1, + "visibleWhen": { + "applyLevels": true + } } }, "inputLow": { @@ -142,15 +219,15 @@ "min": 0, "max": 255, "step": 1, - "optional": true, "vapoursynth": { "name": "input_low" }, "ui": { - "label": "Input Black", - "description": "Input black level", + "label": "Input black", + "description": "The level in the source that should become black. Raise it to deepen a washed-out picture.", "widget": "slider", "visibleWhen": { + "applyAutoLevels": false, "applyLevels": true } } @@ -161,15 +238,15 @@ "min": 0, "max": 255, "step": 1, - "optional": true, "vapoursynth": { "name": "input_high" }, "ui": { - "label": "Input White", - "description": "Input white level", + "label": "Input white", + "description": "The level in the source that should become white.", "widget": "slider", "visibleWhen": { + "applyAutoLevels": false, "applyLevels": true } } @@ -180,15 +257,15 @@ "min": 0, "max": 255, "step": 1, - "optional": true, "vapoursynth": { "name": "output_low" }, "ui": { - "label": "Output Black", - "description": "Output black level", + "label": "Output black", + "description": "What black becomes on the way out. 16 for broadcast-range output.", "widget": "slider", "visibleWhen": { + "applyAutoLevels": false, "applyLevels": true } } @@ -199,36 +276,52 @@ "min": 0, "max": 255, "step": 1, - "optional": true, "vapoursynth": { "name": "output_high" }, "ui": { - "label": "Output White", - "description": "Output white level", + "label": "Output white", + "description": "What white becomes on the way out. 235 for broadcast-range output.", "widget": "slider", "visibleWhen": { + "applyAutoLevels": false, "applyLevels": true } } }, - "gamma": { + "smoothLevels": { + "type": "boolean", + "default": false, + "ui": { + "label": "Smooth the adjustment", + "description": "Dithers and limits the adjustment as it goes, so stretching a narrow range does not leave visible steps in skies and fades. Slower, and it cannot lift the black point and change gamma at the same time.", + "visibleWhen": { + "applyLevels": true + } + } + }, + "applyAutoWhiteBalance": { + "type": "boolean", + "default": false, + "ui": { + "label": "Set white balance automatically", + "description": "Assumes the scene should average out to neutral grey and shifts the colour until it does. Runs before the sliders below, so they offset the corrected picture rather than the original.", + "widget": "checkbox" + } + }, + "autoWhiteBalanceStrength": { "type": "number", "default": 1.0, - "min": 0.1, - "max": 4.0, - "step": 0.1, - "optional": true, - "vapoursynth": { - "name": "gamma" - }, + "min": 0.0, + "max": 1.0, + "step": 0.05, "ui": { - "label": "Gamma", - "description": "Gamma correction", + "label": "Strength", + "description": "Lower this to keep some of the original cast \u2014 useful when the cast is meant to be there, like firelight.", "widget": "slider", - "precision": 1, + "precision": 2, "visibleWhen": { - "applyLevels": true + "applyAutoWhiteBalance": true } } }, @@ -238,13 +331,12 @@ "min": -100.0, "max": 100.0, "step": 1.0, - "optional": true, "vapoursynth": { "name": "temperature" }, "ui": { "label": "Temperature", - "description": "Warm/cool white balance. Negative is cooler (bluer), positive warmer (more amber). Corrects a colour cast rather than styling the image", + "description": "Warm/cool. Negative is cooler (bluer), positive warmer (more amber).", "widget": "slider", "precision": 0 } @@ -255,34 +347,22 @@ "min": -100.0, "max": 100.0, "step": 1.0, - "optional": true, "vapoursynth": { "name": "tint" }, "ui": { "label": "Tint", - "description": "Green/magenta white balance. Negative shifts toward green, positive toward magenta \u2014 the axis VHS and telecine casts usually sit on", + "description": "Green/magenta. Negative shifts toward green, positive toward magenta \u2014 the axis VHS and telecine casts usually sit on.", "widget": "slider", "precision": 0 } }, - "smoothLevels": { - "type": "boolean", - "default": false, - "ui": { - "label": "Smooth Levels", - "description": "Dithers and limits the adjustment as it goes, so stretching a narrow range does not leave visible steps in skies and fades. Slower, and it cannot lift the black point and change gamma at the same time", - "visibleWhen": { - "applyLevels": true - } - } - }, "applyShadowDetail": { "type": "boolean", "default": false, "ui": { - "label": "Lift Shadow Detail", - "description": "Opens up detail hidden in dark areas of underexposed footage, by comparing each part of the picture to its surroundings rather than raising the black level. Brightness only \u2014 colour is untouched" + "label": "Lift shadow detail", + "description": "Opens up detail hidden in dark areas of underexposed footage, by comparing each part of the picture to its surroundings rather than raising the black level. Brightness only \u2014 colour is untouched." } }, "shadowSigma": { @@ -292,151 +372,77 @@ "max": 500.0, "step": 10.0, "ui": { - "label": "Area Size", - "description": "How wide a neighbourhood each pixel is judged against. Larger opens up broad shadows; smaller brings out local texture", + "label": "Area size", + "description": "How wide a neighbourhood each pixel is judged against. Larger opens up broad shadows; smaller brings out local texture.", "widget": "slider", "precision": 0, "visibleWhen": { "applyShadowDetail": true } } - }, - "applyAutoLevels": { - "type": "boolean", - "default": false, - "ui": { - "label": "Auto levels", - "description": "Stretch the picture so its darkest and brightest parts reach the target black and white. The usual fix for a washed-out capture.", - "widget": "checkbox" - } - }, - "autoLevelsBlack": { - "type": "integer", - "default": 16, - "min": 0, - "max": 64, - "step": 1, - "visibleWhen": { - "applyAutoLevels": [ - true - ] - }, - "ui": { - "label": "Target black", - "description": "Where the darkest part of the picture should land. 16 is broadcast black.", - "widget": "slider" - } - }, - "autoLevelsWhite": { - "type": "integer", - "default": 235, - "min": 192, - "max": 255, - "step": 1, - "visibleWhen": { - "applyAutoLevels": [ - true - ] - }, - "ui": { - "label": "Target white", - "description": "Where the brightest part should land. 235 is broadcast white.", - "widget": "slider" - } - }, - "autoLevelsStrength": { - "type": "number", - "default": 1.0, - "min": 0.0, - "max": 1.0, - "step": 0.05, - "visibleWhen": { - "applyAutoLevels": [ - true - ] - }, - "ui": { - "label": "Strength", - "description": "Lower this if the correction moves too much between shots.", - "widget": "slider", - "precision": 2 - } - }, - "applyAutoWhiteBalance": { - "type": "boolean", - "default": false, - "ui": { - "label": "Auto white balance", - "description": "Remove an overall colour cast by assuming the scene should average out to neutral grey. The automatic counterpart to the temperature and tint sliders.", - "widget": "checkbox" - } - }, - "autoWhiteBalanceStrength": { - "type": "number", - "default": 1.0, - "min": 0.0, - "max": 1.0, - "step": 0.05, - "visibleWhen": { - "applyAutoWhiteBalance": [ - true - ] - }, - "ui": { - "label": "Strength", - "description": "Lower this to keep some of the original cast \u2014 useful when the cast is meant to be there, like firelight.", - "widget": "slider", - "precision": 2 - } } }, "ui": { "sections": [ { - "title": "Automatic", + "title": "Brightness and colour", "parameters": [ - "applyAutoLevels", - "autoLevelsBlack", - "autoLevelsWhite", - "autoLevelsStrength", - "applyAutoWhiteBalance", - "autoWhiteBalanceStrength" + "brightness", + "contrast", + "saturation", + "hue" ], "expanded": true }, { - "title": "Basic Adjustments", + "title": "Brightness and colour tuning", "parameters": [ - "brightness", - "contrast", - "saturation", - "hue", - "coring", - "applyShadowDetail", - "shadowSigma" + "coring" ], - "expanded": true + "expanded": false, + "advancedOnly": true }, { "title": "Levels", "parameters": [ + "applyAutoLevels", + "autoLevelsStrength", + "autoLevelsBlack", + "autoLevelsWhite", "applyLevels", "inputLow", "inputHigh", "outputLow", "outputHigh", - "gamma", + "gamma" + ], + "expanded": true + }, + { + "title": "Levels tuning", + "parameters": [ "smoothLevels" ], - "expanded": false + "expanded": false, + "advancedOnly": true }, { - "title": "White Balance", + "title": "White balance", "parameters": [ + "applyAutoWhiteBalance", + "autoWhiteBalanceStrength", "temperature", "tint" ], "expanded": true + }, + { + "title": "Shadow detail", + "parameters": [ + "applyShadowDetail", + "shadowSigma" + ], + "expanded": true } ] }, @@ -444,6 +450,6 @@ "imports": [ "import havsfunc as haf" ], - "generate": "method" + "generate": "custom" } } diff --git a/app/assets/filters/core/crop_resize.json b/app/assets/filters/core/crop_resize.json index 9f8202f..355aa1e 100644 --- a/app/assets/filters/core/crop_resize.json +++ b/app/assets/filters/core/crop_resize.json @@ -15,47 +15,42 @@ }, "methods": [ { - "id": "standard", - "name": "Resize", - "description": "Resample to a target size with a selectable kernel", - "function": "core.resize", + "id": "crop_resize", + "name": "Crop & Resize", + "description": "Cropping, resizing, aspect ratio and edge-directed upscaling, each switched on separately below", + "function": "custom", "parameters": [ + "cropEnabled", + "cropLeft", + "cropRight", + "cropTop", + "cropBottom", + "resizeEnabled", "targetWidth", "targetHeight", "kernel", "bicubicB", "bicubicC", "lanczosTaps", + "maintainAspect", + "pixelAspect", + "customSar", "displayAspect", - "customSar" - ] - }, - { - "id": "nnedi3_2x", - "name": "NNEDI3 Upscale", - "description": "Doubles size with far cleaner edges than a resampler. Best choice for upscaling. Slow", - "function": "core.znedi3.nnedi3", - "parameters": [ + "padToAspect", + "useIntegerUpscale", + "upscaleMethod", + "upscaleFactor", "upscaleNsize", "upscaleNeurons", "upscaleQual", "upscaleEtype", - "upscalePscrn" - ] - }, - { - "id": "eedi3_2x", - "name": "EEDI3 Upscale", - "description": "Doubling guided by NNEDI3. Marginally better on hard diagonals, several times slower", - "function": "core.eedi3m.EEDI3", - "parameters": [ + "upscalePscrn", "upscaleAlpha", "upscaleBeta", "upscaleGamma", "upscaleNrad", "upscaleMdis" - ], - "advancedOnly": true + ] } ], "parameters": { diff --git a/app/assets/filters/core/deblock.json b/app/assets/filters/core/deblock.json index f3f6321..ff2a6ee 100644 --- a/app/assets/filters/core/deblock.json +++ b/app/assets/filters/core/deblock.json @@ -4,7 +4,7 @@ "version": "1.0.0", "name": "Deblock", "description": "Remove blocking artifacts from compressed video", - "longDescription": "Smooths the square 8x8 block edges that low-bitrate compression leaves behind, without flattening the detail inside each block.\n\nUse it on heavily compressed MPEG sources — old DVDs, VCD, downloaded or streamed footage — especially in dark or flat areas where blocking shows most. Run it before sharpening, which would otherwise make the block edges crisper rather than softer.", + "longDescription": "Smooths the square 8x8 block edges that low-bitrate compression leaves behind, without flattening the detail inside each block.\n\nUse it on heavily compressed MPEG sources \u2014 old DVDs, VCD, downloaded or streamed footage \u2014 especially in dark or flat areas where blocking shows most. Run it before sharpening, which would otherwise make the block edges crisper rather than softer.", "category": "cleanup", "icon": "grid_off", "order": 3, @@ -84,7 +84,13 @@ "ui": { "label": "Quant 1", "description": "Quantization level for luma deblocking (20-28 for VHS)", - "widget": "slider" + "widget": "slider", + "visibleWhen": { + "method": [ + "deblock_qed", + "deblock" + ] + } } }, "quant2": { @@ -208,7 +214,7 @@ "ui": { "sections": [ { - "title": "Settings", + "title": "Deblock settings", "parameters": [ "quant1", "quant2", diff --git a/app/assets/filters/core/deflicker.json b/app/assets/filters/core/deflicker.json index 6517093..e4e4b9e 100644 --- a/app/assets/filters/core/deflicker.json +++ b/app/assets/filters/core/deflicker.json @@ -55,16 +55,16 @@ "min": 0.0, "max": 1.0, "step": 0.05, - "visibleWhen": { - "method": [ - "global" - ] - }, "ui": { "label": "Strength", "description": "Lower this if the correction is over-eager on a shot with a genuine brightness change, like a lamp being switched on.", "widget": "slider", - "precision": 2 + "precision": 2, + "visibleWhen": { + "method": [ + "global" + ] + } } }, "window": { @@ -73,15 +73,15 @@ "min": 1, "max": 12, "step": 1, - "visibleWhen": { - "method": [ - "global" - ] - }, "ui": { "label": "Frames compared", "description": "How many frames either side are averaged to decide what the exposure should have been.", - "widget": "slider" + "widget": "slider", + "visibleWhen": { + "method": [ + "global" + ] + } } }, "localStrength": { @@ -90,53 +90,41 @@ "min": 1, "max": 3, "step": 1, - "visibleWhen": { - "method": [ - "local" - ] - }, "ui": { "label": "Strength", "description": "Higher compares against more distant frames.", - "widget": "slider" + "widget": "slider", + "visibleWhen": { + "method": [ + "local" + ] + } } }, "aggressive": { "type": "boolean", "default": false, - "visibleWhen": { - "method": [ - "local" - ] - }, "ui": { "label": "Aggressive", "description": "Corrects harder, at more risk of smearing genuine movement.", - "widget": "checkbox" + "widget": "checkbox", + "visibleWhen": { + "method": [ + "local" + ] + } } } }, "ui": { "sections": [ - { - "title": "Method", - "parameters": [ - "method" - ], - "expanded": true - }, { "title": "Whole frame", "parameters": [ "strength", "window" ], - "expanded": true, - "visibleWhen": { - "method": [ - "global" - ] - } + "expanded": true }, { "title": "Across the frame", @@ -144,12 +132,7 @@ "localStrength", "aggressive" ], - "expanded": true, - "visibleWhen": { - "method": [ - "local" - ] - } + "expanded": true } ] } diff --git a/app/assets/filters/core/dehalo.json b/app/assets/filters/core/dehalo.json index 805b495..12ac5ef 100644 --- a/app/assets/filters/core/dehalo.json +++ b/app/assets/filters/core/dehalo.json @@ -4,7 +4,7 @@ "version": "1.1.0", "name": "Dehalo", "description": "Remove halo, ringing and ghosting around edges", - "longDescription": "Removes the bright outline that sits alongside high-contrast edges — the ringing left by over-sharpening, upscaling or heavy compression.\n\nUse it when edges look traced with a light pen: common on VHS run through a sharpening time-base corrector, and on upscaled or hard-compressed material. Apply it after deinterlacing and denoising. Too much strength eats the fine detail right next to the edge.\n\nThe two Vinverse methods target a different artifact: the vertical comb or ghost residue a deinterlacer leaves behind. They blur vertically, so only run them on progressive frames — after the deinterlace pass, never instead of it.", + "longDescription": "Removes the bright outline that sits alongside high-contrast edges \u2014 the ringing left by over-sharpening, upscaling or heavy compression.\n\nUse it when edges look traced with a light pen: common on VHS run through a sharpening time-base corrector, and on upscaled or hard-compressed material. Apply it after deinterlacing and denoising. Too much strength eats the fine detail right next to the edge.\n\nThe two Vinverse methods target a different artifact: the vertical comb or ghost residue a deinterlacer leaves behind. They blur vertically, so only run them on progressive frames \u2014 after the deinterlace pass, never instead of it.", "category": "cleanup", "icon": "blur_off", "order": 4, @@ -58,7 +58,7 @@ { "id": "fine_dehalo2", "name": "Fine Dehalo 2", - "description": "Removes the ringing left on sharp edges. A follow-up pass — run it after Fine Dehalo, not instead of it", + "description": "Removes the ringing left on sharp edges. A follow-up pass \u2014 run it after Fine Dehalo, not instead of it", "function": "haf.FineDehalo2", "parameters": [], "advancedOnly": true @@ -76,7 +76,7 @@ { "id": "edge_cleaner", "name": "Edge Cleaner", - "description": "Cleans edge noise and weak halos by warping edges (aWarpSharp2). Niche — for line art rather than live action", + "description": "Cleans edge noise and weak halos by warping edges (aWarpSharp2). Niche \u2014 for line art rather than live action", "function": "haf.EdgeCleaner", "parameters": [ "edgeStrength", @@ -114,7 +114,7 @@ { "id": "hq_deringmod", "name": "HQDeringmod", - "description": "Removes ringing — the overshoot immediately beside an edge — while a mask protects the edge itself. A narrower target than dehalo, for over-sharpened or heavily compressed sources", + "description": "Removes ringing \u2014 the overshoot immediately beside an edge \u2014 while a mask protects the edge itself. A narrower target than dehalo, for over-sharpened or heavily compressed sources", "function": "haf.HQDeringmod", "parameters": [ "deringMrad", @@ -208,7 +208,7 @@ }, "ui": { "label": "Dark Halo Strength", - "description": "Strength of dark halo removal. Above 1.0 overshoots — it pushes past the original pixel rather than blending back to it", + "description": "Strength of dark halo removal. Above 1.0 overshoots \u2014 it pushes past the original pixel rather than blending back to it", "widget": "slider", "precision": 2, "visibleWhen": { @@ -231,7 +231,7 @@ }, "ui": { "label": "Bright Halo Strength", - "description": "Strength of bright halo removal. Above 1.0 overshoots — it pushes past the original pixel rather than blending back to it", + "description": "Strength of bright halo removal. Above 1.0 overshoots \u2014 it pushes past the original pixel rather than blending back to it", "widget": "slider", "precision": 2, "visibleWhen": { @@ -360,7 +360,7 @@ }, "ui": { "label": "Limit Low", - "description": "Below this edge strength, dehaloing is fully limited — protects faint detail", + "description": "Below this edge strength, dehaloing is fully limited \u2014 protects faint detail", "widget": "slider", "visibleWhen": { "method": [ @@ -775,7 +775,7 @@ "expanded": true }, { - "title": "Advanced", + "title": "Dehalo tuning", "parameters": [ "lowSens", "highSens", diff --git a/app/assets/filters/core/deinterlace.json b/app/assets/filters/core/deinterlace.json index 9f3fa6f..5dbc463 100644 --- a/app/assets/filters/core/deinterlace.json +++ b/app/assets/filters/core/deinterlace.json @@ -1951,15 +1951,15 @@ "bwdifEdeint": { "type": "boolean", "default": false, - "visibleWhen": { - "method": [ - "bwdif" - ] - }, "ui": { "label": "High-quality interpolation", "description": "Use the NNEDI3 neural interpolator instead of the built-in one. Sharper edges, noticeably slower \u2014 still far quicker than QTGMC.", - "widget": "checkbox" + "widget": "checkbox", + "visibleWhen": { + "method": [ + "bwdif" + ] + } } } }, @@ -2263,12 +2263,7 @@ "parameters": [ "bwdifEdeint" ], - "expanded": true, - "visibleWhen": { - "method": [ - "bwdif" - ] - } + "expanded": true } ] }, diff --git a/app/assets/filters/core/edge_repair.json b/app/assets/filters/core/edge_repair.json index 98c0d0d..2515f23 100644 --- a/app/assets/filters/core/edge_repair.json +++ b/app/assets/filters/core/edge_repair.json @@ -105,7 +105,7 @@ "expanded": true }, { - "title": "Advanced", + "title": "Edge tuning", "parameters": [ "mode" ], diff --git a/app/assets/filters/core/frame_rate.json b/app/assets/filters/core/frame_rate.json index 10bd73a..d8cf3af 100644 --- a/app/assets/filters/core/frame_rate.json +++ b/app/assets/filters/core/frame_rate.json @@ -69,15 +69,15 @@ "min": 4, "max": 32, "step": 4, - "visibleWhen": { - "method": [ - "flowFps" - ] - }, "ui": { "label": "Block size", "description": "Motion is estimated in blocks of this size. Larger is faster and coarser.", - "widget": "slider" + "widget": "slider", + "visibleWhen": { + "method": [ + "flowFps" + ] + } } }, "overlap": { @@ -86,15 +86,15 @@ "min": 0, "max": 16, "step": 2, - "visibleWhen": { - "method": [ - "flowFps" - ] - }, "ui": { "label": "Block overlap", "description": "How much neighbouring blocks overlap. More overlap hides block edges at some cost in speed.", - "widget": "slider" + "widget": "slider", + "visibleWhen": { + "method": [ + "flowFps" + ] + } } } }, @@ -115,12 +115,7 @@ "overlap" ], "expanded": false, - "advancedOnly": true, - "visibleWhen": { - "method": [ - "flowFps" - ] - } + "advancedOnly": true } ] } diff --git a/app/assets/filters/core/noise_reduction.json b/app/assets/filters/core/noise_reduction.json index 3293897..b3d7400 100644 --- a/app/assets/filters/core/noise_reduction.json +++ b/app/assets/filters/core/noise_reduction.json @@ -869,15 +869,15 @@ "min": 0, "max": 20, "step": 1, - "visibleWhen": { - "method": [ - "mclean" - ] - }, "ui": { "label": "Strength", "description": "How hard to denoise.", - "widget": "slider" + "widget": "slider", + "visibleWhen": { + "method": [ + "mclean" + ] + } } }, "mcleanSharp": { @@ -886,15 +886,15 @@ "min": 0, "max": 20, "step": 1, - "visibleWhen": { - "method": [ - "mclean" - ] - }, "ui": { "label": "Detail restore", "description": "How much fine detail to put back afterwards.", - "widget": "slider" + "widget": "slider", + "visibleWhen": { + "method": [ + "mclean" + ] + } } }, "mcleanRn": { @@ -903,15 +903,15 @@ "min": 0, "max": 20, "step": 1, - "visibleWhen": { - "method": [ - "mclean" - ] - }, "ui": { "label": "Grain restore", "description": "How much grain to put back, so the result does not look plastic.", - "widget": "slider" + "widget": "slider", + "visibleWhen": { + "method": [ + "mclean" + ] + } } }, "mcleanThsad": { @@ -920,29 +920,29 @@ "min": 50, "max": 1200, "step": 50, - "visibleWhen": { - "method": [ - "mclean" - ] - }, "ui": { "label": "Motion threshold", "description": "How much movement is tolerated when matching frames.", - "widget": "slider" + "widget": "slider", + "visibleWhen": { + "method": [ + "mclean" + ] + } } }, "mcleanChroma": { "type": "boolean", "default": true, - "visibleWhen": { - "method": [ - "mclean" - ] - }, "ui": { "label": "Denoise colour too", "description": "Leave on unless colour is already clean.", - "widget": "checkbox" + "widget": "checkbox", + "visibleWhen": { + "method": [ + "mclean" + ] + } } }, "td2DegrainTr": { @@ -951,15 +951,15 @@ "min": 1, "max": 3, "step": 1, - "visibleWhen": { - "method": [ - "temporal_degrain2" - ] - }, "ui": { "label": "Temporal radius", "description": "How many frames either side are used. Higher is stronger and much slower.", - "widget": "slider" + "widget": "slider", + "visibleWhen": { + "method": [ + "temporal_degrain2" + ] + } } }, "td2GrainLevel": { @@ -968,15 +968,15 @@ "min": -2, "max": 3, "step": 1, - "visibleWhen": { - "method": [ - "temporal_degrain2" - ] - }, "ui": { "label": "Source noise level", "description": "How noisy the source is. Higher tunes everything for a dirtier picture.", - "widget": "slider" + "widget": "slider", + "visibleWhen": { + "method": [ + "temporal_degrain2" + ] + } } }, "td2PostFft": { @@ -985,15 +985,15 @@ "min": 0, "max": 3, "step": 1, - "visibleWhen": { - "method": [ - "temporal_degrain2" - ] - }, "ui": { "label": "Extra cleanup", "description": "A second frequency-domain pass on top. 0 is off.", - "widget": "slider" + "widget": "slider", + "visibleWhen": { + "method": [ + "temporal_degrain2" + ] + } } }, "td2PostSigma": { @@ -1002,16 +1002,16 @@ "min": 0.0, "max": 16.0, "step": 0.5, - "visibleWhen": { - "method": [ - "temporal_degrain2" - ] - }, "ui": { "label": "Cleanup strength", "description": "How hard the extra pass works.", "widget": "slider", - "precision": 1 + "precision": 1, + "visibleWhen": { + "method": [ + "temporal_degrain2" + ] + } } }, "td2PostMix": { @@ -1020,36 +1020,36 @@ "min": 0, "max": 100, "step": 5, - "visibleWhen": { - "method": [ - "temporal_degrain2" - ] - }, "ui": { "label": "Blend back", "description": "Mix some of the un-cleaned picture back in, to keep texture.", - "widget": "slider" + "widget": "slider", + "visibleWhen": { + "method": [ + "temporal_degrain2" + ] + } } }, "td2ChromaMotion": { "type": "boolean", "default": true, - "visibleWhen": { - "method": [ - "temporal_degrain2" - ] - }, "ui": { "label": "Use colour for motion", "description": "Usually helps on analogue captures.", - "widget": "checkbox" + "widget": "checkbox", + "visibleWhen": { + "method": [ + "temporal_degrain2" + ] + } } } }, "ui": { "sections": [ { - "title": "SMDegrain Settings", + "title": "SMDegrain", "parameters": [ "smDegrainTr", "smDegrainThSAD", @@ -1060,7 +1060,7 @@ "expanded": true }, { - "title": "MCTemporalDenoise Settings", + "title": "MCTemporalDenoise", "parameters": [ "mcTemporalProfile", "mcTemporalSigma", @@ -1069,7 +1069,7 @@ "expanded": true }, { - "title": "QTGMC Built-in Settings", + "title": "QTGMC built-in", "parameters": [ "qtgmcEzDenoise", "qtgmcEzKeepGrain" @@ -1164,12 +1164,7 @@ "mcleanRn", "mcleanChroma" ], - "expanded": true, - "visibleWhen": { - "method": [ - "mclean" - ] - } + "expanded": true }, { "title": "mClean tuning", @@ -1177,12 +1172,7 @@ "mcleanThsad" ], "expanded": false, - "advancedOnly": true, - "visibleWhen": { - "method": [ - "mclean" - ] - } + "advancedOnly": true }, { "title": "TemporalDegrain2", @@ -1195,12 +1185,7 @@ "td2ChromaMotion" ], "expanded": true, - "advancedOnly": true, - "visibleWhen": { - "method": [ - "temporal_degrain2" - ] - } + "advancedOnly": true } ] }, diff --git a/app/assets/filters/core/spotless.json b/app/assets/filters/core/spotless.json index 63dadd3..57efdb7 100644 --- a/app/assets/filters/core/spotless.json +++ b/app/assets/filters/core/spotless.json @@ -56,7 +56,12 @@ "optional": true, "ui": { "label": "Process Chroma", - "description": "Include chroma planes in spot removal (disable to avoid ghost artifacts)" + "description": "Include chroma planes in spot removal (disable to avoid ghost artifacts)", + "visibleWhen": { + "method": [ + "spotless" + ] + } } }, "rec": { @@ -65,7 +70,12 @@ "optional": true, "ui": { "label": "Recalculate Vectors", - "description": "Refine motion vectors at finer block size for more precision (slower)" + "description": "Refine motion vectors at finer block size for more precision (slower)", + "visibleWhen": { + "method": [ + "spotless" + ] + } } }, "blksize": { @@ -79,7 +89,12 @@ "label": "Block Size", "description": "Block size for motion analysis (auto: 32 for 4K, 16 for HD, 8 for SD)", "widget": "slider", - "advanced": true + "advanced": true, + "visibleWhen": { + "method": [ + "spotless" + ] + } } }, "overlap": { @@ -93,7 +108,12 @@ "label": "Overlap", "description": "Block overlap for motion analysis (default: half of block size)", "widget": "slider", - "advanced": true + "advanced": true, + "visibleWhen": { + "method": [ + "spotless" + ] + } } }, "pel": { @@ -114,7 +134,12 @@ "2": "2 (half-pixel)", "4": "4 (quarter-pixel)" }, - "advanced": true + "advanced": true, + "visibleWhen": { + "method": [ + "spotless" + ] + } } }, "method": { @@ -136,15 +161,15 @@ "min": 0, "max": 255, "step": 5, - "visibleWhen": { - "method": [ - "removeDirt" - ] - }, "ui": { "label": "Spot size", "description": "How large a difference from its neighbours counts as dirt.", - "widget": "slider" + "widget": "slider", + "visibleWhen": { + "method": [ + "removeDirt" + ] + } } }, "rdNoisy": { @@ -153,15 +178,15 @@ "min": 0, "max": 64, "step": 1, - "visibleWhen": { - "method": [ - "removeDirt" - ] - }, "ui": { "label": "Spot density", "description": "How many neighbouring pixels must agree before it is treated as damage.", - "widget": "slider" + "widget": "slider", + "visibleWhen": { + "method": [ + "removeDirt" + ] + } } }, "rdGmthreshold": { @@ -170,15 +195,15 @@ "min": 0, "max": 255, "step": 5, - "visibleWhen": { - "method": [ - "removeDirt" - ] - }, "ui": { "label": "Motion tolerance", "description": "How much of the frame may be moving before it is treated as motion rather than damage.", - "widget": "slider" + "widget": "slider", + "visibleWhen": { + "method": [ + "removeDirt" + ] + } } }, "rdDist": { @@ -187,67 +212,50 @@ "min": 0, "max": 8, "step": 1, - "visibleWhen": { - "method": [ - "removeDirt" - ] - }, "ui": { "label": "Spread", "description": "How far around a detected spot to repair.", - "widget": "slider" + "widget": "slider", + "visibleWhen": { + "method": [ + "removeDirt" + ] + } } }, "rdPostDenoise": { "type": "boolean", "default": false, - "visibleWhen": { - "method": [ - "removeDirt" - ] - }, "ui": { "label": "Extra smoothing pass", "description": "The traditional final smoothing step. Off by default \u2014 measured, it alone triples the damage to clean parts of the picture.", - "widget": "checkbox" + "widget": "checkbox", + "visibleWhen": { + "method": [ + "removeDirt" + ] + } } } }, "ui": { "sections": [ { - "title": "Method", - "parameters": [ - "method" - ], - "expanded": true - }, - { - "title": "Settings", + "title": "SpotLess settings", "parameters": [ "chroma", "rec" ], - "expanded": true, - "visibleWhen": { - "method": [ - "spotless" - ] - } + "expanded": true }, { - "title": "Motion Analysis", + "title": "Motion analysis", "parameters": [ "blksize", "overlap", "pel" ], - "expanded": false, - "visibleWhen": { - "method": [ - "spotless" - ] - } + "expanded": false }, { "title": "RemoveDirt settings", @@ -256,12 +264,7 @@ "rdNoisy", "rdGmthreshold" ], - "expanded": true, - "visibleWhen": { - "method": [ - "removeDirt" - ] - } + "expanded": true }, { "title": "RemoveDirt tuning", @@ -270,12 +273,7 @@ "rdPostDenoise" ], "expanded": false, - "advancedOnly": true, - "visibleWhen": { - "method": [ - "removeDirt" - ] - } + "advancedOnly": true } ] }, diff --git a/app/assets/filters/core/subtitles.json b/app/assets/filters/core/subtitles.json index e6fbbc6..51180ad 100644 --- a/app/assets/filters/core/subtitles.json +++ b/app/assets/filters/core/subtitles.json @@ -93,12 +93,6 @@ "burnInPath": { "type": "string", "default": "", - "visibleWhen": { - "output": [ - "burn_in", - "burn_in_and_srt" - ] - }, "ui": { "label": "Subtitle file to burn in", "description": "Path to a subtitle file you already have. Leave this empty to burn in what Whisper transcribes \u2014 transcription runs before the encode, so its subtitles can be drawn into the picture. Setting a file here skips transcription entirely and burns in this file instead.", @@ -107,7 +101,13 @@ "srt", "ass", "ssa" - ] + ], + "visibleWhen": { + "output": [ + "burn_in", + "burn_in_and_srt" + ] + } } } }, diff --git a/app/lib/models/chroma_fix_parameters.dart b/app/lib/models/chroma_fix_parameters.dart index e5cb64a..f3d5548 100644 --- a/app/lib/models/chroma_fix_parameters.dart +++ b/app/lib/models/chroma_fix_parameters.dart @@ -77,16 +77,39 @@ class ChromaFixParameters { /// format afterwards. final bool applyDeRainbow; - /// DeDot — temporal dot crawl / rainbow removal on both planes. + // --- Automatic chroma alignment --- + + /// Measure the chroma misalignment and correct it, rather than asking the + /// user to guess it on the [chromaShiftH]/[chromaShiftV] sliders. Takes + /// precedence over them — see [effectiveApplyChromaShift]. final bool applyAutoChroma; + + /// Largest shift to search for, in pixels. final int autoChromaMaxShift; + + /// Sub-pixel search step. Smaller is finer and slower. final double autoChromaAccuracy; + + /// Measure once on this frame; -1 measures every frame (~23x the cost). final int autoChromaReferenceFrame; + // --- DeDot (temporal dot crawl removal, both planes) --- + + /// Apply DeDot. Complementary to [applyDeCrawl] rather than an alternative: + /// it compares neighbouring frames where LUTDeCrawl works inside one, and + /// each reaches a crawl geometry the other leaves alone. final bool applyDedot; + + /// Spatial luma threshold (0-510). final int dedotLuma2d; + + /// Temporal luma threshold (0-255). final int dedotLumaT; + + /// Chroma threshold (0-255). final int dedotChromaT1; + + /// Chroma motion limit (0-255). 255 leaves chroma alone entirely. final int dedotChromaT2; /// Chroma difference threshold for detecting rainbowing. @@ -146,8 +169,9 @@ class ChromaFixParameters { this.deCrawlYThresh = 10, this.deCrawlCThresh = 10, this.deCrawlMaxDiff = 50, - // Vinverse defaults + // LUTDeRainbow defaults this.applyDeRainbow = false, + // Automatic alignment defaults this.applyAutoChroma = false, this.autoChromaMaxShift = 2, this.autoChromaAccuracy = 0.25, @@ -217,6 +241,10 @@ class ChromaFixParameters { ChromaFixParameters copyWith({ bool? enabled, ChromaFixPreset? preset, + bool? applyAutoChroma, + int? autoChromaMaxShift, + double? autoChromaAccuracy, + int? autoChromaReferenceFrame, bool? applyChromaShift, double? chromaShiftH, double? chromaShiftV, @@ -230,6 +258,11 @@ class ChromaFixParameters { int? deCrawlCThresh, int? deCrawlMaxDiff, bool? applyDeRainbow, + bool? applyDedot, + int? dedotLuma2d, + int? dedotLumaT, + int? dedotChromaT1, + int? dedotChromaT2, int? deRainbowCThresh, int? deRainbowYThresh, bool? deRainbowUseLuma, @@ -245,6 +278,11 @@ class ChromaFixParameters { return ChromaFixParameters( enabled: enabled ?? this.enabled, preset: preset ?? this.preset, + applyAutoChroma: applyAutoChroma ?? this.applyAutoChroma, + autoChromaMaxShift: autoChromaMaxShift ?? this.autoChromaMaxShift, + autoChromaAccuracy: autoChromaAccuracy ?? this.autoChromaAccuracy, + autoChromaReferenceFrame: + autoChromaReferenceFrame ?? this.autoChromaReferenceFrame, applyChromaShift: applyChromaShift ?? this.applyChromaShift, chromaShiftH: chromaShiftH ?? this.chromaShiftH, chromaShiftV: chromaShiftV ?? this.chromaShiftV, @@ -258,6 +296,11 @@ class ChromaFixParameters { deCrawlCThresh: deCrawlCThresh ?? this.deCrawlCThresh, deCrawlMaxDiff: deCrawlMaxDiff ?? this.deCrawlMaxDiff, applyDeRainbow: applyDeRainbow ?? this.applyDeRainbow, + applyDedot: applyDedot ?? this.applyDedot, + dedotLuma2d: dedotLuma2d ?? this.dedotLuma2d, + dedotLumaT: dedotLumaT ?? this.dedotLumaT, + dedotChromaT1: dedotChromaT1 ?? this.dedotChromaT1, + dedotChromaT2: dedotChromaT2 ?? this.dedotChromaT2, deRainbowCThresh: deRainbowCThresh ?? this.deRainbowCThresh, deRainbowYThresh: deRainbowYThresh ?? this.deRainbowYThresh, deRainbowUseLuma: deRainbowUseLuma ?? this.deRainbowUseLuma, @@ -272,6 +315,17 @@ class ChromaFixParameters { ); } + /// Whether the manual Y/C delay sliders actually reach the render. + /// + /// Automatic alignment measures the shift and applies it, and it runs *before* + /// the manual shift in the script — so with both set the picture is corrected + /// twice. The automatic measurement wins: the manual controls are hidden in the + /// panel while it is on, [ScriptGenerator] omits the manual block, and this is + /// the single derivation everything else asks. It deliberately does not clear + /// [applyChromaShift], so turning automatic back off restores what the user + /// had set by hand. + bool get effectiveApplyChromaShift => applyChromaShift && !applyAutoChroma; + /// Get a human-readable summary of the current settings. String get summary { if (!enabled) return 'Off'; @@ -287,10 +341,17 @@ class ChromaFixParameters { return preset.name; } } + // Every repair the pass will actually perform. A fix missing from this list + // is a fix the pass row claims not to be doing — which is how DeDot and + // automatic alignment ran invisibly on the VHS Cleanup and DV presets. final fixes = []; - if (applyChromaShift) fixes.add('Shift'); + if (applyAutoChroma) fixes.add('Auto align'); + if (effectiveApplyChromaShift) fixes.add('Shift'); if (applyChromaBleedingFix) fixes.add('Bleed'); if (applyDeCrawl) fixes.add('Crawl'); + if (applyDedot) fixes.add('DeDot'); + if (applyDeRainbow) fixes.add('Rainbow'); + if (applyBifrost) fixes.add('Bifrost'); if (applyVinverse) fixes.add('Vinv'); return fixes.isEmpty ? 'Custom' : fixes.join('+'); } diff --git a/app/lib/models/color_correction_parameters.dart b/app/lib/models/color_correction_parameters.dart index 81d3c69..d545d60 100644 --- a/app/lib/models/color_correction_parameters.dart +++ b/app/lib/models/color_correction_parameters.dart @@ -187,6 +187,12 @@ class ColorCorrectionParameters { ColorCorrectionParameters copyWith({ bool? enabled, ColorCorrectionPreset? preset, + bool? applyAutoLevels, + int? autoLevelsBlack, + int? autoLevelsWhite, + double? autoLevelsStrength, + bool? applyAutoWhiteBalance, + double? autoWhiteBalanceStrength, double? brightness, double? contrast, double? hue, @@ -209,6 +215,14 @@ class ColorCorrectionParameters { return ColorCorrectionParameters( enabled: enabled ?? this.enabled, preset: preset ?? this.preset, + applyAutoLevels: applyAutoLevels ?? this.applyAutoLevels, + autoLevelsBlack: autoLevelsBlack ?? this.autoLevelsBlack, + autoLevelsWhite: autoLevelsWhite ?? this.autoLevelsWhite, + autoLevelsStrength: autoLevelsStrength ?? this.autoLevelsStrength, + applyAutoWhiteBalance: + applyAutoWhiteBalance ?? this.applyAutoWhiteBalance, + autoWhiteBalanceStrength: + autoWhiteBalanceStrength ?? this.autoWhiteBalanceStrength, brightness: brightness ?? this.brightness, contrast: contrast ?? this.contrast, hue: hue ?? this.hue, @@ -230,6 +244,28 @@ class ColorCorrectionParameters { ); } + /// The input/output points the levels adjustment actually applies. + /// + /// Automatic levels measures the picture and places black and white itself, + /// running before this — so the manual points are dropped and only gamma + /// survives, which is the one thing automatic levels does not touch. The + /// panel hides those four sliders to match, and + /// `ColorCorrectionParameters::effective_levels_points` is the worker's twin + /// of this. The stored values are left alone, so unticking automatic levels + /// brings back what the user set by hand. + (int, int, int, int) get effectiveLevelsPoints => applyAutoLevels + ? (0, 255, 0, 255) + : (inputLow, inputHigh, outputLow, outputHigh); + + /// Whether the levels adjustment will do anything: its switch, and a change + /// to make. `applyLevels` used to be a UI-only flag the worker ignored. + bool get effectiveApplyLevels { + if (!applyLevels) return false; + final (inLow, inHigh, outLow, outHigh) = effectiveLevelsPoints; + return inLow != 0 || inHigh != 255 || outLow != 0 || outHigh != 255 || + gamma != 1.0; + } + /// Get a human-readable summary of the current settings. String get summary { if (!enabled) return 'Off'; @@ -245,10 +281,17 @@ class ColorCorrectionParameters { return preset.name; } } + // Everything the pass will actually do. Auto levels, auto white balance, + // levels and shadow detail were all missing, so a pass doing four things + // could summarise itself as "Custom". final parts = []; + if (applyAutoLevels) parts.add('Auto levels'); + if (applyAutoWhiteBalance) parts.add('Auto WB'); if (brightness != 0) parts.add('B:${brightness.toStringAsFixed(0)}'); if (contrast != 1) parts.add('C:${contrast.toStringAsFixed(1)}'); if (saturation != 1) parts.add('S:${saturation.toStringAsFixed(1)}'); + if (effectiveApplyLevels) parts.add('Levels'); + if (applyShadowDetail) parts.add('Shadows'); if (temperature != 0) parts.add('Temp:${temperature.toStringAsFixed(0)}'); if (tint != 0) parts.add('Tint:${tint.toStringAsFixed(0)}'); return parts.isEmpty ? 'Custom' : parts.join(' '); diff --git a/app/lib/models/noise_reduction_parameters.dart b/app/lib/models/noise_reduction_parameters.dart index aa019f0..015cb24 100644 --- a/app/lib/models/noise_reduction_parameters.dart +++ b/app/lib/models/noise_reduction_parameters.dart @@ -346,6 +346,19 @@ class NoiseReductionParameters { int? stpressoTthr, int? ctmfRadius, int? ctmfPlanes, + bool? contraSharpen, + int? contraSharpenRep, + int? mcleanStrength, + int? mcleanSharp, + int? mcleanRn, + int? mcleanThsad, + bool? mcleanChroma, + int? td2DegrainTr, + int? td2GrainLevel, + int? td2PostFft, + double? td2PostSigma, + int? td2PostMix, + bool? td2ChromaMotion, }) { return NoiseReductionParameters( enabled: enabled ?? this.enabled, @@ -385,6 +398,19 @@ class NoiseReductionParameters { stpressoTthr: stpressoTthr ?? this.stpressoTthr, ctmfRadius: ctmfRadius ?? this.ctmfRadius, ctmfPlanes: ctmfPlanes ?? this.ctmfPlanes, + contraSharpen: contraSharpen ?? this.contraSharpen, + contraSharpenRep: contraSharpenRep ?? this.contraSharpenRep, + mcleanStrength: mcleanStrength ?? this.mcleanStrength, + mcleanSharp: mcleanSharp ?? this.mcleanSharp, + mcleanRn: mcleanRn ?? this.mcleanRn, + mcleanThsad: mcleanThsad ?? this.mcleanThsad, + mcleanChroma: mcleanChroma ?? this.mcleanChroma, + td2DegrainTr: td2DegrainTr ?? this.td2DegrainTr, + td2GrainLevel: td2GrainLevel ?? this.td2GrainLevel, + td2PostFft: td2PostFft ?? this.td2PostFft, + td2PostSigma: td2PostSigma ?? this.td2PostSigma, + td2PostMix: td2PostMix ?? this.td2PostMix, + td2ChromaMotion: td2ChromaMotion ?? this.td2ChromaMotion, ); } diff --git a/app/lib/models/pass_advice.dart b/app/lib/models/pass_advice.dart index 5c4eb91..5c083fe 100644 --- a/app/lib/models/pass_advice.dart +++ b/app/lib/models/pass_advice.dart @@ -111,6 +111,57 @@ List adviseOn(ProcessingPipeline pipeline) { )); } + // --- Levels set by hand on top of levels set automatically --- + // Automatic levels measures the picture and places black and white itself, + // and it runs first — so the manual input/output points are dropped (the + // panel hides them) and only gamma survives. Said out loud, because a preset + // can carry both and the user would otherwise see neither the sliders nor a + // reason for their absence. + if (on(PassType.colorCorrection) && + pipeline.colorCorrection.applyAutoLevels && + pipeline.colorCorrection.applyLevels) { + advice.add(const PassAdvice( + PassType.colorCorrection, + 'Levels are being set automatically, so the input and output points here ' + 'are not used — the automatic pass has already placed black and white. ' + 'Gamma still applies, and switching automatic levels off brings the ' + 'sliders back.', + )); + } + + // --- White balance by hand after white balance by measurement --- + // Unlike the levels pair these compose legitimately: the automatic pass + // neutralises the cast and the sliders then offset the result deliberately + // ("neutral, but a little warmer"). Worth saying which order they happen in, + // not worth preventing. + if (on(PassType.colorCorrection) && + pipeline.colorCorrection.applyAutoWhiteBalance && + (pipeline.colorCorrection.temperature != 0 || + pipeline.colorCorrection.tint != 0)) { + advice.add(const PassAdvice( + PassType.colorCorrection, + 'Temperature and tint are applied after the automatic white balance, so ' + 'they shift the corrected picture rather than the original.', + )); + } + + // --- A manual chroma shift that automatic alignment has superseded --- + // The two are alternatives, and the panel hides the manual sliders while + // automatic alignment is on — so a preset or saved job that set both leaves + // the user no way to see that their hand-set shift is not being used. + // ChromaFixParameters.effectiveApplyChromaShift is what decides. + if (on(PassType.chromaFixes) && + pipeline.chromaFixes.applyAutoChroma && + pipeline.chromaFixes.applyChromaShift) { + advice.add(const PassAdvice( + PassType.chromaFixes, + 'Colour alignment is being measured automatically, so the manual Y/C ' + 'delay shift saved with this preset is not applied — correcting a ' + 'measured shift again by hand would double it. Switch automatic ' + 'alignment off to go back to the sliders.', + )); + } + // --- Rotating interlaced material destroys it --- // Fields are stored as alternating horizontal lines. A quarter turn puts them // in alternating COLUMNS, where no deinterlacer can find them: measured, diff --git a/app/lib/models/qtgmc_parameters.dart b/app/lib/models/qtgmc_parameters.dart index 9fb05ac..9d63d0e 100644 --- a/app/lib/models/qtgmc_parameters.dart +++ b/app/lib/models/qtgmc_parameters.dart @@ -460,6 +460,7 @@ class QTGMCParameters { int? ivtcCycle, double? ivtcDupthresh, double? ivtcScthresh, + bool? bwdifEdeint, }) { return QTGMCParameters( enabled: enabled ?? this.enabled, @@ -552,6 +553,7 @@ class QTGMCParameters { ivtcCycle: ivtcCycle ?? this.ivtcCycle, ivtcDupthresh: ivtcDupthresh ?? this.ivtcDupthresh, ivtcScthresh: ivtcScthresh ?? this.ivtcScthresh, + bwdifEdeint: bwdifEdeint ?? this.bwdifEdeint, ); } } diff --git a/app/lib/models/spotless_parameters.dart b/app/lib/models/spotless_parameters.dart index af5e243..6afb0b6 100644 --- a/app/lib/models/spotless_parameters.dart +++ b/app/lib/models/spotless_parameters.dart @@ -67,6 +67,12 @@ class SpotLessParameters { int? blksize, int? overlap, int? pel, + SpotLessMethod? method, + int? rdNoise, + int? rdNoisy, + int? rdGmthreshold, + int? rdDist, + bool? rdPostDenoise, }) { return SpotLessParameters( enabled: enabled ?? this.enabled, @@ -75,6 +81,12 @@ class SpotLessParameters { blksize: blksize ?? this.blksize, overlap: overlap ?? this.overlap, pel: pel ?? this.pel, + method: method ?? this.method, + rdNoise: rdNoise ?? this.rdNoise, + rdNoisy: rdNoisy ?? this.rdNoisy, + rdGmthreshold: rdGmthreshold ?? this.rdGmthreshold, + rdDist: rdDist ?? this.rdDist, + rdPostDenoise: rdPostDenoise ?? this.rdPostDenoise, ); } diff --git a/app/lib/models/subtitle_parameters.dart b/app/lib/models/subtitle_parameters.dart index 6739d83..5578a6f 100644 --- a/app/lib/models/subtitle_parameters.dart +++ b/app/lib/models/subtitle_parameters.dart @@ -68,12 +68,14 @@ class SubtitleParameters { WhisperModel? model, SubtitleOutput? output, String? language, + String? burnInPath, }) { return SubtitleParameters( enabled: enabled ?? this.enabled, model: model ?? this.model, output: output ?? this.output, language: language ?? this.language, + burnInPath: burnInPath ?? this.burnInPath, ); } diff --git a/app/lib/views/settings/dynamic_filter_panel.dart b/app/lib/views/settings/dynamic_filter_panel.dart index 50a3dd8..b7a6933 100644 --- a/app/lib/views/settings/dynamic_filter_panel.dart +++ b/app/lib/views/settings/dynamic_filter_panel.dart @@ -386,15 +386,28 @@ class DynamicFilterPanelCompact extends StatelessWidget { // Skip sections with no visible parameters if (sectionWidgets.isEmpty) continue; - // In advanced mode with sections, show section headers - if (advancedMode && section.advancedOnly) { + // Headings, so a section is a visible group and not merely an ordering. + // Only where there is more than one to tell apart: a schema with a + // single section has nothing to distinguish, and most of those call it + // "Settings", which is a heading that says nothing. Advanced-only + // sections keep the accent colour, so it stays obvious which controls + // appeared because advanced mode is on. + if (sections.length > 1) { widgets.add( Padding( - padding: const EdgeInsets.only(top: 16, bottom: 8), + padding: EdgeInsets.only( + top: widgets.isEmpty ? 0 : 16, + bottom: 8, + ), child: Text( section.title, style: Theme.of(context).textTheme.titleSmall?.copyWith( - color: Theme.of(context).colorScheme.primary, + color: section.advancedOnly + ? Theme.of(context).colorScheme.primary + : Theme.of(context) + .colorScheme + .onSurface + .withValues(alpha: 0.7), ), ), ), diff --git a/app/test/chroma_fixes_panel_test.dart b/app/test/chroma_fixes_panel_test.dart new file mode 100644 index 0000000..09815cb --- /dev/null +++ b/app/test/chroma_fixes_panel_test.dart @@ -0,0 +1,216 @@ +// What the Chroma Fixes panel actually shows, pumped from the shipped schema. +// +// The pass covers five unrelated repairs, and it was reported as unintuitive +// because every repair's tuning sliders were on screen at once whether the +// repair was switched on or not. The cause was not the grouping: the conditions +// were written at `parameters..visibleWhen`, one level above the only place +// `ParameterUiConfig` reads them from, so they were dropped at parse time and +// nothing was ever hidden. +// +// filter_schema_curation_test.dart lints the JSON for that mistake. This is the +// other half — proof that the panel really does hide what the schema says to +// hide, which a lint over the file cannot show. It pumps the real +// chroma_fixes.json rather than a fixture, so a future edit that reintroduces an +// always-visible slider fails here. +// +// Run with: flutter test test/chroma_fixes_panel_test.dart + +import 'dart:convert'; +import 'dart:io'; + +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:provider/provider.dart'; +import 'package:shared_preferences/shared_preferences.dart'; +import 'package:vapourbox/models/chroma_fix_parameters.dart'; +import 'package:vapourbox/models/filter_schema.dart'; +import 'package:vapourbox/models/parameter_converter.dart'; +import 'package:vapourbox/services/advanced_mode_service.dart'; +import 'package:vapourbox/views/settings/dynamic_filter_panel.dart'; + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + final advanced = AdvancedModeService.instance; + + final schema = FilterSchema.fromJson( + jsonDecode(File('assets/filters/core/chroma_fixes.json').readAsStringSync()) + as Map, + ); + + setUp(() { + SharedPreferences.setMockInitialValues({}); + advanced.resetForTesting(); + }); + + /// Pump the panel for [params], exactly as the pass settings panel builds it. + Future pump( + WidgetTester tester, + ChromaFixParameters params, { + bool advancedMode = false, + }) async { + await advanced.initialize(); + if (advancedMode) await advanced.setEnabled(true); + + await tester.pumpWidget( + ChangeNotifierProvider.value( + value: advanced, + child: MaterialApp( + home: Scaffold( + body: SingleChildScrollView( + child: DynamicFilterPanelCompact( + schema: schema, + params: ParameterConverter.fromChromaFixes(params), + onChanged: (_) {}, + ), + ), + ), + ), + ), + ); + await tester.pumpAndSettle(); + } + + /// A slider renders as "Label: value", so match on the label alone. + Finder slider(String label) => find.textContaining('$label:'); + + group('nothing is on screen for a repair that is switched off', () { + testWidgets('the default panel is switches only', (tester) async { + await pump(tester, const ChromaFixParameters(enabled: true)); + + // Every repair is offered... + for (final label in [ + 'Correct colour alignment automatically', + 'Correct colour alignment by hand (Y/C delay)', + 'Fix colour bleeding past edges', + 'Remove dot crawl', + 'Remove rainbow shimmer', + 'Remove chroma combing', + ]) { + expect(find.text(label), findsOneWidget, reason: label); + } + + // ...and not one of them has a control on screen yet. + for (final label in [ + 'Horizontal shift', + 'Vertical shift', + 'Strength', + 'Colour blur', + 'Brightness threshold', + 'Colour threshold', + 'Motion threshold', + 'Caution', + 'Maximum change', + ]) { + expect(slider(label), findsNothing, reason: label); + } + }); + + testWidgets('a switch reveals only its own controls', (tester) async { + await pump( + tester, + const ChromaFixParameters(enabled: true, applyChromaBleedingFix: true), + ); + + expect(slider('Strength'), findsOneWidget); + expect(slider('Colour blur'), findsOneWidget); + // Vinverse also has a "Strength", and its repair is off — so exactly one + // of that label may be present. + expect(slider('Horizontal shift'), findsNothing); + expect(slider('Colour threshold'), findsNothing); + }); + }); + + group('each repair is a named group', () { + testWidgets('simple mode shows a heading per repair', (tester) async { + // Without headings the panel is one flat run of switches, and which + // slider belongs to which repair is left to the reader. The panel prints + // section titles whenever a schema has more than one section. + await pump(tester, const ChromaFixParameters(enabled: true)); + + for (final title in [ + 'Colour alignment', + 'Colour bleeding', + 'Dot crawl', + 'Rainbowing', + 'Chroma combing', + ]) { + expect(find.text(title), findsOneWidget, reason: title); + } + + // Tuning sections belong to advanced mode, headings included. + expect(find.text('Automatic alignment tuning'), findsNothing); + }); + }); + + group('automatic and manual alignment are alternatives', () { + testWidgets('automatic alignment withdraws the manual sliders', + (tester) async { + await pump( + tester, + const ChromaFixParameters( + enabled: true, + applyAutoChroma: true, + applyChromaShift: true, + chromaShiftH: 2.0, + ), + ); + + // Not merely disabled — gone, because the worker drops the manual block + // when automatic alignment is on (test_150) and a visible slider that + // changes nothing is worse than no slider. + expect(find.text('Correct colour alignment by hand (Y/C delay)'), + findsNothing); + expect(slider('Horizontal shift'), findsNothing); + expect(slider('Vertical shift'), findsNothing); + }); + + testWidgets('with automatic off the manual sliders come back', + (tester) async { + await pump( + tester, + const ChromaFixParameters( + enabled: true, + applyChromaShift: true, + chromaShiftH: 2.0, + ), + ); + + expect(find.text('Correct colour alignment by hand (Y/C delay)'), + findsOneWidget); + expect(slider('Horizontal shift'), findsOneWidget); + expect(slider('Vertical shift'), findsOneWidget); + }); + }); + + group('thresholds wait for advanced mode', () { + testWidgets('simple mode shows no tuning for an enabled repair', + (tester) async { + await pump( + tester, + const ChromaFixParameters(enabled: true, applyAutoChroma: true), + ); + + expect(find.text('Correct colour alignment automatically'), findsOneWidget); + expect(slider('Search range'), findsNothing); + expect(find.text('Reference frame'), findsNothing); + }); + + testWidgets('advanced mode shows it, under its own heading', (tester) async { + await pump( + tester, + const ChromaFixParameters(enabled: true, applyAutoChroma: true), + advancedMode: true, + ); + + expect(find.text('Automatic alignment tuning'), findsOneWidget); + expect(slider('Search range'), findsOneWidget); + expect(find.text('Reference frame'), findsOneWidget); + + // Tuning for a repair that is off stays hidden even here — the advanced + // switch is not a "show me all 25 controls" switch. + expect(find.text('Dot crawl tuning'), findsNothing); + expect(slider('Colour motion limit'), findsNothing); + }); + }); +} diff --git a/app/test/color_correction_panel_test.dart b/app/test/color_correction_panel_test.dart new file mode 100644 index 0000000..c704f0b --- /dev/null +++ b/app/test/color_correction_panel_test.dart @@ -0,0 +1,164 @@ +// What the Color Correction panel shows, pumped from the shipped schema. +// +// The pass offers two adjustments that can be made automatically or by hand — +// levels and white balance — and the automatic half used to live in its own +// "Automatic" section three groups above the manual half it supersedes, with its +// own settings on screen whether it was switched on or not. It also rendered a +// Method dropdown ("Tweak" / "White Balance") that changed nothing at all: no +// parameter was conditional on it, no model had a method field, and the +// converter hardcoded 'tweak'. +// +// Run with: flutter test test/color_correction_panel_test.dart + +import 'dart:convert'; +import 'dart:io'; + +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:provider/provider.dart'; +import 'package:shared_preferences/shared_preferences.dart'; +import 'package:vapourbox/models/color_correction_parameters.dart'; +import 'package:vapourbox/models/filter_schema.dart'; +import 'package:vapourbox/models/parameter_converter.dart'; +import 'package:vapourbox/services/advanced_mode_service.dart'; +import 'package:vapourbox/views/settings/dynamic_filter_panel.dart'; + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + final advanced = AdvancedModeService.instance; + + final schema = FilterSchema.fromJson(jsonDecode( + File('assets/filters/core/color_correction.json').readAsStringSync(), + ) as Map); + + setUp(() { + SharedPreferences.setMockInitialValues({}); + advanced.resetForTesting(); + }); + + Future pump( + WidgetTester tester, + ColorCorrectionParameters params, { + bool advancedMode = false, + }) async { + await advanced.initialize(); + if (advancedMode) await advanced.setEnabled(true); + + await tester.pumpWidget( + ChangeNotifierProvider.value( + value: advanced, + child: MaterialApp( + home: Scaffold( + body: SingleChildScrollView( + child: DynamicFilterPanelCompact( + schema: schema, + params: ParameterConverter.fromColorCorrection(params), + onChanged: (_) {}, + ), + ), + ), + ), + ), + ); + await tester.pumpAndSettle(); + } + + Finder slider(String label) => find.textContaining('$label:'); + + testWidgets('no Method dropdown, and the groups are named', (tester) async { + await pump(tester, const ColorCorrectionParameters(enabled: true)); + + expect(find.text('Method'), findsNothing); + expect(find.byType(DropdownButtonFormField), findsNothing); + + for (final title in [ + 'Brightness and colour', + 'Levels', + 'White balance', + 'Shadow detail', + ]) { + expect(find.text(title), findsOneWidget, reason: title); + } + }); + + testWidgets('automatic settings stay hidden until it is switched on', + (tester) async { + await pump(tester, const ColorCorrectionParameters(enabled: true)); + expect(slider('Strength'), findsNothing); + + await pump( + tester, + const ColorCorrectionParameters(enabled: true, applyAutoLevels: true), + ); + // Auto levels' own strength appears; auto white balance's does not, because + // that half is still off. Both are labelled "Strength" and only their group + // tells them apart, which is what the headings are for. + expect(slider('Strength'), findsOneWidget); + // Its targets come with it, in simple mode: which black and white to aim + // for is the main thing to say about an automatic levels pass. + expect(slider('Target black'), findsOneWidget); + expect(slider('Target white'), findsOneWidget); + }); + + testWidgets('both automatic switches are named for what they set', + (tester) async { + await pump(tester, const ColorCorrectionParameters(enabled: true)); + + expect(find.text('Set levels automatically'), findsOneWidget); + expect(find.text('Set white balance automatically'), findsOneWidget); + }); + + testWidgets('automatic levels withdraws the manual points but keeps gamma', + (tester) async { + await pump( + tester, + const ColorCorrectionParameters( + enabled: true, + applyAutoLevels: true, + applyLevels: true, + inputLow: 16, + ), + ); + + for (final label in ['Input black', 'Input white', 'Output black', 'Output white']) { + expect(slider(label), findsNothing, reason: '$label is superseded'); + } + expect(slider('Gamma'), findsOneWidget, + reason: 'automatic levels never touches the midtones, and this is the ' + 'only control for them'); + }); + + testWidgets('with automatic levels off the points come back', (tester) async { + await pump( + tester, + const ColorCorrectionParameters( + enabled: true, + applyLevels: true, + inputLow: 16, + ), + ); + + expect(slider('Input black'), findsOneWidget); + expect(slider('Output white'), findsOneWidget); + expect(slider('Gamma'), findsOneWidget); + }); + + testWidgets('white balance keeps both halves, automatic first', + (tester) async { + // These compose legitimately — neutralise the cast, then warm it a little — + // so unlike the levels pair the sliders stay put. + await pump( + tester, + const ColorCorrectionParameters( + enabled: true, + applyAutoWhiteBalance: true, + temperature: 20, + ), + ); + + expect(find.text('Set white balance automatically'), findsOneWidget); + expect(slider('Temperature'), findsOneWidget); + expect(slider('Tint'), findsOneWidget); + }); +} diff --git a/app/test/dynamic_filter_panel_advanced_test.dart b/app/test/dynamic_filter_panel_advanced_test.dart index 22e12cb..abd933a 100644 --- a/app/test/dynamic_filter_panel_advanced_test.dart +++ b/app/test/dynamic_filter_panel_advanced_test.dart @@ -38,7 +38,7 @@ void main() { advancedOnly: advancedOnly, ); - FilterSchema buildSchema() => FilterSchema( + FilterSchema buildSchema({List? sections}) => FilterSchema( id: 'test_filter', version: '1.0.0', name: 'Test Filter', @@ -58,12 +58,21 @@ void main() { max: 10.0, ui: ParameterUiConfig(label: 'Strength'), ), + 'depth': const ParameterDefinition( + type: ParameterType.number, + defaultValue: 1.0, + min: 0.0, + max: 10.0, + ui: ParameterUiConfig(label: 'Depth'), + ), }, + ui: sections == null ? null : UiLayout(sections: sections), ); Future pumpPanel( WidgetTester tester, { required String selectedMethod, + List? sections, }) async { await tester.pumpWidget( ChangeNotifierProvider.value( @@ -72,11 +81,15 @@ void main() { home: Scaffold( body: SingleChildScrollView( child: DynamicFilterPanelCompact( - schema: buildSchema(), + schema: buildSchema(sections: sections), params: DynamicParameters( filterId: 'test_filter', enabled: true, - values: {'method': selectedMethod, 'strength': 1.0}, + values: { + 'method': selectedMethod, + 'strength': 1.0, + 'depth': 1.0, + }, ), onChanged: (_) {}, ), @@ -149,6 +162,39 @@ void main() { }); }); + group('section headings', () { + // A section is only a heading when there is another one to tell it apart + // from. Most single-section schemas call theirs "Settings", which as a + // heading says nothing at all — and it would sit above every pass in the + // app. + testWidgets('one section gets no heading', (tester) async { + await advanced.initialize(); + await pumpPanel( + tester, + selectedMethod: 'basic', + sections: const [UiSection(title: 'Settings', parameters: ['strength'])], + ); + + expect(find.text('Settings'), findsNothing); + expect(find.textContaining('Strength:'), findsOneWidget); + }); + + testWidgets('two sections are both named, in simple mode', (tester) async { + await advanced.initialize(); + await pumpPanel( + tester, + selectedMethod: 'basic', + sections: const [ + UiSection(title: 'Strength', parameters: ['strength']), + UiSection(title: 'Shape', parameters: ['depth']), + ], + ); + + expect(find.text('Strength'), findsOneWidget); + expect(find.text('Shape'), findsOneWidget); + }); + }); + group('AdvancedModeService provider scope', () { // The Settings dialog reads this service, and `showDialog` pushes onto the // MaterialApp's Navigator — so a provider placed inside `home` is out of diff --git a/app/test/filter_schema_curation_test.dart b/app/test/filter_schema_curation_test.dart index c4e3d8c..c23e17d 100644 --- a/app/test/filter_schema_curation_test.dart +++ b/app/test/filter_schema_curation_test.dart @@ -12,6 +12,16 @@ // - Every method needs a description, because that is the only guidance shown // beside it in the dropdown. A dropdown of sixteen bare names is worse than // one of four. +// - `visibleWhen` lives at `parameters..ui.visibleWhen` and nowhere else. +// `ParameterDefinition` and `UiSection` do not declare the key, so a copy +// written one level up (or on a section) is dropped at parse time and the +// control is simply always visible. Eight schemas shipped that way, which +// is how Chroma Fixes came to show its automatic-alignment sliders with +// automatic alignment switched off. +// - A condition must name something that exists. A key naming no parameter, +// or a `method` condition naming no method, can never be satisfied, so the +// control it guards is invisible forever — the same silent failure in the +// other direction. // // Run with: flutter test test/filter_schema_curation_test.dart @@ -27,12 +37,15 @@ void main() { ) as List) .cast(); - final schemas = [ + final rawSchemas = >{ for (final filename in manifest) - FilterSchema.fromJson( - jsonDecode(File('assets/filters/core/$filename').readAsStringSync()) - as Map, - ), + filename: jsonDecode( + File('assets/filters/core/$filename').readAsStringSync(), + ) as Map, + }; + + final schemas = [ + for (final raw in rawSchemas.values) FilterSchema.fromJson(raw), ]; test('the manifest lists every schema file', () { @@ -136,4 +149,304 @@ void main() { expect(sharpen.visibleMethods(showAdvanced: false).length, 3); }); }); + + group('conditional visibility is where the model can see it', () { + // Both halves matter: a condition in the wrong place never hides anything, + // and a condition naming something that does not exist never shows + // anything. Neither fails loudly at runtime. + rawSchemas.forEach((filename, raw) { + final id = raw['id'] as String; + final parameters = (raw['parameters'] as Map).cast(); + final methodIds = [ + for (final m in (raw['methods'] as List)) (m as Map)['id'] as String, + ]; + final sections = + ((raw['ui'] as Map?)?['sections'] as List?) ?? const []; + + test('$id: no parameter puts visibleWhen outside its ui block', () { + final misplaced = [ + for (final entry in parameters.entries) + if ((entry.value as Map).containsKey('visibleWhen')) entry.key, + ]; + expect(misplaced, isEmpty, + reason: 'ParameterDefinition has no visibleWhen field, so these ' + 'conditions are dropped at parse time and the controls are ' + 'always visible: $misplaced. Move each one into its `ui`.'); + }); + + test('$id: no section carries a visibleWhen', () { + final misplaced = [ + for (final s in sections) + if ((s as Map).containsKey('visibleWhen')) s['title'], + ]; + expect(misplaced, isEmpty, + reason: 'UiSection has no visibleWhen field, so these sections are ' + 'shown regardless: $misplaced. Put the condition on each ' + 'parameter in the section instead.'); + }); + + test('$id: every condition names something that exists', () { + for (final entry in parameters.entries) { + final ui = (entry.value as Map)['ui'] as Map?; + final condition = ui?['visibleWhen'] as Map?; + if (condition == null) continue; + + for (final key in condition.keys) { + expect(parameters.keys, contains(key), + reason: '${entry.key} is conditional on "$key", which is not ' + 'a parameter of this filter — it can never be satisfied'); + + if (key != 'method') continue; + final expected = condition[key]; + for (final value in expected is List ? expected : [expected]) { + expect(methodIds, contains(value), + reason: '${entry.key} is conditional on method "$value", ' + 'which this filter does not offer'); + } + } + } + }); + }); + }); + + group('colour correction pairs each automatic control with its manual one', () { + // The pass offers two adjustments that can be made automatically or by + // hand — levels and white balance — and the automatic half used to sit in + // its own "Automatic" section at the top, three groups away from the manual + // half it supersedes. Each pair now shares a section, automatic first. + final colour = schemas.firstWhere((s) => s.id == 'color_correction'); + final sections = colour.ui!.sections!; + + test('the sections pair them up', () { + expect(sections.map((s) => '${s.title}${s.advancedOnly ? " (adv)" : ""}'), + [ + 'Brightness and colour', + 'Brightness and colour tuning (adv)', + 'Levels', + 'Levels tuning (adv)', + 'White balance', + 'Shadow detail', + ]); + + final levels = sections.firstWhere((s) => s.title == 'Levels').parameters; + expect(levels.indexOf('applyAutoLevels'), + lessThan(levels.indexOf('applyLevels')), + reason: 'the automatic control comes first — it is the one that ' + 'supersedes the other'); + + final wb = + sections.firstWhere((s) => s.title == 'White balance').parameters; + expect(wb.indexOf('applyAutoWhiteBalance'), lessThan(wb.indexOf('temperature'))); + }); + + test('the automatic targets sit beside the switch that uses them', () { + // 16/235 (broadcast) against 0/255 (full range) is the main choice + // automatic levels offers, not a tuning detail — it was briefly moved into + // the advanced section and immediately read as missing. + final levels = sections.firstWhere((s) => s.title == 'Levels').parameters; + expect(levels, containsAll(['autoLevelsBlack', 'autoLevelsWhite'])); + expect(sections.firstWhere((s) => s.title == 'Levels tuning').parameters, + ['smoothLevels']); + }); + + test('the automatic switches say what they set', () { + // Both are named for the adjustment, not for the symptom, so the group + // heading and the switch agree and the words the user is looking for + // ("levels", "white balance") are on the control itself. + expect(colour.parameters['applyAutoLevels']!.ui!.label, + 'Set levels automatically'); + expect(colour.parameters['applyAutoWhiteBalance']!.ui!.label, + 'Set white balance automatically'); + }); + + test('there is no method dropdown to mislead anyone', () { + // It declared "Tweak" and "White Balance" as methods, but no parameter was + // ever conditional on the choice, no model had a method field, and the + // converter hardcoded 'tweak'. The dropdown rendered and changed nothing. + expect(colour.methods, hasLength(1)); + }); + + test('the manual levels points yield to the automatic measurement', () { + for (final id in ['inputLow', 'inputHigh', 'outputLow', 'outputHigh']) { + expect(colour.parameters[id]!.ui!.visibleWhen!['applyAutoLevels'], false, + reason: '$id is superseded by automatic levels and must hide'); + } + // Gamma is the exception, and deliberately so: automatic levels does not + // touch the midtones, and this is the only place in the app to reach them. + expect(colour.parameters['gamma']!.ui!.visibleWhen, + {'applyLevels': true}); + }); + + test('nothing carries a second enable checkbox', () { + for (final entry in colour.parameters.entries) { + expect(entry.value.optional ?? false, false, reason: entry.key); + } + }); + }); + + group('a method dropdown has to change something', () { + // Three schemas declared methods that gated nothing: Color Correction + // ("Tweak" / "White Balance") and Crop & Resize ("standard" / "nnedi3_2x" / + // "eedi3_2x"). In both, every control rendered whatever was selected, no + // model had a `method` field, and the converter either hardcoded one value + // or emitted none. The dropdown appeared, responded, and did nothing — + // which is worse than no dropdown, because it teaches the user the panel + // reacts to it. Crop & Resize was the sharper case: its real upscaler + // choice is the `upscaleMethod` PARAMETER, so the dropdown was an inert + // second copy of a control that works. + rawSchemas.forEach((filename, raw) { + final id = raw['id'] as String; + final methods = (raw['methods'] as List).cast>(); + if (methods.length < 2) return; + + final parameters = (raw['parameters'] as Map).cast(); + + test('$id: at least one parameter is conditional on the method', () { + final gated = parameters.entries.where((e) { + final ui = (e.value as Map)['ui'] as Map?; + return ((ui?['visibleWhen'] as Map?) ?? const {}) + .containsKey('method'); + }); + + expect(gated, isNotEmpty, + reason: 'the ${methods.length} methods of $id change nothing on ' + 'screen — either gate parameters on the choice or declare one ' + 'method, which suppresses the dropdown'); + }); + + test('$id: a parameter only some methods use says so', () { + // deblock's `quant1` was ungated while DCTFilter, which does not take + // it, was one of the three methods offered — so the panel showed a + // control that could not do anything. + for (final entry in parameters.entries) { + final param = entry.value as Map; + final ui = param['ui'] as Map?; + if (ui?['hidden'] == true) continue; + + final users = methods + .where((m) => (m['parameters'] as List).contains(entry.key)) + .map((m) => m['id'] as String) + .toSet(); + if (users.isEmpty || users.length == methods.length) continue; + + final condition = (ui?['visibleWhen'] as Map?)?['method']; + expect(condition, isNotNull, + reason: '${entry.key} is used by ${users.toList()} but shown for ' + 'every method'); + + final declared = { + ...(condition is List ? condition.cast() : [condition as String]), + }; + expect(declared, users, + reason: '${entry.key} is shown for ${declared.toList()} but used ' + 'by ${users.toList()}'); + } + }); + }); + + rawSchemas.forEach((filename, raw) { + final id = raw['id'] as String; + final sections = + ((raw['ui'] as Map?)?['sections'] as List?) ?? const []; + + test('$id: no section exists only to hold the method', () { + // The panel draws the dropdown itself and always skips the `method` + // parameter, so such a section renders nothing — and now that sections + // print their titles, it would have printed a heading with no content + // had the panel not skipped empty ones. + for (final section in sections.cast>()) { + expect(section['parameters'], isNot(['method']), + reason: '"${section['title']}" in $id renders nothing'); + } + }); + }); + }); + + group('chroma fixes stays grouped by symptom', () { + // The pass covers five unrelated repairs, and the panel renders sections in + // schema order with no headings in simple mode — so this order IS the + // grouping the user sees. Each repair's switch comes first, its everyday + // controls next, and its thresholds in an advanced-only section straight + // after. Reordering these rows shuffles unrelated sliders together again. + final chroma = schemas.firstWhere((s) => s.id == 'chroma_fixes'); + final sections = chroma.ui!.sections!; + + test('the sections are one repair at a time, tuning after each', () { + expect(sections.map((s) => '${s.title}${s.advancedOnly ? " (adv)" : ""}'), + [ + 'Colour alignment', + 'Automatic alignment tuning (adv)', + 'Colour bleeding', + 'Colour bleeding tuning (adv)', + 'Dot crawl', + // Two separate tuning sections, not one: several labels repeat + // between LUTDeCrawl and DeDot ("Colour threshold"), and the + // heading is the only thing that says which filter a slider + // belongs to. + 'Dot crawl tuning (adv)', + 'Dot crawl (across frames) tuning (adv)', + 'Rainbowing', + 'Rainbow tuning (adv)', + 'Rainbow (across frames) tuning (adv)', + 'Chroma combing', + 'Chroma combing tuning (adv)', + ]); + }); + + test('every switch is reachable in simple mode', () { + // A repair whose switch sat in an advanced-only section could be left on + // by a preset — VHS Cleanup enables DeDot, Anime DVD enables two — with + // no way for a simple-mode user to see or clear it. + final simpleParams = sections + .where((s) => !s.advancedOnly) + .expand((s) => s.parameters) + .toSet(); + final switches = chroma.parameters.keys.where((k) => k.startsWith('apply')); + + expect(switches, isNotEmpty); + for (final id in switches) { + expect(simpleParams, contains(id), reason: '$id is only reachable in ' + 'advanced mode, so a preset could enable it invisibly'); + } + }); + + test('every control below a switch is gated on it', () { + // The complaint this schema was rewritten for: sliders belonging to a + // repair that is switched off, sitting on screen with nothing to do. + for (final entry in chroma.parameters.entries) { + if (entry.key == 'enabled' || entry.key.startsWith('apply')) continue; + + final condition = entry.value.ui?.visibleWhen; + expect(condition, isNotNull, + reason: '${entry.key} is always visible — gate it on the ' + 'apply* switch of the repair it belongs to'); + expect(condition!.keys.any((k) => k.startsWith('apply')), isTrue, + reason: '${entry.key} is conditional on ${condition.keys}, none of ' + 'which is a repair switch'); + } + }); + + test('the manual alignment sliders yield to the automatic measurement', () { + // Both are emitted into the script when both are set, and the automatic + // pass runs first — so a manual shift on top of it double-corrects. + // ScriptGenerator drops the manual shift when automatic is on + // (test_150 in worker/tests/filter_integration_test.rs) and + // ParameterConverter.toChromaFixes agrees, so the controls hide rather + // than lie about what the render will do. + for (final id in ['applyChromaShift', 'chromaShiftH', 'chromaShiftV']) { + expect(chroma.parameters[id]!.ui!.visibleWhen!['applyAutoChroma'], false, + reason: '$id must disappear while automatic alignment is on'); + } + }); + + test('nothing carries a second enable checkbox', () { + // Every control here already sits behind an apply* switch. `optional` + // would add a checkbox of its own promising "leave it out and the plugin + // default applies" — which is not what happens: the worker sends a value + // for all of these regardless. + for (final entry in chroma.parameters.entries) { + expect(entry.value.optional ?? false, false, reason: entry.key); + } + }); + }); } diff --git a/app/test/integration_filter_parameters_test.dart b/app/test/integration_filter_parameters_test.dart index fe3d0b5..6395f74 100644 --- a/app/test/integration_filter_parameters_test.dart +++ b/app/test/integration_filter_parameters_test.dart @@ -641,6 +641,49 @@ void main() { print(' PASS'); }, timeout: const Timeout(Duration(minutes: 2))); + // Chroma Fixes offers two ways to put the colour back where it belongs, and + // they are alternatives: the automatic pass measures the misalignment and + // corrects it, and it runs *before* the manual shift in the script, so both + // together shift the picture twice. The panel hides the manual sliders while + // automatic alignment is on (`visibleWhen: {applyAutoChroma: false}` in + // chroma_fixes.json) and the worker drops the manual block to match. This is + // the Dart-side half of test_150: the Rust test builds the struct directly, + // so only this one can catch a `@JsonValue`/serde name drifting apart. + test('chroma_fixes: automatic alignment supersedes the manual shift', + () async { + loadSchema('chroma_fixes'); + + Future scriptFor({required bool auto, required bool manual}) { + return generateScriptViaWorker(buildJob( + testName: 'chroma_align_auto_${auto}_manual_$manual', + chromaFixes: ChromaFixParameters( + enabled: true, + applyAutoChroma: auto, + applyChromaShift: manual, + chromaShiftH: 2.5, + chromaShiftV: -1.0, + ), + )); + } + + final manualOnly = await scriptFor(auto: false, manual: true); + expect(manualOnly, contains('_shift_h = 2.5')); + expect(manualOnly, isNot(contains('_auto_chroma_fix('))); + + final autoOnly = await scriptFor(auto: true, manual: false); + expect(autoOnly, contains('_auto_chroma_fix(')); + expect(autoOnly, isNot(contains('_shift_h ='))); + + final both = await scriptFor(auto: true, manual: true); + expect(both, contains('_auto_chroma_fix('), + reason: 'automatic alignment must still run'); + expect(both, isNot(contains('_shift_h =')), + reason: 'the manual shift must not be applied on top of a measured ' + 'correction'); + + print(' PASS'); + }, timeout: const Timeout(Duration(minutes: 2))); + // --- COLOR CORRECTION (Tweak + Levels) --- test('color_correction: Tweak and Levels params', () async { loadSchema('color_correction'); // confirm schema parses @@ -679,6 +722,59 @@ void main() { print(' PASS'); }, timeout: const Timeout(Duration(minutes: 2))); + // The Levels switch used to be a UI-only flag: the worker emitted the block + // whenever a level differed from its default, so unticking it left the + // adjustment running. And automatic levels measures and places black and + // white before this block, so manual points on top graded an already-graded + // picture. Rust-side twin: test_151. + test('color_correction: levels honour their switch and yield to auto', + () async { + loadSchema('color_correction'); + + Future scriptFor({ + required bool applyLevels, + required bool auto, + double gamma = 1.0, + }) { + return generateScriptViaWorker(buildJob( + testName: 'levels_${applyLevels}_auto_${auto}_g$gamma', + colorCorrection: ColorCorrectionParameters( + enabled: true, + applyLevels: applyLevels, + applyAutoLevels: auto, + inputLow: 16, + inputHigh: 235, + gamma: gamma, + ), + )); + } + + // _auto_levels calls std.Levels internally, so the manual block is + // recognised by its own 8-bit scaling helper. + const manualLevels = 'def _levels_8bit'; + + expect(await scriptFor(applyLevels: true, auto: false), + contains('min_in=_levels_8bit(16)')); + + expect(await scriptFor(applyLevels: false, auto: false), + isNot(contains(manualLevels)), + reason: 'unticking the switch must stop the adjustment'); + + final autoOnly = await scriptFor(applyLevels: true, auto: true); + expect(autoOnly, contains('_auto_levels(')); + expect(autoOnly, isNot(contains(manualLevels)), + reason: 'the measured black and white points supersede the manual ' + 'ones, leaving an identity mapping worth emitting as nothing'); + + final autoWithGamma = + await scriptFor(applyLevels: true, auto: true, gamma: 1.4); + expect(autoWithGamma, contains('_auto_levels(')); + expect(autoWithGamma, contains('gamma=1.4')); + expect(autoWithGamma, isNot(contains('min_in=_levels_8bit(16)'))); + + print(' PASS'); + }, timeout: const Timeout(Duration(minutes: 2))); + // --- DESCRATCH (core.descratch.DeScratch) --- test('descratch: DeScratch params', () async { loadSchema('descratch'); // confirm schema parses diff --git a/app/test/parameter_copy_with_test.dart b/app/test/parameter_copy_with_test.dart new file mode 100644 index 0000000..69742b9 --- /dev/null +++ b/app/test/parameter_copy_with_test.dart @@ -0,0 +1,201 @@ +// Every parameter model's `copyWith()` must carry every field. +// +// `copyWith` is how `ProcessingPipeline.togglePass` rebuilds a pass when its +// switch is flicked, so a field the method forgot is a field that silently +// reverts to its default the moment the user turns the pass off and on. Nothing +// fails: no error, no compile break — the pass simply does something different +// from what the panel says it will. +// +// It had happened five times over, all in fields added during the 2026-08-17 +// build-out. The worst pair were preset-facing: VHS Cleanup turns on DeDot and +// DV Camcorder Tape turns on automatic chroma alignment, and both were thrown +// away by one click. Colour Correction lost all six of its automatic levels / +// automatic white balance fields the same way, SpotLess lost its `method` (so +// RemoveDirt silently reverted to SpotLess), Noise Reduction lost thirteen +// fields including every mClean and TemporalDegrain2 setting, and Subtitles lost +// the burn-in file path. +// +// So this enumerates the models rather than testing the one that broke: it fills +// every field with a non-default value through `fromJson`, calls `copyWith()` +// with no arguments, and requires the result to serialise identically. A model +// added without an entry here is caught by the count assertion at the end. +// +// Run with: flutter test test/parameter_copy_with_test.dart + +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:vapourbox/models/anti_alias_parameters.dart'; +import 'package:vapourbox/models/chroma_denoise_parameters.dart'; +import 'package:vapourbox/models/chroma_fix_parameters.dart'; +import 'package:vapourbox/models/color_correction_parameters.dart'; +import 'package:vapourbox/models/crop_resize_parameters.dart'; +import 'package:vapourbox/models/deband_parameters.dart'; +import 'package:vapourbox/models/deblock_parameters.dart'; +import 'package:vapourbox/models/deflicker_parameters.dart'; +import 'package:vapourbox/models/dehalo_parameters.dart'; +import 'package:vapourbox/models/descratch_parameters.dart'; +import 'package:vapourbox/models/edge_repair_parameters.dart'; +import 'package:vapourbox/models/frame_rate_parameters.dart'; +import 'package:vapourbox/models/geometry_parameters.dart'; +import 'package:vapourbox/models/ghost_removal_parameters.dart'; +import 'package:vapourbox/models/grain_parameters.dart'; +import 'package:vapourbox/models/noise_reduction_parameters.dart'; +import 'package:vapourbox/models/qtgmc_parameters.dart'; +import 'package:vapourbox/models/sharpen_parameters.dart'; +import 'package:vapourbox/models/spotless_parameters.dart'; +import 'package:vapourbox/models/stabilize_parameters.dart'; +import 'package:vapourbox/models/subtitle_parameters.dart'; + +/// One model under test: its defaults, and how to rebuild it from JSON. +class _Model { + final String name; + final Map Function() defaults; + final Map Function(Map) roundTrip; + + const _Model(this.name, this.defaults, this.roundTrip); +} + +/// A value different from [value], of the same JSON type. +/// +/// Strings are left alone: they are enum wire values here, and an invented one +/// would not decode. Every other field gets a value it cannot hold by accident, +/// so a dropped one shows up as its default rather than as a coincidence. +dynamic _perturb(dynamic value) { + if (value is bool) return !value; + if (value is int) return value + 7; + if (value is double) return value + 0.5; + return value; +} + +void main() { + final models = <_Model>[ + _Model('AntiAliasParameters', () => const AntiAliasParameters().toJson(), + (j) => AntiAliasParameters.fromJson(j).copyWith().toJson()), + _Model('ChromaDenoiseParameters', + () => const ChromaDenoiseParameters().toJson(), + (j) => ChromaDenoiseParameters.fromJson(j).copyWith().toJson()), + _Model('ChromaFixParameters', () => const ChromaFixParameters().toJson(), + (j) => ChromaFixParameters.fromJson(j).copyWith().toJson()), + _Model('ColorCorrectionParameters', + () => const ColorCorrectionParameters().toJson(), + (j) => ColorCorrectionParameters.fromJson(j).copyWith().toJson()), + _Model('CropResizeParameters', () => const CropResizeParameters().toJson(), + (j) => CropResizeParameters.fromJson(j).copyWith().toJson()), + _Model('DebandParameters', () => const DebandParameters().toJson(), + (j) => DebandParameters.fromJson(j).copyWith().toJson()), + _Model('DeblockParameters', () => const DeblockParameters().toJson(), + (j) => DeblockParameters.fromJson(j).copyWith().toJson()), + _Model('DeflickerParameters', () => const DeflickerParameters().toJson(), + (j) => DeflickerParameters.fromJson(j).copyWith().toJson()), + _Model('DehaloParameters', () => const DehaloParameters().toJson(), + (j) => DehaloParameters.fromJson(j).copyWith().toJson()), + _Model('DeScratchParameters', () => const DeScratchParameters().toJson(), + (j) => DeScratchParameters.fromJson(j).copyWith().toJson()), + _Model('EdgeRepairParameters', () => const EdgeRepairParameters().toJson(), + (j) => EdgeRepairParameters.fromJson(j).copyWith().toJson()), + _Model('FrameRateParameters', () => const FrameRateParameters().toJson(), + (j) => FrameRateParameters.fromJson(j).copyWith().toJson()), + _Model('GeometryParameters', () => const GeometryParameters().toJson(), + (j) => GeometryParameters.fromJson(j).copyWith().toJson()), + _Model('GhostRemovalParameters', + () => const GhostRemovalParameters().toJson(), + (j) => GhostRemovalParameters.fromJson(j).copyWith().toJson()), + _Model('GrainParameters', () => const GrainParameters().toJson(), + (j) => GrainParameters.fromJson(j).copyWith().toJson()), + _Model('NoiseReductionParameters', + () => const NoiseReductionParameters().toJson(), + (j) => NoiseReductionParameters.fromJson(j).copyWith().toJson()), + _Model('QTGMCParameters', () => const QTGMCParameters().toJson(), + (j) => QTGMCParameters.fromJson(j).copyWith().toJson()), + _Model('SharpenParameters', () => const SharpenParameters().toJson(), + (j) => SharpenParameters.fromJson(j).copyWith().toJson()), + _Model('SpotLessParameters', () => const SpotLessParameters().toJson(), + (j) => SpotLessParameters.fromJson(j).copyWith().toJson()), + _Model('StabilizeParameters', () => const StabilizeParameters().toJson(), + (j) => StabilizeParameters.fromJson(j).copyWith().toJson()), + _Model('SubtitleParameters', () => const SubtitleParameters().toJson(), + (j) => SubtitleParameters.fromJson(j).copyWith().toJson()), + ]; + + group('copyWith() carries every field', () { + for (final model in models) { + test(model.name, () { + final filled = { + for (final entry in model.defaults().entries) + entry.key: _perturb(entry.value), + }; + + final copied = model.roundTrip(Map.from(filled)); + + // Compare field by field so a failure names the field, not the map. + for (final entry in filled.entries) { + expect(copied[entry.key], entry.value, + reason: '${model.name}.copyWith() dropped "${entry.key}" — add it ' + 'to both the parameter list and the constructor call'); + } + }); + } + + test('every parameter model is covered', () { + // The count is the guard: a new model with a forgetful copyWith would + // otherwise be caught by nothing at all. + expect(models.length, 21, + reason: 'a parameter model was added or removed — add it to this ' + 'list and update the count'); + }); + }); + + // The pass above cannot see string or enum fields: a perturbed enum value + // would not decode, so it is left alone and a dropped one still matches. That + // is not hypothetical — SpotLess dropped `method`, which is exactly that + // shape, and Subtitles dropped a plain `String`. So the same rule is also + // checked against the source, where the type does not matter. + group('copyWith() names every field, whatever its type', () { + final files = Directory('lib/models') + .listSync() + .whereType() + .where((f) => f.path.endsWith('_parameters.dart')) + .toList() + ..sort((a, b) => a.path.compareTo(b.path)); + + for (final file in files) { + final name = file.uri.pathSegments.last; + + test(name, () { + final source = file.readAsStringSync(); + + // The parameter class only — enums declared in the same file carry + // `final` members of their own (`displayName`, `value`), and those are + // no business of copyWith. + final classBody = RegExp( + r'^class (\w*Parameters) \{$(.*?)^\}$', + multiLine: true, + dotAll: true, + ).firstMatch(source); + if (classBody == null) return; // dynamic_parameters.dart has no copyWith + + final body = classBody.group(2)!; + final copyWith = RegExp( + r'\w+ copyWith\(\{(.*?)\}\) \{(.*?)\n \}', + dotAll: true, + ).firstMatch(body); + if (copyWith == null) return; + + final fields = RegExp(r'^ final [\w<>,? ]+? (\w+);', multiLine: true) + .allMatches(body) + .map((m) => m.group(1)!) + .toSet(); + final assigned = + RegExp(r'^ (\w+):', multiLine: true) + .allMatches(copyWith.group(2)!) + .map((m) => m.group(1)!) + .toSet(); + + expect(fields.difference(assigned), isEmpty, + reason: 'copyWith() in $name does not carry these fields, so ' + 'toggling the pass resets them to their defaults'); + }); + } + }); +} diff --git a/app/test/pass_advice_test.dart b/app/test/pass_advice_test.dart index bd8d6a3..e7cd3fc 100644 --- a/app/test/pass_advice_test.dart +++ b/app/test/pass_advice_test.dart @@ -9,6 +9,7 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:vapourbox/models/chroma_denoise_parameters.dart'; +import 'package:vapourbox/models/color_correction_parameters.dart'; import 'package:vapourbox/models/deband_parameters.dart'; import 'package:vapourbox/models/geometry_parameters.dart'; import 'package:vapourbox/models/dehalo_parameters.dart'; @@ -205,6 +206,99 @@ void main() { }); }); + group('colour correction set both automatically and by hand', () { + test('says the manual levels points are not used', () { + final advice = adviceFor( + PassType.colorCorrection, + const ProcessingPipeline( + deinterlace: QTGMCParameters(enabled: false), + colorCorrection: ColorCorrectionParameters( + enabled: true, + applyAutoLevels: true, + applyLevels: true, + inputLow: 16, + ), + ), + ); + expect(advice, isNotNull); + expect(advice, contains('automatically')); + }); + + test('says which order white balance happens in', () { + final advice = adviceFor( + PassType.colorCorrection, + const ProcessingPipeline( + deinterlace: QTGMCParameters(enabled: false), + colorCorrection: ColorCorrectionParameters( + enabled: true, + applyAutoWhiteBalance: true, + temperature: 20, + ), + ), + ); + expect(advice, isNotNull); + expect(advice, contains('after')); + }); + + test('silent when only the automatic side is on', () { + expect( + adviceFor( + PassType.colorCorrection, + const ProcessingPipeline( + deinterlace: QTGMCParameters(enabled: false), + colorCorrection: ColorCorrectionParameters( + enabled: true, + applyAutoLevels: true, + applyAutoWhiteBalance: true, + ), + ), + ), + isNull, + ); + }); + }); + + group('a manual chroma shift the automatic measurement has superseded', () { + // The panel hides the manual sliders while automatic alignment is on, so a + // preset that saved both leaves the user nothing to look at — the advice is + // the only place the dropped shift is mentioned. + test('says the hand-set shift is not being used', () { + final advice = adviceFor( + PassType.chromaFixes, + const ProcessingPipeline( + deinterlace: QTGMCParameters(enabled: false), + chromaFixes: ChromaFixParameters( + enabled: true, + applyAutoChroma: true, + applyChromaShift: true, + chromaShiftH: 2.0, + ), + ), + ); + expect(advice, isNotNull); + expect(advice, contains('automatically')); + }); + + test('silent when only one of the two is on', () { + for (final chroma in const [ + ChromaFixParameters(enabled: true, applyAutoChroma: true), + ChromaFixParameters( + enabled: true, applyChromaShift: true, chromaShiftH: 2.0), + ]) { + expect( + adviceFor( + PassType.chromaFixes, + ProcessingPipeline( + deinterlace: const QTGMCParameters(enabled: false), + chromaFixes: chroma, + ), + ), + isNull, + ); + } + }); + }); + group('rotating interlaced material', () { test('warns when a quarter turn runs with deinterlacing off', () { final advice = adviceFor( diff --git a/app/test/preset_visibility_test.dart b/app/test/preset_visibility_test.dart new file mode 100644 index 0000000..ec0aca5 --- /dev/null +++ b/app/test/preset_visibility_test.dart @@ -0,0 +1,193 @@ +// Every setting a built-in preset turns on must be a setting the user can find. +// +// A preset is the main way settings arrive without anyone touching a control, so +// it is also the main way a *hidden* setting arrives. The panel hides a parameter +// whose `visibleWhen` is unsatisfied and skips one that no section lists, and +// neither reports anything — so a preset can enable a filter the user cannot see, +// cannot adjust and cannot switch off, while the pass summary says nothing about +// it. That is exactly what the 2026-08-18 review found in the panels themselves, +// and the presets are the other half of it. +// +// The check walks each built-in preset's enabled passes through the real +// converter — the same call the settings panel makes — and asserts that anything +// differing from the schema default is actually on screen for that preset's own +// state. +// +// Two exemptions, both deliberate: +// +// * A value hidden **only** by a `method` condition. Carrying another method's +// value is normal and sometimes intended: `builtin-fast` selects Bwdif and +// still sets a QTGMC preset, so that switching the method in the UI lands +// somewhere sensible. Nothing is applied, so nothing is misreported. +// * The allowlist below, for values in advanced-only sections. Those are +// visible, just not in simple mode, and bundling expert settings is what a +// quality tier is for — but each one is named here so that adding another is +// a decision rather than a drift. +// +// Run with: flutter test test/preset_visibility_test.dart + +import 'dart:convert'; +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:vapourbox/models/filter_schema.dart'; +import 'package:vapourbox/models/parameter_converter.dart'; +import 'package:vapourbox/models/processing_pipeline.dart'; +import 'package:vapourbox/models/processing_preset.dart'; + +/// Values a built-in preset may set that only appear in advanced mode. +/// +/// All three are QTGMC's, in a pass whose own control (the QTGMC preset) is +/// visible and whose summary names it — so the user can see *that* the tier is +/// doing something expert, just not each knob. +const _advancedAllowed = { + 'deinterlace.sourceMatch', // High Quality: the fidelity mode it is named for + 'deinterlace.lossless', // ditto, and only meaningful alongside sourceMatch + 'deinterlace.chromaUpsampleFix', // DV Camcorder: 4:2:0 chroma is its whole problem +}; + +/// The pass-to-schema mapping the settings panel uses. +/// Mirrors `PassSettingsInline._getFilterId`. +const _filterIds = { + PassType.deinterlace: 'deinterlace', + PassType.descratch: 'descratch', + PassType.spotless: 'spotless', + PassType.noiseReduction: 'noise_reduction', + PassType.chromaDenoise: 'chroma_denoise', + PassType.dehalo: 'dehalo', + PassType.deblock: 'deblock', + PassType.deband: 'deband', + PassType.sharpen: 'sharpen', + PassType.antiAlias: 'anti_alias', + PassType.stabilize: 'stabilize', + PassType.geometry: 'geometry', + PassType.chromaFixes: 'chroma_fixes', + PassType.colorCorrection: 'color_correction', + PassType.cropResize: 'crop_resize', + PassType.grain: 'grain', + PassType.subtitles: 'subtitles', + PassType.edgeRepair: 'edge_repair', + PassType.deflicker: 'deflicker', + PassType.ghostRemoval: 'ghost_removal', + PassType.frameRate: 'frame_rate', +}; + +FilterSchema _schema(String id) => FilterSchema.fromJson( + jsonDecode(File('assets/filters/core/$id.json').readAsStringSync()) + as Map, + ); + +/// Schema defaults and model defaults are asserted equal elsewhere +/// (`schema_converter_integration_test`), so either side can stand for "the user +/// did not choose this". Numbers arrive as int or double depending on the route. +bool _isDefault(dynamic value, dynamic defaultValue) => + value == defaultValue || + (value is num && defaultValue is num && + value.toDouble() == defaultValue.toDouble()) || + value.toString() == defaultValue.toString(); + +void main() { + final schemas = { + for (final id in _filterIds.values) id: _schema(id), + }; + + for (final preset in ProcessingPreset.builtInPresets()) { + group(preset.id, () { + final dynamic_ = ParameterConverter.fromPipeline(preset.pipeline); + + for (final pass in preset.pipeline.enabledPasses) { + final filterId = _filterIds[pass]; + + test('${filterId ?? pass.name}: every setting it changes is reachable', + () { + expect(filterId, isNotNull, + reason: '$pass has no schema, so its settings have no UI at all'); + final schema = schemas[filterId]!; + final params = dynamic_.get(filterId!); + expect(params, isNotNull, + reason: 'the converter does not know $filterId, so the panel ' + 'would show schema defaults instead of the preset'); + + final sectioned = { + for (final s in schema.ui?.sections ?? const []) + ...s.parameters, + }; + final advancedOnly = { + for (final s in schema.ui?.sections ?? const []) + if (s.advancedOnly) ...s.parameters, + }; + + for (final entry in params!.values.entries) { + final key = entry.key; + final definition = schema.parameters[key]; + if (definition == null) continue; // worker-only field + if (key == 'enabled' || key == 'method') continue; + if (definition.ui?.hidden == true) continue; // owned elsewhere in the UI + if (entry.value == null) continue; + if (_isDefault(entry.value, definition.defaultValue)) continue; + + expect(sectioned, contains(key), + reason: '$filterId.$key is set to ${entry.value} but no section ' + 'lists it, so the panel never renders it'); + + final condition = definition.ui?.visibleWhen ?? const {}; + final hiddenOnlyByMethod = + condition.keys.isNotEmpty && + condition.keys.every((k) => k == 'method'); + if (!hiddenOnlyByMethod) { + for (final gate in condition.entries) { + final current = params.values[gate.key]; + final expected = gate.value; + final satisfied = expected is List + ? expected.contains(current) + : current == expected; + expect(satisfied, isTrue, + reason: '$filterId.$key is set to ${entry.value} but the ' + 'panel hides it while ${gate.key} is $current — the ' + 'preset would apply a setting with no control on screen'); + } + } + + if (advancedOnly.contains(key)) { + expect(_advancedAllowed, contains('$filterId.$key'), + reason: '$filterId.$key is set to ${entry.value} but only ' + 'appears in advanced mode. Either move the control into a ' + 'simple-mode section or add it to _advancedAllowed with a ' + 'reason'); + } + } + }); + } + }); + } + + test('the advanced allowlist has no stale entries', () { + // A control moved out of an advanced section, or a preset that stopped + // setting it, should shrink this list rather than leave it asserting + // nothing. + final used = {}; + for (final preset in ProcessingPreset.builtInPresets()) { + final dynamic_ = ParameterConverter.fromPipeline(preset.pipeline); + for (final pass in preset.pipeline.enabledPasses) { + final filterId = _filterIds[pass]; + if (filterId == null) continue; + final schema = schemas[filterId]!; + final params = dynamic_.get(filterId); + if (params == null) continue; + final advancedOnly = { + for (final s in schema.ui?.sections ?? const []) + if (s.advancedOnly) ...s.parameters, + }; + for (final entry in params.values.entries) { + final definition = schema.parameters[entry.key]; + if (definition == null || definition.ui?.hidden == true) continue; + if (entry.value == null || + _isDefault(entry.value, definition.defaultValue)) continue; + if (advancedOnly.contains(entry.key)) used.add('$filterId.${entry.key}'); + } + } + } + + expect(used, _advancedAllowed); + }); +} diff --git a/app/test/processing_preset_test.dart b/app/test/processing_preset_test.dart index 2d9434e..b9bfb32 100644 --- a/app/test/processing_preset_test.dart +++ b/app/test/processing_preset_test.dart @@ -270,6 +270,27 @@ void main() { }); }); + group('switching a pass off and on keeps what the preset configured', () { + // togglePass rebuilds the pass through copyWith, so a copyWith missing a + // field silently discards it. ChromaFixParameters.copyWith never took + // applyAutoChroma or applyDedot, which meant flicking the Chroma Fixes + // switch threw away the DeDot that VHS Cleanup turns on and the automatic + // alignment that DV Camcorder Tape turns on — no error, and the pass + // afterwards just did nothing. + test('chroma fixes survives a round trip through togglePass', () { + for (final id in ['builtin-vhs-cleanup', 'builtin-dv-camcorder', 'builtin-anime-dvd']) { + final before = byId(id).pipeline; + final after = before + .togglePass(PassType.chromaFixes, false) + .togglePass(PassType.chromaFixes, true); + + expect(after.chromaFixes.toJson(), before.chromaFixes.toJson(), + reason: '$id lost chroma-fix settings by switching the pass off ' + 'and on again'); + } + }); + }); + group('QTGMC presets are ordered fast to slow', () { // The source presets pick a QTGMC preset by name, so this pins the meaning // of those names not drifting underneath them. diff --git a/docs/FILTER_SCHEMA.md b/docs/FILTER_SCHEMA.md index 7440f54..46131a8 100644 --- a/docs/FILTER_SCHEMA.md +++ b/docs/FILTER_SCHEMA.md @@ -81,6 +81,15 @@ parameters that apply to it. | `parameters` | array | **Yes** | Parameter ids this method uses | | `advancedOnly` | boolean | No (`false`) | Method is only offered in advanced mode | +> **A method must gate something, or it should not exist.** The listed +> `parameters` do **not** drive the panel when the schema declares `ui.sections` +> — sections take priority — so a method only changes what the user sees through +> `visibleWhen: {"method": [...]}` on the parameters themselves, and only changes +> the render if the worker has a matching field to switch on. `color_correction` +> declared two methods with neither: the dropdown rendered, both groups of +> controls stayed on screen, and the converter hardcoded one value the worker +> never read. One method means no dropdown at all, which is the honest result. + ### `advancedOnly` on a method This is how a filter offers a short list to everyone and the full set to someone @@ -189,9 +198,26 @@ method (above). "visibleWhen": { "method": ["dehalo_alpha", "fine_dehalo"] } // any of these "visibleWhen": { "method": "standard" } // single value "visibleWhen": { "enabled": true, "method": "advanced" } // all must match +"visibleWhen": { "applyAutoChroma": false } // booleans too ``` -Multiple keys are ANDed; a list of values for one key is ORed. +Multiple keys are ANDed; a list of values for one key is ORed. `method` is read +from the values map like any other key, so a condition on the selected method +works the same way as one on a checkbox. + +> **It belongs at `parameters..ui.visibleWhen` and nowhere else.** +> `ParameterDefinition` doesn't declare the key, so a copy written one level up +> is dropped at parse time — no error, and the control is simply always visible. +> **Eight of the shipped schemas had it in the wrong place** (35 parameters and +> 12 sections), which is how Chroma Fixes came to show its automatic-alignment +> sliders with automatic alignment switched off, and Noise Reduction its mClean +> knobs under SMDegrain. Fixed 2026-08-18 and now linted by +> `app/test/filter_schema_curation_test.dart`. +> +> The same test asserts the opposite failure: a condition naming a parameter +> that doesn't exist, or a `method` condition naming a method the filter doesn't +> offer, can *never* be satisfied — so the control it guards is invisible +> forever, which is the worse of the two silent failures. ## Parameter Presets @@ -244,8 +270,27 @@ A dropdown that writes several parameters at once — distinct from the top-leve | `expanded` | boolean | `true` | Initially expanded | | `advancedOnly` | boolean | `false` | Only in advanced mode | -A section has **no `visibleWhen`** — `UiSection` doesn't declare one, so putting it -there does nothing. Hide the individual parameters instead. +A section has **no `visibleWhen`** — `UiSection` doesn't declare one, so putting +it there does nothing. Put the condition on every parameter in the section +instead; `filter_schema_curation_test.dart` fails if a section carries one. + +**A section is a heading, but only where there is more than one.** +`DynamicFilterPanelCompact` prints `title` above the section's controls whenever +the schema declares two or more sections, in both modes; an `advancedOnly` one is +printed in the accent colour, so it stays obvious which controls appeared because +advanced mode is on. A schema with a **single** section gets no heading at all — +there is nothing to tell it apart from, and most of those call it "Settings". + +Order still carries the structure, because a heading is all you get — there is no +indent, box or rule. Put the switch that turns a feature on first, the controls it +reveals next, and its thresholds in an `advancedOnly` section immediately after. +`chroma_fixes.json` is the worked example (a switch per repair, and a tuning +section per filter directly after it). + +> Until 2026-08-18 the heading was printed **only** for an `advancedOnly` section +> in advanced mode, so every ordinary section ran together into one flat list and +> a `title` was, in practice, a comment. Don't assume a section groups anything +> visually beyond its heading. ## Code Template @@ -281,3 +326,15 @@ Read a real one rather than starting from this page: - **`deinterlace.json`** (2242 lines) — every QTGMC parameter; the extreme case. - **`spotless.json`** (112 lines) — near-minimal, a good starting skeleton. - **`descratch.json`** — the only user of `maxBitDepth`. +- **`chroma_fixes.json`** — one pass holding five unrelated repairs: the model + for progressive disclosure (a switch per repair, controls gated on it, an + `advancedOnly` tuning section per repair) and for two controls that are + alternatives rather than a pair (`applyAutoChroma` hides the manual sliders, + and `ChromaFixParameters::effective_apply_chroma_shift` keeps the generated + script agreeing with what the panel shows). +- **`color_correction.json`** — an adjustment offered both automatically and by + hand belongs in **one** section with the automatic control first, not in a + separate "Automatic" group. It also shows the two ways that pairing can go: + automatic levels hides the manual input/output points it supersedes but leaves + gamma, which it never touches, while automatic white balance leaves temperature + and tint alone because an offset still means what it says after a correction. diff --git a/worker/src/models/chroma_fix_parameters.rs b/worker/src/models/chroma_fix_parameters.rs index f81c58f..1fc62ac 100644 --- a/worker/src/models/chroma_fix_parameters.rs +++ b/worker/src/models/chroma_fix_parameters.rs @@ -247,6 +247,26 @@ mod tests { assert!(!params.apply_vinverse); } + #[test] + fn test_automatic_alignment_supersedes_the_manual_shift() { + let mut params = ChromaFixParameters { + apply_chroma_shift: true, + chroma_shift_h: 2.0, + ..ChromaFixParameters::default() + }; + assert!(params.effective_apply_chroma_shift()); + + params.apply_auto_chroma = true; + assert!( + !params.effective_apply_chroma_shift(), + "the automatic pass already corrects the shift; applying the manual \ + one on top would double-correct it" + ); + // The user's own setting survives, so unticking automatic restores it. + assert!(params.apply_chroma_shift); + assert_eq!(params.chroma_shift_h, 2.0); + } + #[test] fn test_serialization() { let params = ChromaFixParameters::default(); @@ -261,4 +281,21 @@ impl ChromaFixParameters { pub fn bifrost_effective_variation(&self) -> i32 { self.bifrost_variation.clamp(0, 10) } + + /// Whether the manual Y/C delay shift reaches the script. + /// + /// Automatic alignment measures the misalignment and corrects it, and its + /// block runs *before* the manual shift — so with both set the picture is + /// shifted twice, the second time by a number the user guessed on top of a + /// correction that was already measured. The automatic measurement wins: + /// the panel hides the manual controls while it is on + /// (`chroma_fixes.json`, `visibleWhen: {applyAutoChroma: false}`) and this + /// keeps the generated script agreeing with what the panel shows. + /// + /// It deliberately does not clear `apply_chroma_shift` in the job, so + /// turning automatic alignment off restores what the user set by hand. + /// `ChromaFixParameters.effectiveApplyChromaShift` is the Dart twin. + pub fn effective_apply_chroma_shift(&self) -> bool { + self.apply_chroma_shift && !self.apply_auto_chroma + } } diff --git a/worker/src/models/color_correction_parameters.rs b/worker/src/models/color_correction_parameters.rs index 4d404e4..0f7f5c3 100644 --- a/worker/src/models/color_correction_parameters.rs +++ b/worker/src/models/color_correction_parameters.rs @@ -140,6 +140,15 @@ pub struct ColorCorrectionParameters { pub tint: f64, } +/// The four input/output points of a levels adjustment, in 8-bit UI units. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct LevelsPoints { + pub input_low: i32, + pub input_high: i32, + pub output_low: i32, + pub output_high: i32, +} + /// Chroma levels shifted per unit of temperature/tint, at 8-bit scale. /// /// A full-scale slider therefore moves chroma by 25 levels — a strong but still @@ -259,6 +268,52 @@ mod tests { } impl ColorCorrectionParameters { + /// The input/output points the levels block actually applies. + /// + /// Automatic levels measures the picture and places black and white itself, + /// and it runs **before** this block — so a manual mapping on top is applied + /// to an already-graded picture, where the numbers no longer mean what their + /// labels say ("input black = 16" was measured against the original). The + /// panel hides those four sliders while automatic levels is on + /// (`visibleWhen: {"applyAutoLevels": false}` in color_correction.json) and + /// this is what keeps the script agreeing with it. + /// + /// Gamma is deliberately **not** dropped: automatic levels does not touch + /// the midtones, so it is the one levels control that still means something + /// afterwards — and the only place in the app to reach it. + pub fn effective_levels_points(&self) -> LevelsPoints { + if self.apply_auto_levels { + LevelsPoints { input_low: 0, input_high: 255, output_low: 0, output_high: 255 } + } else { + LevelsPoints { + input_low: self.input_low, + input_high: self.input_high, + output_low: self.output_low, + output_high: self.output_high, + } + } + } + + /// Whether the manual levels block runs at all. + /// + /// `apply_levels` is the switch in the panel, and it used to be **ignored + /// here**: the block was emitted whenever any level differed from its + /// default, so unticking the switch left the adjustment running with no + /// control on screen that could turn it off. The switch decides now, and it + /// still needs something to do — an identity mapping is emitted as nothing + /// rather than as a no-op filter. + pub fn effective_apply_levels(&self) -> bool { + if !self.apply_levels { + return false; + } + let p = self.effective_levels_points(); + p.input_low != 0 + || p.input_high != 255 + || p.output_low != 0 + || p.output_high != 255 + || (self.gamma - 1.0).abs() > 0.001 + } + /// The black point actually passed to SmoothLevels. /// /// havsfunc builds its lookup table over the whole `0..peak` domain and @@ -275,7 +330,7 @@ impl ColorCorrectionParameters { if (self.gamma - 1.0).abs() > f64::EPSILON { 0 } else { - self.input_low + self.effective_levels_points().input_low } } @@ -284,7 +339,7 @@ impl ColorCorrectionParameters { pub fn smooth_levels_drops_black_point(&self) -> bool { self.smooth_levels && self.apply_levels - && self.input_low > 0 + && self.effective_levels_points().input_low > 0 && (self.gamma - 1.0).abs() > f64::EPSILON } } diff --git a/worker/src/script_generator.rs b/worker/src/script_generator.rs index 928a73f..c3d703f 100644 --- a/worker/src/script_generator.rs +++ b/worker/src/script_generator.rs @@ -1561,8 +1561,13 @@ impl ScriptGenerator { script = script.replace("{{#CHROMA_FIXES}}", ""); script = script.replace("{{/CHROMA_FIXES}}", ""); - // Chroma Shift (Y/C Delay) - if chroma.apply_chroma_shift && (chroma.chroma_shift_h != 0.0 || chroma.chroma_shift_v != 0.0) { + // Chroma Shift (Y/C Delay). Skipped when automatic alignment is on — + // that block above has already measured and applied the shift, so a + // manual one on top double-corrects. See + // ChromaFixParameters::effective_apply_chroma_shift. + if chroma.effective_apply_chroma_shift() + && (chroma.chroma_shift_h != 0.0 || chroma.chroma_shift_v != 0.0) + { script = script.replace("{{#CHROMA_SHIFT}}", ""); script = script.replace("{{/CHROMA_SHIFT}}", ""); script = process_optional_double("CHROMA_SHIFT_H", Some(chroma.chroma_shift_h), script); @@ -1724,21 +1729,21 @@ impl ScriptGenerator { script = remove_block("{{#COLOR_TWEAK}}", "{{/COLOR_TWEAK}}", script); } - // Levels - let has_levels = color.input_low != 0 - || color.input_high != 255 - || color.output_low != 0 - || color.output_high != 255 - || (color.gamma - 1.0).abs() > 0.001; + // Levels. The switch decides whether this runs, and automatic + // levels supersedes the input/output points — see + // ColorCorrectionParameters::{effective_apply_levels, + // effective_levels_points}. + let levels = color.effective_levels_points(); + let has_levels = color.effective_apply_levels(); if has_levels && !color.smooth_levels { script = script.replace("{{#COLOR_LEVELS}}", ""); script = script.replace("{{/COLOR_LEVELS}}", ""); script = remove_block("{{#COLOR_SMOOTH_LEVELS}}", "{{/COLOR_SMOOTH_LEVELS}}", script); - script = process_optional_int("LEVELS_INPUT_LOW", if color.input_low != 0 { Some(color.input_low) } else { None }, script); - script = process_optional_int("LEVELS_INPUT_HIGH", if color.input_high != 255 { Some(color.input_high) } else { None }, script); - script = process_optional_int("LEVELS_OUTPUT_LOW", if color.output_low != 0 { Some(color.output_low) } else { None }, script); - script = process_optional_int("LEVELS_OUTPUT_HIGH", if color.output_high != 255 { Some(color.output_high) } else { None }, script); + script = process_optional_int("LEVELS_INPUT_LOW", if levels.input_low != 0 { Some(levels.input_low) } else { None }, script); + script = process_optional_int("LEVELS_INPUT_HIGH", if levels.input_high != 255 { Some(levels.input_high) } else { None }, script); + script = process_optional_int("LEVELS_OUTPUT_LOW", if levels.output_low != 0 { Some(levels.output_low) } else { None }, script); + script = process_optional_int("LEVELS_OUTPUT_HIGH", if levels.output_high != 255 { Some(levels.output_high) } else { None }, script); script = process_optional_double("LEVELS_GAMMA", if (color.gamma - 1.0).abs() > 0.001 { Some(color.gamma) } else { None }, script); } else if has_levels { // SmoothLevels reuses the _levels_8bit() helper defined in the @@ -1761,9 +1766,9 @@ impl ScriptGenerator { "{{SMOOTH_INPUT_LOW}}", &color.smooth_levels_input_low().to_string(), ); - script = script.replace("{{SMOOTH_INPUT_HIGH}}", &color.input_high.to_string()); - script = script.replace("{{SMOOTH_OUTPUT_LOW}}", &color.output_low.to_string()); - script = script.replace("{{SMOOTH_OUTPUT_HIGH}}", &color.output_high.to_string()); + script = script.replace("{{SMOOTH_INPUT_HIGH}}", &levels.input_high.to_string()); + script = script.replace("{{SMOOTH_OUTPUT_LOW}}", &levels.output_low.to_string()); + script = script.replace("{{SMOOTH_OUTPUT_HIGH}}", &levels.output_high.to_string()); script = script.replace("{{SMOOTH_GAMMA}}", &format_double(color.gamma)); // -2 is havsfunc's own default and the best-measured setting. script = script.replace("{{SMOOTH_MODE}}", "-2"); diff --git a/worker/tests/filter_integration_test.rs b/worker/tests/filter_integration_test.rs index 40d6c45..0163b8b 100644 --- a/worker/tests/filter_integration_test.rs +++ b/worker/tests/filter_integration_test.rs @@ -5453,3 +5453,130 @@ fn test_149_every_noise_reduction_method_emits_its_filter() { assert!(!script.contains(call), "{call} survived a disabled pass"); } } + +#[test] +fn test_150_automatic_chroma_alignment_supersedes_the_manual_shift() { + // Both blocks used to be emitted when both were set, with the automatic one + // running first — so the picture was shifted by a measured amount and then + // again by a guessed one, silently. The panel now hides the manual sliders + // while automatic alignment is on, and this keeps the generated script + // agreeing with what the panel shows. + create_output_dir(); + + let build = |auto: bool, manual: bool| { + let mut job = create_base_job("test_150_chroma_alignment"); + job.qtgmc_parameters.enabled = false; + job.processing_pipeline = Some(ProcessingPipeline { + deinterlace: QTGMCParameters { enabled: false, ..Default::default() }, + chroma_fixes: ChromaFixParameters { + enabled: true, + apply_auto_chroma: auto, + apply_chroma_shift: manual, + chroma_shift_h: 2.5, + chroma_shift_v: -1.0, + ..Default::default() + }, + ..ProcessingPipeline::default() + }); + script_text(&job) + }; + + // The manual shift is recognisable by the per-plane ShufflePlanes/Spline36 + // pair the CHROMA_SHIFT block builds. + let manual_only = build(false, true); + assert!(!manual_only.contains("_auto_chroma_fix(")); + assert!(manual_only.contains("_shift_h = 2.5")); + + let auto_only = build(true, false); + assert!(auto_only.contains("_auto_chroma_fix(")); + assert!(!auto_only.contains("_shift_h =")); + + let both = build(true, true); + assert!( + both.contains("_auto_chroma_fix("), + "automatic alignment must still run" + ); + assert!( + !both.contains("_shift_h ="), + "the manual shift must be dropped when automatic alignment is on, or the \ + measured correction is applied twice" + ); + + let neither = build(false, false); + assert!(!neither.contains("_auto_chroma_fix(")); + assert!(!neither.contains("_shift_h =")); +} + +#[test] +fn test_151_levels_honour_their_switch_and_yield_to_automatic_levels() { + // Two faults in the same block, both silent. + // + // `apply_levels` — the panel's "Set levels by hand" switch — was never read + // here: the block was emitted whenever any level differed from its default, + // so unticking the switch left the adjustment running with nothing on screen + // that could stop it. + // + // And automatic levels measures the picture and places black and white + // itself, running *before* this block, so manual input/output points on top + // grade an already-graded picture with numbers that were measured against + // the original. The panel hides them; the script drops them. Gamma survives, + // because automatic levels never touches the midtones and this is the only + // place to reach it. + create_output_dir(); + + let build = |apply_levels: bool, auto: bool, gamma: f64| { + let mut job = create_base_job("test_151_levels"); + job.qtgmc_parameters.enabled = false; + job.processing_pipeline = Some(ProcessingPipeline { + deinterlace: QTGMCParameters { enabled: false, ..Default::default() }, + color_correction: ColorCorrectionParameters { + enabled: true, + apply_levels, + apply_auto_levels: auto, + input_low: 16, + input_high: 235, + output_low: 0, + output_high: 255, + gamma, + ..ColorCorrectionParameters::default() + }, + ..ProcessingPipeline::default() + }); + script_text(&job) + }; + + // `_auto_levels` calls std.Levels internally, so the manual block is + // recognised by its own 8-bit scaling helper rather than by the call. + const MANUAL_LEVELS: &str = "def _levels_8bit"; + + let on = build(true, false, 1.0); + assert!(on.contains(MANUAL_LEVELS)); + assert!(on.contains("min_in=_levels_8bit(16)")); + + let off = build(false, false, 1.0); + assert!( + !off.contains(MANUAL_LEVELS), + "unticking the switch must actually stop the adjustment" + ); + + // Automatic levels on: its own block runs, the manual points do not, and a + // manual levels block is emitted only if gamma gives it something to do. + let auto_no_gamma = build(true, true, 1.0); + assert!(auto_no_gamma.contains("_auto_levels(")); + assert!( + !auto_no_gamma.contains(MANUAL_LEVELS), + "with black and white measured, an identity mapping is nothing to emit" + ); + + let auto_with_gamma = build(true, true, 1.4); + assert!(auto_with_gamma.contains("_auto_levels(")); + assert!(auto_with_gamma.contains(MANUAL_LEVELS)); + assert!( + auto_with_gamma.contains("gamma=1.4"), + "gamma is the one levels control automatic levels does not supersede" + ); + assert!( + !auto_with_gamma.contains("min_in=_levels_8bit(16)"), + "the manual input point must not be applied on top of the measured one" + ); +}