fix: review coderabbit

This commit is contained in:
anh doan 2026-03-23 15:29:45 +07:00
parent 6cbc8d9d1b
commit 22a42d509b
No known key found for this signature in database
10 changed files with 92 additions and 51 deletions

View File

@ -24,7 +24,7 @@
"@hugeicons/core-free-icons": "^3.1.1",
"@hugeicons/react": "^1.1.4",
"@huggingface/transformers": "^3.8.1",
"@mediapipe/face_mesh": "0.4.1633559619",
"@mediapipe/face_mesh": "0.4.1657299874",
"@opencut/effects": "workspace:*",
"@opencut/env": "workspace:*",
"@opencut/ui": "workspace:*",

View File

@ -10,8 +10,10 @@ import type { FaceMesh as FaceMeshType, Results } from "@mediapipe/face_mesh";
let faceMeshInstance: FaceMeshType | null = null;
let isLoading = false;
/** Resolve function for the current pending detection — avoids race conditions */
/** Shared in-flight promise for pending detections — avoids race conditions */
let pendingDetection: Promise<Results> | null = null;
let pendingResolve: ((results: Results) => void) | null = null;
let pendingReject: ((error: Error) => void) | null = null;
/** Lazy-load MediaPipe Face Mesh WASM module */
async function loadFaceMesh(): Promise<FaceMeshType | null> {
@ -91,12 +93,30 @@ export async function detectFace(
return { faceDetected: false };
}
// Promise-based approach avoids race conditions with concurrent calls
const results = await new Promise<Results>((resolve) => {
// Reuse existing in-flight detection if one exists
if (pendingDetection) {
const results = await pendingDetection;
if (
!results?.multiFaceLandmarks ||
results.multiFaceLandmarks.length === 0
) {
return { faceDetected: false };
}
return landmarksToContext(results.multiFaceLandmarks[0]);
}
// Create new detection promise
pendingDetection = new Promise<Results>((resolve, reject) => {
pendingResolve = resolve;
pendingReject = reject;
fm.send({ image: source as HTMLCanvasElement });
});
const results = await pendingDetection;
pendingDetection = null;
pendingResolve = null;
pendingReject = null;
if (
!results?.multiFaceLandmarks ||
results.multiFaceLandmarks.length === 0
@ -114,9 +134,15 @@ export function isFaceMeshReady(): boolean {
/** Clean up MediaPipe resources */
export function disposeFaceMesh(): void {
// Settle any pending detection before disposing
if (pendingReject) {
pendingReject(new Error("Face mesh disposed"));
pendingReject = null;
pendingResolve = null;
pendingDetection = null;
}
if (faceMeshInstance) {
faceMeshInstance.close();
faceMeshInstance = null;
}
pendingResolve = null;
}

File diff suppressed because one or more lines are too long

View File

@ -1,9 +1,11 @@
/** Effect categories for UI grouping */
export enum EffectCategory {
COLOR_TONE = "color-tone",
ARTISTIC = "artistic",
BEAUTY = "beauty",
}
export const EffectCategory = {
COLOR_TONE: "color-tone",
ARTISTIC: "artistic",
BEAUTY: "beauty",
} as const;
export type EffectCategory = (typeof EffectCategory)[keyof typeof EffectCategory];
export interface EffectCategoryMeta {
label: string;

View File

@ -2,6 +2,18 @@ import type { EffectDefinition } from "../../types";
import { EffectCategory } from "../../categories";
import blurFragmentShader from "./blur.frag.glsl";
/** Parse and validate intensity parameter, guarding against NaN and clamping to valid range */
const parseIntensity = (rawIntensity: unknown): number => {
const parsedIntensity =
typeof rawIntensity === "number"
? rawIntensity
: Number.parseFloat(String(rawIntensity));
if (!Number.isFinite(parsedIntensity)) {
return 0;
}
return Math.min(Math.max(parsedIntensity, 0), 100);
};
/** Gaussian blur — 2-pass (horizontal + vertical) for smooth, artifact-free results */
export const blurEffectDefinition: EffectDefinition = {
type: "blur",
@ -25,10 +37,7 @@ export const blurEffectDefinition: EffectDefinition = {
{
fragmentShader: blurFragmentShader,
uniforms: ({ effectParams, width }) => {
const intensity =
typeof effectParams.intensity === "number"
? effectParams.intensity
: Number.parseFloat(String(effectParams.intensity));
const intensity = parseIntensity(effectParams.intensity);
return {
u_sigma: Math.max((intensity / 5) * (width / 1920), 0.001),
u_direction: [1, 0],
@ -38,10 +47,7 @@ export const blurEffectDefinition: EffectDefinition = {
{
fragmentShader: blurFragmentShader,
uniforms: ({ effectParams, height }) => {
const intensity =
typeof effectParams.intensity === "number"
? effectParams.intensity
: Number.parseFloat(String(effectParams.intensity));
const intensity = parseIntensity(effectParams.intensity);
return {
u_sigma: Math.max((intensity / 5) * (height / 1080), 0.001),
u_direction: [0, 1],

View File

@ -9,15 +9,15 @@ export const colorBalanceEffectDefinition: EffectDefinition = {
category: EffectCategory.COLOR_TONE,
keywords: ["color balance", "grading", "shadows", "highlights", "midtones"],
params: [
{ key: "shadowsR", label: "Shadows Red", type: "number", default: 0, min: -100, max: 100, step: 1 },
{ key: "shadowsG", label: "Shadows Green", type: "number", default: 0, min: -100, max: 100, step: 1 },
{ key: "shadowsB", label: "Shadows Blue", type: "number", default: 0, min: -100, max: 100, step: 1 },
{ key: "midsR", label: "Midtones Red", type: "number", default: 0, min: -100, max: 100, step: 1 },
{ key: "midsG", label: "Midtones Green", type: "number", default: 0, min: -100, max: 100, step: 1 },
{ key: "midsB", label: "Midtones Blue", type: "number", default: 0, min: -100, max: 100, step: 1 },
{ key: "highlightsR", label: "Highlights Red", type: "number", default: 0, min: -100, max: 100, step: 1 },
{ key: "highlightsG", label: "Highlights Green", type: "number", default: 0, min: -100, max: 100, step: 1 },
{ key: "highlightsB", label: "Highlights Blue", type: "number", default: 0, min: -100, max: 100, step: 1 },
{ key: "shadowsRed", label: "Shadows Red", type: "number", default: 0, min: -100, max: 100, step: 1 },
{ key: "shadowsGreen", label: "Shadows Green", type: "number", default: 0, min: -100, max: 100, step: 1 },
{ key: "shadowsBlue", label: "Shadows Blue", type: "number", default: 0, min: -100, max: 100, step: 1 },
{ key: "midtonesRed", label: "Midtones Red", type: "number", default: 0, min: -100, max: 100, step: 1 },
{ key: "midtonesGreen", label: "Midtones Green", type: "number", default: 0, min: -100, max: 100, step: 1 },
{ key: "midtonesBlue", label: "Midtones Blue", type: "number", default: 0, min: -100, max: 100, step: 1 },
{ key: "highlightsRed", label: "Highlights Red", type: "number", default: 0, min: -100, max: 100, step: 1 },
{ key: "highlightsGreen", label: "Highlights Green", type: "number", default: 0, min: -100, max: 100, step: 1 },
{ key: "highlightsBlue", label: "Highlights Blue", type: "number", default: 0, min: -100, max: 100, step: 1 },
],
renderer: {
type: "webgl",
@ -26,19 +26,19 @@ export const colorBalanceEffectDefinition: EffectDefinition = {
fragmentShader,
uniforms: ({ effectParams }) => ({
u_shadows: [
(effectParams.shadowsR as number) / 200,
(effectParams.shadowsG as number) / 200,
(effectParams.shadowsB as number) / 200,
(effectParams.shadowsRed as number) / 200,
(effectParams.shadowsGreen as number) / 200,
(effectParams.shadowsBlue as number) / 200,
],
u_midtones: [
(effectParams.midsR as number) / 200,
(effectParams.midsG as number) / 200,
(effectParams.midsB as number) / 200,
(effectParams.midtonesRed as number) / 200,
(effectParams.midtonesGreen as number) / 200,
(effectParams.midtonesBlue as number) / 200,
],
u_highlights: [
(effectParams.highlightsR as number) / 200,
(effectParams.highlightsG as number) / 200,
(effectParams.highlightsB as number) / 200,
(effectParams.highlightsRed as number) / 200,
(effectParams.highlightsGreen as number) / 200,
(effectParams.highlightsBlue as number) / 200,
],
}),
},

View File

@ -16,11 +16,18 @@ export const slimFaceEffectDefinition: EffectDefinition = {
passes: [
{
fragmentShader,
uniforms: ({ effectParams, context }) => ({
u_intensity: (effectParams.intensity as number) / 100,
// Default jaw center if no face detection
u_jawCenter: context?.jawPoints?.slice(0, 2) ?? [0.5, 0.6],
}),
uniforms: ({ effectParams, context }) => {
const jawCenter =
context?.jawPoints && context.jawPoints.length >= 2
? context.jawPoints.slice(0, 2)
: null;
return {
// No-op when jaw landmarks are missing to avoid warping arbitrary content
u_intensity: jawCenter ? (effectParams.intensity as number) / 100 : 0,
u_jawCenter: jawCenter ?? [0.5, 0.6],
};
},
},
],
},

View File

@ -10,9 +10,9 @@ export const cinematicPresets: EffectPreset[] = [
{
effectType: "color-balance",
params: {
shadowsR: -30, shadowsG: 10, shadowsB: 30,
midsR: 0, midsG: 0, midsB: 0,
highlightsR: 30, highlightsG: 10, highlightsB: -20,
shadowsRed: -30, shadowsGreen: 10, shadowsBlue: 30,
midtonesRed: 0, midtonesGreen: 0, midtonesBlue: 0,
highlightsRed: 30, highlightsGreen: 10, highlightsBlue: -20,
},
},
{ effectType: "contrast", params: { intensity: 15 } },

View File

@ -1,9 +1,9 @@
/** Convert hex color string (#RRGGBB) to normalized RGB array [0..1, 0..1, 0..1] */
export function hexToRgb(hex: string): number[] {
const h = hex.replace("#", "");
export function hexToRgb(hex: string): [number, number, number] {
const hexString = hex.replace("#", "");
return [
Number.parseInt(h.substring(0, 2), 16) / 255,
Number.parseInt(h.substring(2, 4), 16) / 255,
Number.parseInt(h.substring(4, 6), 16) / 255,
Number.parseInt(hexString.slice(0, 2), 16) / 255,
Number.parseInt(hexString.slice(2, 4), 16) / 255,
Number.parseInt(hexString.slice(4, 6), 16) / 255,
];
}

View File

@ -141,7 +141,7 @@ describe("Oil Paint", () => {
test("radius max is capped at 5 for performance", () => {
const radiusParam = oilPaintEffectDefinition.params.find((p) => p.key === "radius");
if (radiusParam?.type === "number") {
expect(radiusParam.max).toBeLessThanOrEqual(10);
expect(radiusParam.max).toBeLessThanOrEqual(5);
}
});
});