Skip to content

feat(crop): soft-edge feathering with GPU transition support#150

Merged
walterlow merged 6 commits intomainfrom
develop
Apr 1, 2026
Merged

feat(crop): soft-edge feathering with GPU transition support#150
walterlow merged 6 commits intomainfrom
develop

Conversation

@walterlow
Copy link
Copy Markdown
Owner

@walterlow walterlow commented Apr 1, 2026

Summary

  • Add softness parameter to crop settings (-1 to 1) that creates feathered edges on cropped media
  • Implement rendering in DOM preview (CSS masks), canvas export (gradient alpha masks), and GPU transition pipeline
  • Fix GPU transition pipeline dropping crop alpha by setting premultipliedAlpha: true on texture uploads
  • Fix fade transition blend weights from cos/sin to cos²/sin² so alpha-bearing content blends without over-brightness
  • Eliminate sub-pixel seams at viewport edges via pixel-boundary rounding and +1px CSS coverage

Test plan

  • Apply crop (left/right/top/bottom) to a video clip, adjust softness slider — verify soft feathered edges in preview
  • Scrub through a fade transition between a cropped clip and an uncropped clip — verify feathered edges remain visible during the transition
  • Test negative softness values (inward feathering) and positive (outward expansion)
  • Export a project with cropped+softness clips and transitions — verify feathering in exported video
  • Verify no visible seams at crop boundaries at various softness values

Summary by CodeRabbit

  • New Features

    • Added media cropping for images and videos with per-edge controls and adjustable softness/feathering, plus live-preview in the properties sidebar.
    • Improved media containment so cropped assets are positioned and masked accurately within their containers.
  • Bug Fixes

    • Smoother fade transitions using equal-power blending for more natural crossfades.
    • Reduced unexpected fast-scrub overlay activation during gizmo-driven previews.

…upport

Add a softness parameter to crop settings that creates feathered edges
on cropped media. Implements rendering in both DOM preview (CSS masks in
ContainedMediaLayout) and canvas/export pipeline (gradient alpha masks
in applyCropFeatherMask).

Fix GPU transition pipeline dropping crop softness alpha by setting
premultipliedAlpha: true on texture uploads — without this, the browser
un-premultiplied Canvas 2D data before writing to GPU textures, mangling
semi-transparent pixels from feathered edges.

Fix fade transition blend weights from cos/sin (sum=1.414 at midpoint)
to cos²/sin² (sum=1.0 always) so alpha-bearing content like soft crops
and masks blends correctly without over-brightness.
Anchor viewport div to far edge (right: 0 / bottom: 0) when no crop
exists on that side, avoiding independent percentage rounding gaps.

Round canvas clip rect to pixel boundaries to prevent antialiased
seams in the export/scrub renderer path.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 1, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

Adds full media-crop support: types, normalization, geometry and feathering utilities, a ContainedMediaLayout React component with mask-based cropping, integration into item rendering and export pipelines, preview/UI crop controls, and updates fade crossfade weighting/compositing across canvas and GPU paths.

Changes

