fix(pipeline): run Sharpen and the crop in their documented order (#109)

CLAUDE.md

@@ -319,6 +319,9 @@ Adding a filter touches many files. Missing any step causes silent failures (fil
   `pass_list_stages_test.dart` fails if one is missed. Stages are labels over the
   **existing pipeline order**, so a pass goes in the stage its position already
   falls in; never reorder rows to suit a grouping.
+  The README's "The filter pipeline" table is the user-facing statement of the
+  same order — add the pass's row at its position there too;
+  `readme_pipeline_order_test.dart` fails if the table and `stages` disagree.
 - `app/lib/views/pass_list/pass_list_item.dart` — add icon in `_getIconForPass()`
 - `app/lib/views/pass_settings/pass_settings_inline.dart` — add case in `_getFilterId()`
 - `app/lib/viewmodels/main_viewmodel.dart` — add case in BOTH `_convertToParams()` AND `_updatePipelineFromDynamic()`

README.md

@@ -93,37 +93,44 @@ GPU-accelerated deinterlacing (NNEDI3CL) needs your GPU's OpenCL driver installe
 
 ## The filter pipeline
 
-Twenty-one filters, each switchable independently, applied in a fixed order. Most sources need none or a few.
-
-| Filter | What it addresses |
-|--------|-------------------|
-| **Deinterlace** | Comb-like jagged edges on moving objects. QTGMC for interlaced video, IVTC to recover the original film frames from telecined DVD, or Bwdif when you want it done in a fraction of the time. |
-| **Edge Repair** | The dirty rows and columns at the very edge of a tape capture — rebuilt from the picture just inside, instead of cropped away. |
-| **Ghost Removal** | A faint second copy of the picture shifted sideways, left behind by an aerial or a long cable run. |
-| **Deflicker** | Brightness pulsing between frames, which is what scanned cine film almost always has. |
-| **DeScratch** | Vertical scratch lines on scanned film. |
-| **SpotLess** | Dust, dirt and single-frame specks. RemoveDirt is the faster choice — around six times the speed for about 60% of the removal. |
-| **Noise Reduction** | Grain and video noise across the whole frame. Motion-compensated by default, with mClean as a gentler alternative that keeps more detail; DFTTest, FFT3DFilter, TTempSmooth, FluxSmooth, STPresso, TemporalDegrain2 and a large-window median are available under advanced options for noise the default handles badly. |
-| **Chroma Denoise** | Blotchy, smeared color — common on VHS captures and old camcorder footage. Leaves luma detail untouched. |
-| **Dehalo** | Bright outlines around edges, ringing, and residual ghosting left by a deinterlacer. HQDeringmod targets ringing specifically. |
-| **Deblock** | Square blocking from heavy compression, and the ringing around edges that comes with it. |
-| **Deband** | Visible steps in gradients and skies. |
-| **Anti-Aliasing** | Stair-stepping on diagonal edges, left by deinterlacing or upscaling. Runs before sharpening, which would otherwise make the steps more visible. |
-| **Stabilize** | Shake and weave — telecine wobble, jittery film scans, handheld footage. Runs last before cropping, so a small crop removes the edges it exposes. |
-| **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** | 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 — plus bars in a colour of your choice to bring a cropped picture back to an exact frame size (720×576 for PAL DVD, 720×480 for NTSC) without rescaling it. |
-| **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 — written alongside the video as `.srt`, embedded as a selectable track, burnt into the picture, or a combination. |
+Twenty-one filters, each switchable independently. Most sources need none or a few. They always run in the order below, top to bottom, whichever ones are switched on — the same order the list in the app shows them in. The order cannot be rearranged.
+
+| # | Filter | What it addresses |
+|---|--------|-------------------|
+| 1 | **Deinterlace** | Comb-like jagged edges on moving objects. QTGMC for interlaced video, IVTC to recover the original film frames from telecined DVD, or Bwdif when you want it done in a fraction of the time. |
+| 2 | **Edge Repair** | The dirty rows and columns at the very edge of a tape capture — rebuilt from the picture just inside, instead of cropped away. |
+| 3 | **Ghost Removal** | A faint second copy of the picture shifted sideways, left behind by an aerial or a long cable run. |
+| 4 | **Deflicker** | Brightness pulsing between frames, which is what scanned cine film almost always has. |
+| 5 | **DeScratch** | Vertical scratch lines on scanned film. |
+| 6 | **SpotLess** | Dust, dirt and single-frame specks. RemoveDirt is the faster choice — around six times the speed for about 60% of the removal. |
+| 7 | **Noise Reduction** | Grain and video noise across the whole frame. Motion-compensated by default, with mClean as a gentler alternative that keeps more detail; DFTTest, FFT3DFilter, TTempSmooth, FluxSmooth, STPresso, TemporalDegrain2 and a large-window median are available under advanced options for noise the default handles badly. |
+| 8 | **Chroma Denoise** | Blotchy, smeared color — common on VHS captures and old camcorder footage. Leaves luma detail untouched. |
+| 9 | **Dehalo** | Bright outlines around edges, ringing, and residual ghosting left by a deinterlacer. HQDeringmod targets ringing specifically. |
+| 10 | **Deblock** | Square blocking from heavy compression, and the ringing around edges that comes with it. |
+| 11 | **Deband** | Visible steps in gradients and skies. |
+| 12 | **Anti-Aliasing** | Stair-stepping on diagonal edges, left by deinterlacing or upscaling. Runs before sharpening, which would otherwise make the steps more visible. |
+| 13 | **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. |
+| 14 | **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. |
+| 15 | **Stabilize** | Shake and weave — telecine wobble, jittery film scans, handheld footage. Runs last before cropping, so a small crop removes the edges it exposes. |
+| 16 | **Rotate / Flip** | Footage shot sideways, mirrored captures, scans that came off the scanner the wrong way round. |
+| 17 | **Crop & Resize** | Trimming overscan, scaling, and edge-directed upscaling — plus bars in a colour of your choice to bring a cropped picture back to an exact frame size (720×576 for PAL DVD, 720×480 for NTSC) without rescaling it. |
+| 18 | **Sharpen** | Soft sources needing edge and fine detail recovery. aWarpSharp2 sharpens by warping edges instead of raising contrast, so it adds no halos. Runs after the resize, on the picture that is actually delivered, and before any added grain. |
+| 19 | **Film Grain** | Grain added back after denoising, so the picture is not left plastic — and to hide banding in skies and fades. |
+| 20 | **Frame Rate** | Converting between PAL and NTSC rates, for a tape that was already converted once and now plays at the wrong speed. |
+| 21 | **Subtitles** | Whisper AI speech-to-text — written alongside the video as `.srt`, embedded as a selectable track, burnt into the picture, or a combination. |
+
+A few things about the order are worth knowing:
+
+- **Crop & Resize is one step, cropping first.** It runs after Stabilize and Rotate / Flip, so a small crop removes the thin edges stabilising exposes, and the four crop sides are the sides of the picture after any rotation. Everything before it works on the uncropped frame.
+- **Clean up, then frame, then finish.** Damage and noise are removed first, colour is corrected next, the picture is stabilised, turned and resized, and only then sharpened. Sharpening earlier would have its work softened again by the resize.
+- **Grain goes on after sharpening**, so it is neither sharpened into grit nor resampled away, and the frame rate is converted last of all, so every other filter works on real frames rather than invented ones.
+- **After the filters:** any Custom VapourSynth code runs next, then the conversion to the output colour format, then added borders — last, so that nothing touches the bars. Subtitles are transcribed from the source and added once the video has been encoded.
 
 Each filter leads with a plain-language summary and a **More** expander describing what it does and when it's the right choice, so the settings can be understood in place rather than looked up elsewhere.
 
 The list also reacts to the file you dropped in. Filters that match what was detected in your source are marked **Suggested** with the reason — "source is hard telecine (3:2 pulldown)", "anamorphic source (10:11) — check pixel aspect" — and ones that can't apply say so, such as deinterlacing a progressive file. Nothing is switched on or off for you; detection is sometimes wrong, so it stays a hint. Filters whose problems can't be spotted from the file alone — dirt, scratches, grain, halos — say nothing either way.
 
-Where two filters work against each other, the one that loses out says so when you open it: sharpening ahead of a denoiser that will undo it, for instance.
+Where two filters work against each other, the one that loses out says so when you open it: sharpening that would put back the halos Dehalo has just removed, for instance.
 
 ## Details
 

app/lib/models/pass_advice.dart

@@ -16,8 +16,8 @@ class PassAdvice {
 ///
 /// At a dozen-odd passes the complexity that actually costs users is not the
 /// length of the list, it is *interaction*: two denoisers stacked until the
-/// picture is plastic, sharpening applied before the denoiser eats it again,
-/// grain re-added before the deband that will smooth it away. None of that is
+/// picture is plastic, sharpening that puts back the halos Dehalo just removed,
+/// sharpening that exaggerates the grain Deband added. None of that is
 /// an error — every combination here produces a valid render — so none of it can
 /// be caught by validation. It has to be said out loud at the point the user is
 /// looking.
@@ -29,12 +29,16 @@ class PassAdvice {
 ///   app's job is to make sure they meant it.
 /// - **Only fires on enabled passes.** Advice about a pass nobody has turned on
 ///   is noise.
-/// - **Says what to do, not just what is wrong.** "Sharpen runs before Noise
-///   Reduction" is a fact; the useful half is that the denoiser will undo it.
+/// - **Says what to do, not just what is wrong.** "Dehalo runs before Sharpen"
+///   is a fact; the useful half is that sharpening can put the halos back.
+/// - **Never states an order the pipeline does not have.** Every "runs
+///   before/after" below is pinned against `enabledPasses` in
+///   `pass_advice_test.dart`, because a wrong one reads as a design flaw in the
+///   app rather than as a typo (issue #109).
 ///
 /// Pass order is fixed (see `PassListPanel.stages` and `script_generator.rs`),
 /// which is what makes the ordering advice statable at all: Sharpen genuinely
-/// always runs before Color Correction, so there is no case to qualify.
+/// always runs after Noise Reduction, so there is no case to qualify.
 List<PassAdvice> adviseOn(ProcessingPipeline pipeline) {
   final advice = <PassAdvice>[];
 
@@ -49,18 +53,13 @@ List<PassAdvice> adviseOn(ProcessingPipeline pipeline) {
     // combination. Deliberately silent.
   }
 
-  // --- Sharpening fights the denoiser, and loses ---
-  // Sharpen runs *before* Noise Reduction in the pipeline, so a denoiser set
-  // strongly enough to matter will remove most of what was just sharpened, and
-  // amplified noise is what survives.
-  if (on(PassType.sharpen) && on(PassType.noiseReduction)) {
-    advice.add(const PassAdvice(
-      PassType.sharpen,
-      'Sharpen runs before Noise Reduction, so the denoiser will soften much '
-      'of this again — and sharpened noise is what it has to work on. '
-      'Consider denoising alone first and judging the result.',
-    ));
-  }
+  // --- Sharpening after the denoiser ---
+  // Deliberately silent, and asserted so in pass_advice_test.dart. Sharpen runs
+  // *after* Noise Reduction — after every clean-up pass and the resize, in
+  // fact, with only Grain and Frame Rate behind it — which is the order a
+  // restoration chain wants: there is nothing to warn about. This used to claim
+  // the reverse and told users the denoiser would undo their sharpening
+  // (issue #109) — it never did.
 
   // --- Deband after grain ---
   // Deband runs after the denoiser but the f3kdb grain it adds back is applied

app/lib/models/processing_pipeline.dart

@@ -254,10 +254,9 @@ class ProcessingPipeline {
   /// Get the ordered list of enabled passes.
   List<PassType> get enabledPasses {
     final passes = <PassType>[];
-    // Order: Crop first (pre-processing), then deinterlace, noise, dehalo, deblock, deband, sharpen, chroma, color, resize last
-    if (cropResize.enabled && cropResize.cropEnabled) {
-      passes.add(PassType.cropResize); // Pre-crop
-    }
+    // Order: deinterlace, damage, noise, dehalo, deblock, deband, anti-alias,
+    // chroma, color, stabilize, rotate, crop + resize, then sharpen, grain,
+    // frame rate.
     if (deinterlace.enabled) {
       passes.add(PassType.deinterlace);
     }
@@ -304,9 +303,6 @@ class ProcessingPipeline {
     if (antiAlias.enabled) {
       passes.add(PassType.antiAlias);
     }
-    if (sharpen.enabled) {
-      passes.add(PassType.sharpen);
-    }
     if (chromaFixes.enabled) {
       passes.add(PassType.chromaFixes);
     }
@@ -324,11 +320,19 @@ class ProcessingPipeline {
     if (geometry.hasEffect) {
       passes.add(PassType.geometry);
     }
-    if (cropResize.enabled && cropResize.resizeEnabled) {
-      // Resize (post-processing) - if not already added for crop
-      if (!passes.contains(PassType.cropResize)) {
-        passes.add(PassType.cropResize);
-      }
+    // Crop and resize are one pass and run together, crop first. The crop used
+    // to run ahead of everything else, which meant it could not remove the
+    // edges Stabilize exposes (issue #109).
+    if (cropResize.enabled &&
+        (cropResize.cropEnabled || cropResize.resizeEnabled)) {
+      passes.add(PassType.cropResize);
+    }
+    // Sharpening follows the resize (issue #109), so it works on the delivered
+    // pixels: sharpened earlier, a downscale softens the edges again and an
+    // upscale enlarges the halos. It still follows anti-aliasing, and precedes
+    // the grain it would otherwise exaggerate.
+    if (sharpen.enabled) {
+      passes.add(PassType.sharpen);
     }
     // Grain goes last of the video passes: added before the resize it is
     // resampled away, before the deband it is smoothed away.

app/lib/views/pass_list/pass_list_panel.dart

@@ -58,7 +58,6 @@ class PassListPanel extends StatelessWidget {
       title: 'Detail & Color',
       passes: [
         PassType.antiAlias,
-        PassType.sharpen,
         PassType.chromaFixes,
         PassType.colorCorrection,
       ],
@@ -69,7 +68,7 @@ class PassListPanel extends StatelessWidget {
     ),
     (
       title: 'Finishing',
-      passes: [PassType.grain, PassType.frameRate],
+      passes: [PassType.sharpen, PassType.grain, PassType.frameRate],
     ),
     (
       title: 'Post-Processing',

app/test/integration_new_passes_test.dart

@@ -23,6 +23,7 @@ import 'package:vapourbox/models/chroma_denoise_parameters.dart';
 import 'package:vapourbox/models/dehalo_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/descratch_parameters.dart';
 import 'package:vapourbox/models/deblock_parameters.dart';
 import 'package:vapourbox/models/grain_parameters.dart';
@@ -235,6 +236,66 @@ void main() {
       await _expectValidVideo(result);
     }, timeout: const Timeout(Duration(minutes: 6)));
 
+    // Issue #109: Sharpen moved to after the resize. Every method has to run
+    // on a clip the resize has already changed the size of, in the encode — the
+    // script-only tests prove the order, not that the plugins accept it.
+    for (final method in SharpenMethod.values) {
+      test('sharpen: ${method.name} runs after a resize', () async {
+        final label = 'sharpen_after_resize_${method.name}';
+        final job = _baseJob(
+          label,
+          pipeline: ProcessingPipeline(
+            deinterlace: const QTGMCParameters(enabled: false),
+            cropResize: const CropResizeParameters(
+              enabled: true,
+              resizeEnabled: true,
+              targetWidth: 640,
+              targetHeight: 480,
+            ),
+            sharpen: SharpenParameters(enabled: true, method: method),
+          ),
+        );
+        final result = await WorkerHarness.runJob(job.toJson(), label: label);
+        await _expectValidVideo(result);
+        final v = await WorkerHarness.firstStream(result.outputPath!,
+            selector: 'v:0', entries: ['width', 'height']);
+        // 720x576 fitted inside 640x480 with the stored aspect kept: 600x480.
+        expect(v?['width'], 600, reason: 'sharpening must not undo the resize');
+        expect(v?['height'], 480);
+      }, timeout: const Timeout(Duration(minutes: 6)));
+    }
+
+    // Issue #109: the crop used to run before everything else. It now runs
+    // with the resize, after Stabilize and Rotate / Flip, so its four sides
+    // are the sides of the turned picture. The frame size tells the two orders
+    // apart: 720x576 turned is 576x720, and cropping 16 off each side and 8 off
+    // top and bottom leaves 544x704. Cropped before the turn it was 560x688.
+    test('crop: runs after stabilize and rotation', () async {
+      const label = 'crop_after_rotation';
+      final job = _baseJob(
+        label,
+        pipeline: const ProcessingPipeline(
+          deinterlace: QTGMCParameters(enabled: false),
+          stabilize: StabilizeParameters(enabled: true),
+          geometry: GeometryParameters(enabled: true, rotation: Rotation.cw90),
+          cropResize: CropResizeParameters(
+            enabled: true,
+            cropEnabled: true,
+            cropLeft: 16,
+            cropRight: 16,
+            cropTop: 8,
+            cropBottom: 8,
+          ),
+        ),
+      );
+      final result = await WorkerHarness.runJob(job.toJson(), label: label);
+      await _expectValidVideo(result);
+      final v = await WorkerHarness.firstStream(result.outputPath!,
+          selector: 'v:0', entries: ['width', 'height']);
+      expect(v?['width'], 544);
+      expect(v?['height'], 704);
+    }, timeout: const Timeout(Duration(minutes: 6)));
+
     test('dehalo: HQDeringmod runs end-to-end', () async {
       final job = _baseJob(
         'dehalo_hqderingmod',