Cohort / File(s) Summary
Crop Types & Utilities
src/types/transform.ts, src/types/project.ts, src/types/timeline.ts, src/shared/utils/media-crop.ts, src/shared/utils/media-crop.test.ts
Introduce CropSettings and exported crop geometry APIs: normalization, ratio↔pixel conversions, contained-media layout and feather calculations. Add tests covering normalization, geometry, feathering, and hasMediaCrop detection.
Contained Media Layout & Integration
src/features/composition-runtime/components/contained-media-layout.tsx, src/features/composition-runtime/components/contained-media-layout.test.tsx, src/features/composition-runtime/components/item-visual-wrapper.tsx, src/features/composition-runtime/components/item.tsx
Add ContainedMediaLayout component that computes contain-fit geometry and builds CSS mask gradients for feathered edges. Wire ItemVisualWrapper to wrap media when fitMode='contain' and propagate source dimensions and crop from items. Update item rendering to pass mediaContent.
Editor UI & Preview
src/features/editor/components/properties-sidebar/clip-panel/video-section.tsx, src/features/composition-runtime/components/video-content.tsx, src/features/composition-runtime/components/stable-video-sequence.tsx, src/features/composition-runtime/components/stable-video-sequence.test.tsx, src/features/preview/stores/gizmo-store.ts
Extend video properties UI with crop edge and softness sliders, preview commit flow, and reset actions. Add crop to preview store shape. Add fitMode support in preview video and update tests to surface crop softness.
Export/Canvas Rendering Pipeline
src/features/export/utils/canvas-item-renderer.ts, src/features/export/utils/canvas-item-renderer.transition-state.test.ts, src/features/export/utils/canvas-render-orchestrator.ts, src/features/export/utils/client-render-engine.ts
Introduce transition participant render-state resolver and preview override hooks on ItemRenderContext. Replace direct draw logic with drawContainedMediaSource() pipeline, add contained-layout calculation, crop-feather masking, and ensure cropped items are treated non-occluding for culling. Add tests for resolver behavior.
Fade Crossfade & GPU
src/domain/timeline/transitions/renderers/basic.ts, src/features/export/utils/canvas-transitions.ts, src/lib/gpu-transitions/transitions/fade.ts, src/lib/gpu-transitions/transition-pipeline.ts
Switch fade weight to equal-power cosine-squared weights (canvas + WGSL shader adjustment). Refactor canvas fade rendering to apply separate incoming/outgoing weights and composite ops. Preserve premultiplied-alpha on GPU texture uploads.
Item Normalization & Schema
src/features/timeline/stores/items-store.ts, src/features/project-bundle/schemas/project-schema.ts, src/types/project.ts, src/types/timeline.ts
Integrate crop normalization into item normalization paths and project schema; add optional per-item crop in timeline/project types and validate with schema.
Preview Player Behavior
src/features/preview/components/video-preview.tsx
Prevent gizmo-store subscription from triggering fast-scrub overlay when forceFastScrubOverlay is false; adjust overlay gating to avoid replacing DOM Player seek behavior during gizmo previews.

Sequence Diagram

sequenceDiagram
    actor User
    participant UI as VideoSection UI
    participant Store as Preview Store
    participant Wrapper as ItemVisualWrapper
    participant Layout as ContainedMediaLayout
    participant Utils as MediaCropUtils
    participant Canvas as Canvas Renderer

    User->>UI: drag crop slider
    UI->>Store: setPropertiesPreviewNew({ crop })
    Store->>Wrapper: preview crop present on item props
    Wrapper->>Wrapper: sees mediaContent.fitMode='contain'
    Wrapper->>Layout: pass source/container dims + crop
    Layout->>Utils: calculateMediaCropLayout()
    Utils-->>Layout: MediaCropLayout (mediaRect, viewport, feather)
    Layout->>Wrapper: render children with mask-image and positioned rect
    User->>UI: commit crop
    UI->>Store: updateItem({ crop }) then clearPreview
    Store->>Canvas: render request with item.crop
    Canvas->>Utils: calculateContainedMediaDrawLayout()
    Canvas->>Canvas: drawContainedMediaSource() with feather masking
    Canvas-->>User: final frame rendered with crop applied
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 I nibbled edges, feathered light,
I hopped through masks and pixels bright.
From ratios, rectangles, a gentle sweep,
I stitched the view where shadows sleep.
A perfect frame — a rabbit's leap! 🎞️🐇

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 24.07% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'feat(crop): soft-edge feathering with GPU transition support' directly and clearly summarizes the primary changes: adding crop softness (feathering) functionality with GPU transition support.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch develop

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 1, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
freecut Ready Ready Preview, Comment Apr 1, 2026 3:26pm

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (2)
src/types/project.ts (1)

1-3: Reuse the shared CropSettings type here.

This is now another copy of the crop shape. Importing CropSettings keeps the project model aligned with src/types/transform.ts and reduces drift when crop fields evolve.

♻️ Suggested cleanup
 import type { AnimatableProperty, EasingType, EasingConfig } from './keyframe';
 import type { Transition } from './transition';
+import type { CropSettings } from './transform';
@@
-    crop?: {
-      left?: number;
-      right?: number;
-      top?: number;
-      bottom?: number;
-      softness?: number;
-    };
+    crop?: CropSettings;

Also applies to: 105-111

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/types/project.ts` around lines 1 - 3, The file currently duplicates the
crop shape instead of reusing the canonical type; import and use the shared
CropSettings type from src/types/transform.ts (add an import for CropSettings
alongside the existing imports) and replace the local duplicated crop/interface
usages with CropSettings (update the type references where the duplicate
appears, e.g., the project model fields that currently define a local crop shape
around the duplicated block at 105-111) so the project types reference
CropSettings directly.
src/features/preview/stores/gizmo-store.ts (1)

9-12: Store design allows partial crop payloads without deep-merging, creating fragility for future callers.

setPropertiesPreviewNew merges properties with shallow spread at line 378: properties: { ...merged[itemId]?.properties, ...props }. Current crop updates in video-section.tsx defensively spread ...item.crop before modifying a field, so they send full CropSettings—but the store does not enforce this pattern. A future caller sending { crop: { left: 0.2 } } would lose right/top/bottom/softness. Consider deep-merging crop fields in the store to prevent silent data loss.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/features/preview/stores/gizmo-store.ts` around lines 9 - 12, The store's
shallow merge in setPropertiesPreviewNew (specifically the merge at properties:
{ ...merged[itemId]?.properties, ...props }) can drop nested crop fields if a
caller sends a partial CropSettings; change the merge to deep-merge the crop
object: when building the new properties, compute mergedCrop = {
...merged[itemId]?.properties?.crop, ...props?.crop } (handling undefined) and
set properties to { ...merged[itemId]?.properties, ...props, crop: mergedCrop }
so existing right/top/bottom/softness are preserved even when callers provide
partial crop updates.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/domain/timeline/transitions/renderers/basic.ts`:
- Around line 19-25: The fade curve is inconsistent: update the fade helper(s)
in the transition engine and GPU renderer to use the same cos²/sin² weighting as
calculateFadeOpacity. Either extract a shared utility (e.g., export a single
calculateFadeOpacity/calculateFadeWeight function) and import it into
src/domain/timeline/transitions/engine.ts and
src/domain/timeline/transitions/renderers/gpu.ts, or change the local helpers in
those files to compute c = cos((progress * PI)/2) and return c*c for outgoing
and 1 - c*c for incoming so both render paths use identical fade
timing/brightness.

In `@src/features/composition-runtime/components/item.tsx`:
- Around line 210-214: The wrapper is using getSourceDimensions() (mediaSource)
to populate mediaContent causing legacy items with undefined intrinsic sizes to
fallback to transformed box and mismatch export; update the component that sets
mediaContent (the mediaContent prop block around mediaContent={{...}}) to only
pass crop/contain behavior when intrinsic dimensions are known (i.e., when
getSourceDimensions() returns a defined width/height), otherwise omit/leave
mediaContent undefined or pass a flag to indicate unknown intrinsic sizes;
alternatively, plumb the real media dimensions into mediaSource earlier by
resolving actual media dimensions before rendering this wrapper (referencing
getSourceDimensions(), mediaSource, mediaContent, and item.crop) so preview uses
the true intrinsic size—apply the same gating/fix to the other occurrences noted
(lines ~315-319 and ~345-349).

In `@src/features/export/utils/canvas-item-renderer.ts`:
- Around line 1872-1895: The pooled scratch canvas acquired via
canvasPool.acquire() may be smaller than the active render target, causing the
feather mask to rasterize into an undersized buffer and clip the right/bottom
when calling ctx.drawImage(scratchCanvas, 0, 0); fix drawContainedMediaSource()
by checking the acquired pooledCanvas.canvas dimensions against the active
canvas.width/height and, when pooledCanvas exists but is too small, resize the
pooled canvas to the target dimensions (set canvas.width/height and re-obtain
the 2D context, clear the canvas) before using applyCropFeatherMask and
ctx.drawImage; ensure you still release the pooled canvas via
canvasPool.release(...) and handle the scratchCanvas/scratchCtx fallback path
the same way as the non-pooled case (clearRect when newly created).

---

Nitpick comments:
In `@src/features/preview/stores/gizmo-store.ts`:
- Around line 9-12: The store's shallow merge in setPropertiesPreviewNew
(specifically the merge at properties: { ...merged[itemId]?.properties, ...props
}) can drop nested crop fields if a caller sends a partial CropSettings; change
the merge to deep-merge the crop object: when building the new properties,
compute mergedCrop = { ...merged[itemId]?.properties?.crop, ...props?.crop }
(handling undefined) and set properties to { ...merged[itemId]?.properties,
...props, crop: mergedCrop } so existing right/top/bottom/softness are preserved
even when callers provide partial crop updates.

In `@src/types/project.ts`:
- Around line 1-3: The file currently duplicates the crop shape instead of
reusing the canonical type; import and use the shared CropSettings type from
src/types/transform.ts (add an import for CropSettings alongside the existing
imports) and replace the local duplicated crop/interface usages with
CropSettings (update the type references where the duplicate appears, e.g., the
project model fields that currently define a local crop shape around the
duplicated block at 105-111) so the project types reference CropSettings
directly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c7df9d79-5d4f-4cdd-aa4f-2e97e6fd2eae

📥 Commits

Reviewing files that changed from the base of the PR and between d26cd18 and 581acc1.

📒 Files selected for processing (24)
  • src/domain/timeline/transitions/renderers/basic.ts
  • src/features/composition-runtime/components/contained-media-layout.test.tsx
  • src/features/composition-runtime/components/contained-media-layout.tsx
  • src/features/composition-runtime/components/item-visual-wrapper.tsx
  • src/features/composition-runtime/components/item.tsx
  • src/features/composition-runtime/components/stable-video-sequence.test.tsx
  • src/features/composition-runtime/components/stable-video-sequence.tsx
  • src/features/composition-runtime/components/video-content.tsx
  • src/features/editor/components/properties-sidebar/clip-panel/video-section.tsx
  • src/features/export/utils/canvas-item-renderer.transition-state.test.ts
  • src/features/export/utils/canvas-item-renderer.ts
  • src/features/export/utils/canvas-render-orchestrator.ts
  • src/features/export/utils/canvas-transitions.ts
  • src/features/export/utils/client-render-engine.ts
  • src/features/preview/stores/gizmo-store.ts
  • src/features/project-bundle/schemas/project-schema.ts
  • src/features/timeline/stores/items-store.ts
  • src/lib/gpu-transitions/transition-pipeline.ts
  • src/lib/gpu-transitions/transitions/fade.ts
  • src/shared/utils/media-crop.test.ts
  • src/shared/utils/media-crop.ts
  • src/types/project.ts
  • src/types/timeline.ts
  • src/types/transform.ts

Comment on lines 19 to +25
function calculateFadeOpacity(progress: number, isOutgoing: boolean): number {
// cos²/sin² weights — always sum to 1, preserving alpha for soft crop & masks.
const c = Math.cos((progress * Math.PI) / 2);
if (isOutgoing) {
return Math.cos((progress * Math.PI) / 2);
return c * c;
}
return Math.sin((progress * Math.PI) / 2);
return 1 - c * c;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Finish the fade-curve migration in the remaining fade helpers.

calculateFadeOpacity() switched to cos²/sin² here, but src/domain/timeline/transitions/engine.ts:1-15 and src/domain/timeline/transitions/renderers/gpu.ts:1-15 still return linear cos/sin. That leaves different callers with different opacity curves, so fade timing/brightness will diverge between render paths. Please update those helpers too, or extract a shared fade-weight utility.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/domain/timeline/transitions/renderers/basic.ts` around lines 19 - 25, The
fade curve is inconsistent: update the fade helper(s) in the transition engine
and GPU renderer to use the same cos²/sin² weighting as calculateFadeOpacity.
Either extract a shared utility (e.g., export a single
calculateFadeOpacity/calculateFadeWeight function) and import it into
src/domain/timeline/transitions/engine.ts and
src/domain/timeline/transitions/renderers/gpu.ts, or change the local helpers in
those files to compute c = cos((progress * PI)/2) and return c*c for outgoing
and 1 - c*c for incoming so both render paths use identical fade
timing/brightness.

Comment on lines +210 to +214
mediaContent={{
fitMode: 'contain',
sourceWidth: mediaSource?.width,
sourceHeight: mediaSource?.height,
crop: item.crop,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Use intrinsic media dimensions before enabling the contained crop path.

getSourceDimensions() only returns persisted metadata. For legacy video/image items it can be undefined, and the wrapper then falls back to the transformed box as the “source” aspect. Export still renders from the real media dimensions in src/features/export/utils/canvas-item-renderer.ts, so crop/softness can preview with a different viewport than export on older projects. Please gate mediaContent on known intrinsic sizes, or plumb the actual media dimensions into the wrapper first.

Also applies to: 315-319, 345-349

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/features/composition-runtime/components/item.tsx` around lines 210 - 214,
The wrapper is using getSourceDimensions() (mediaSource) to populate
mediaContent causing legacy items with undefined intrinsic sizes to fallback to
transformed box and mismatch export; update the component that sets mediaContent
(the mediaContent prop block around mediaContent={{...}}) to only pass
crop/contain behavior when intrinsic dimensions are known (i.e., when
getSourceDimensions() returns a defined width/height), otherwise omit/leave
mediaContent undefined or pass a flag to indicate unknown intrinsic sizes;
alternatively, plumb the real media dimensions into mediaSource earlier by
resolving actual media dimensions before rendering this wrapper (referencing
getSourceDimensions(), mediaSource, mediaContent, and item.crop) so preview uses
the true intrinsic size—apply the same gating/fix to the other occurrences noted
(lines ~315-319 and ~345-349).

Comment on lines +1872 to +1895
const pooledCanvas = canvasPool?.acquire();
const scratchCanvas = pooledCanvas?.canvas ?? new OffscreenCanvas(canvas.width, canvas.height);
const scratchCtx = pooledCanvas?.ctx ?? scratchCanvas.getContext('2d');
if (!scratchCtx) {
if (pooledCanvas) {
canvasPool?.release(scratchCanvas);
}
return false;
}

try {
if (!pooledCanvas) {
scratchCtx.clearRect(0, 0, canvas.width, canvas.height);
}

scratchCtx.save();
clipToViewport(scratchCtx, drawLayout.viewportRect);
drawSource(scratchCtx);
scratchCtx.restore();
applyCropFeatherMask(scratchCtx, drawLayout.viewportRect, drawLayout.featherPixels);
ctx.drawImage(scratchCanvas, 0, 0);
} finally {
if (pooledCanvas) {
canvasPool?.release(scratchCanvas);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Resize pooled scratch canvases to the active render target before feathering.

drawContainedMediaSource() assumes canvasPool.acquire() matches canvasSettings, but renderCompositionItem() and renderItemWithCornerPin() reuse the main pool while rendering into different-sized targets. When the target is larger than the pool canvas, the feather mask is rasterized into an undersized scratch buffer and ctx.drawImage(scratchCanvas, 0, 0) clips the right/bottom of the item. The same sizing guard is needed in Lines 732-752.

💡 Suggested guard
-  const pooledCanvas = canvasPool?.acquire();
-  const scratchCanvas = pooledCanvas?.canvas ?? new OffscreenCanvas(canvas.width, canvas.height);
-  const scratchCtx = pooledCanvas?.ctx ?? scratchCanvas.getContext('2d');
+  const pooledCanvas = canvasPool?.acquire();
+  const scratchCanvas = pooledCanvas?.canvas ?? new OffscreenCanvas(canvas.width, canvas.height);
+  if (scratchCanvas.width !== canvas.width || scratchCanvas.height !== canvas.height) {
+    scratchCanvas.width = canvas.width;
+    scratchCanvas.height = canvas.height;
+  }
+  const scratchCtx = pooledCanvas?.ctx ?? scratchCanvas.getContext('2d');
   if (!scratchCtx) {
     if (pooledCanvas) {
       canvasPool?.release(scratchCanvas);
     }
     return false;
   }
+  scratchCtx.clearRect(0, 0, canvas.width, canvas.height);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/features/export/utils/canvas-item-renderer.ts` around lines 1872 - 1895,
The pooled scratch canvas acquired via canvasPool.acquire() may be smaller than
the active render target, causing the feather mask to rasterize into an
undersized buffer and clip the right/bottom when calling
ctx.drawImage(scratchCanvas, 0, 0); fix drawContainedMediaSource() by checking
the acquired pooledCanvas.canvas dimensions against the active
canvas.width/height and, when pooledCanvas exists but is too small, resize the
pooled canvas to the target dimensions (set canvas.width/height and re-obtain
the 2D context, clear the canvas) before using applyCropFeatherMask and
ctx.drawImage; ensure you still release the pooled canvas via
canvasPool.release(...) and handle the scratchCanvas/scratchCtx fallback path
the same way as the non-pooled case (clearRect when newly created).

Replace inline crop type definition with import from transform.ts
to avoid maintaining duplicate type shapes.
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 1, 2026

Greptile Summary

This PR adds a full crop-with-soft-feathering pipeline for video and image clips, covering DOM preview (CSS mask-image gradients via ContainedMediaLayout), canvas export (gradient alpha masks via applyCropFeatherMask), and the GPU transition pipeline. It also fixes two pre-existing fade transition bugs: the GPU shader's argument order in mix() was reversed (left/right swapped), and all three fade renderers (DOM, canvas 2D, and GPU) now use cos²/sin² weights that correctly sum to 1 when clips carry real transparency.

Key changes:

  • New CropSettings type and media-crop.ts utilities implementing contain-fit geometry, edge-crop viewport derivation, and signed softness feathering (negative = inward, positive = outward expansion)
  • ContainedMediaLayout React component mirrors the same geometry for deterministic DOM preview
  • Canvas export path in canvas-item-renderer.ts replaced all drawImage calls with drawContainedMediaSource, which clip-rects to the viewport and optionally applies gradient feather masks on a scratch canvas
  • GPU texture uploads now pass premultipliedAlpha: true to correctly handle alpha-carrying frames
  • Fade blend model changed from "incoming as opaque bed + outgoing at cos weight" to "incoming via copy at sin² + outgoing via lighter at cos²", ensuring fully correct linear compositing for alpha-bearing content
  • Crop section added to the properties sidebar with four edge sliders and a shared softness slider

Minor issues found:

  • applyCropFeatherMask has a copy-paste duplicate addColorStop(0, …) for the right and bottom feather passes (lines 1771–1772 and 1804–1805) — functionally harmless but worth cleaning up
  • The crop shape in project.ts is defined inline rather than importing CropSettings, creating a type that could silently diverge"

Confidence Score: 5/5

Safe to merge — all identified findings are P2 style/cleanup items with no functional impact

The core crop geometry math (calculateMediaCropLayout, resolveCropSettings) is well-tested and correct. The feathering pipeline is consistent across DOM, canvas 2D, and GPU paths. The fade-blend and GPU argument-order fixes are mathematically sound. The only issues found are a copy-paste duplicate gradient stop and an inline type definition that duplicates CropSettings — neither affects runtime behavior.

src/features/export/utils/canvas-item-renderer.ts — duplicate addColorStop calls in applyCropFeatherMask (lines 1771–1772, 1804–1805)

Important Files Changed

Filename Overview
src/shared/utils/media-crop.ts New core utility for crop geometry: contains axis-pair clamping, contained-rect calculation, viewport/feather-pixel derivation. Math is correct; well-guarded against degenerate inputs.
src/features/export/utils/canvas-item-renderer.ts Major rework to support crop+feathering in canvas export. Logic is sound, but applyCropFeatherMask has duplicate addColorStop(0) calls for right and bottom edges (cosmetic copy-paste artifact).
src/domain/timeline/transitions/renderers/basic.ts Fade weights changed from cos/sin to cos²/sin² (always sum to 1) and canvas composite switches to copy+lighter, correctly preserving alpha from soft-cropped clips.
src/lib/gpu-transitions/transitions/fade.ts GPU fade fixed: t=sin²(p·π/2) replaces cos(p·π/2) and mix argument order corrected (left→right instead of right→left), aligning GPU output with canvas 2D behavior.
src/lib/gpu-transitions/transition-pipeline.ts Added premultipliedAlpha: true on texture uploads so GPU shader operates on pre-multiplied data matching Canvas 2D output; correct fix for alpha-carrying cropped frames.
src/features/composition-runtime/components/contained-media-layout.tsx New DOM component that replicates canvas crop geometry with CSS masks for soft edges. +1px coverage on viewport width/height is intentional to prevent sub-pixel seams.
src/features/export/utils/canvas-transitions.ts Fade transition in canvas export path updated to match basic.ts: copy+lighter composite with cos²/sin² weights and a dedicated drawFadeParticipant helper.
src/types/project.ts Adds crop to the project data shape inline rather than reusing CropSettings from transform.ts — minor type duplication that could drift over time.
src/features/project-bundle/schemas/project-schema.ts cropSchema added with correct min/max constraints (0–1 for edges, -1–1 for softness); correctly attached as optional to the base timeline item schema.
src/features/editor/components/properties-sidebar/clip-panel/video-section.tsx New Cropping property section with four edge sliders and a softness slider; correctly converts pixels↔ratios through media-crop utilities and uses gizmo-store for live preview.
src/features/composition-runtime/components/item.tsx Wires ContainedMediaLayout into video and image items by passing fitMode/sourceWidth/sourceHeight/crop to ItemVisualWrapper; objectFit changed to 'fill' since layout geometry is now handled by the parent component.
src/types/transform.ts New CropSettings interface with clear inline docs; all fields optional with softness range documented.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[CropSettings\nleft/right/top/bottom/softness] --> B[resolveCropSettings\nclamp + axis-pair clamping]
    B --> C[calculateMediaCropLayout\nmediaRect · cropViewportRect\nviewportRect · featherPixels]

    C --> D{Render path}

    D -->|DOM preview| E[ContainedMediaLayout\nCSS mask-image gradients]
    D -->|Canvas export\nnon-mediabunny| F[drawContainedMediaSource\nclipToViewport + applyCropFeatherMask\ndestination-in gradient passes]
    D -->|Canvas export\nmediabunny extractor| G[drawExtractorFrame on scratch canvas\napplyCropFeatherMask\nctx.drawImage result]
    D -->|GPU transition| H[premultipliedAlpha: true\nuploaded to WebGPU textures]

    F --> I[ctx.drawImage scratchCanvas]
    G --> I
    H --> J[WGSL fade shader\nmix left right t\nt = sin^2 p*pi/2]
Loading

Fix All in Claude Code Fix All in Codex

Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/features/export/utils/canvas-item-renderer.ts
Line: 1771-1772

Comment:
**Duplicate `addColorStop` at position 0 (right and bottom edges)**

Lines 1771–1772 and 1804–1805 both add an identical stop at position `0` for the right and bottom feather gradient passes respectively. While browsers handle this gracefully (the duplicate is redundant), it's a copy-paste artifact — the `right` and `bottom` blocks were likely adapted from the `left`/`top` blocks without removing the extra opening stop.

The first `addColorStop(0, ...)` is sufficient; remove the duplicate on each affected block:

```suggestion
    gradient.addColorStop(0, 'rgba(0, 0, 0, 1)');
    gradient.addColorStop(
      Math.max(0, Math.min(1, (viewportRect.width - featherPixels.right) / viewportRect.width)),
      'rgba(0, 0, 0, 1)',
    );
```

And similarly at line 1804–1805 for the `bottom` feather block.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/types/project.ts
Line: 105-111

Comment:
**Inline crop type duplicates `CropSettings`**

The `crop` shape defined inline here (`left`, `right`, `top`, `bottom`, `softness`) is structurally identical to the new `CropSettings` interface in `src/types/transform.ts`. If the canonical definition evolves (e.g. a `featherMode` field is added), this inline copy will silently diverge.

Consider importing and using `CropSettings` directly:

```suggestion
    crop?: import('@/types/transform').CropSettings;
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "fix(crop): eliminate sub-pixel seam at u..." | Re-trigger Greptile

Comment on lines +1771 to +1772
gradient.addColorStop(0, 'rgba(0, 0, 0, 1)');
gradient.addColorStop(0, 'rgba(0, 0, 0, 1)');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Duplicate addColorStop at position 0 (right and bottom edges)

Lines 1771–1772 and 1804–1805 both add an identical stop at position 0 for the right and bottom feather gradient passes respectively. While browsers handle this gracefully (the duplicate is redundant), it's a copy-paste artifact — the right and bottom blocks were likely adapted from the left/top blocks without removing the extra opening stop.

The first addColorStop(0, ...) is sufficient; remove the duplicate on each affected block:

Suggested change
gradient.addColorStop(0, 'rgba(0, 0, 0, 1)');
gradient.addColorStop(0, 'rgba(0, 0, 0, 1)');
gradient.addColorStop(0, 'rgba(0, 0, 0, 1)');
gradient.addColorStop(
Math.max(0, Math.min(1, (viewportRect.width - featherPixels.right) / viewportRect.width)),
'rgba(0, 0, 0, 1)',
);

And similarly at line 1804–1805 for the bottom feather block.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/features/export/utils/canvas-item-renderer.ts
Line: 1771-1772

Comment:
**Duplicate `addColorStop` at position 0 (right and bottom edges)**

Lines 1771–1772 and 1804–1805 both add an identical stop at position `0` for the right and bottom feather gradient passes respectively. While browsers handle this gracefully (the duplicate is redundant), it's a copy-paste artifact — the `right` and `bottom` blocks were likely adapted from the `left`/`top` blocks without removing the extra opening stop.

The first `addColorStop(0, ...)` is sufficient; remove the duplicate on each affected block:

```suggestion
    gradient.addColorStop(0, 'rgba(0, 0, 0, 1)');
    gradient.addColorStop(
      Math.max(0, Math.min(1, (viewportRect.width - featherPixels.right) / viewportRect.width)),
      'rgba(0, 0, 0, 1)',
    );
```

And similarly at line 1804–1805 for the `bottom` feather block.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code Fix in Codex

Comment thread src/types/project.ts Outdated
Comment on lines +105 to +111
crop?: {
left?: number;
right?: number;
top?: number;
bottom?: number;
softness?: number;
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Inline crop type duplicates CropSettings

The crop shape defined inline here (left, right, top, bottom, softness) is structurally identical to the new CropSettings interface in src/types/transform.ts. If the canonical definition evolves (e.g. a featherMode field is added), this inline copy will silently diverge.

Consider importing and using CropSettings directly:

Suggested change
crop?: {
left?: number;
right?: number;
top?: number;
bottom?: number;
softness?: number;
};
crop?: import('@/types/transform').CropSettings;
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/types/project.ts
Line: 105-111

Comment:
**Inline crop type duplicates `CropSettings`**

The `crop` shape defined inline here (`left`, `right`, `top`, `bottom`, `softness`) is structurally identical to the new `CropSettings` interface in `src/types/transform.ts`. If the canonical definition evolves (e.g. a `featherMode` field is added), this inline copy will silently diverge.

Consider importing and using `CropSettings` directly:

```suggestion
    crop?: import('@/types/transform').CropSettings;
```

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code Fix in Codex

…seams

The intermediate viewport div with overflow:hidden caused persistent
sub-pixel seams from CSS percentage rounding — no amount of +1px hacks
could reliably cover all crop/softness value combinations.

Replace with a single composite CSS mask-image on the mediaRect div
that handles both hard crop edges and soft feather gradients. Masks
render in the element's own coordinate space (paint-time, not layout),
so sub-pixel positioning issues are structurally impossible.

Also fix ±1 frame shift on crop slider drag by skipping scrub overlay
activation for non-GPU gizmo interactions — the DOM Player handles
crop/transform previews through React props without decoder mismatch.

Remove duplicate addColorStop(0) in right/bottom feather gradient
passes (copy-paste artifact, functionally harmless but confusing).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant