diff --git a/web/packages/new/photos/components/PeopleList.tsx b/web/packages/new/photos/components/PeopleList.tsx index 2fb88b4983..5493fc7339 100644 --- a/web/packages/new/photos/components/PeopleList.tsx +++ b/web/packages/new/photos/components/PeopleList.tsx @@ -211,12 +211,12 @@ export const SuggestionFaceList: React.FC = ({ return ( {faces.map(({ file, faceID }) => ( - + - + ))} ); @@ -225,7 +225,18 @@ export const SuggestionFaceList: React.FC = ({ const SuggestionFaceList_ = styled("div")` display: flex; flex-wrap: wrap; - gap: 5px; + gap: 6px; +`; + +const SuggestionFace = styled("div")` + width: 87px; + height: 87px; + border-radius: 50%; + overflow: hidden; + & > img { + width: 100%; + height: 100%; + } `; type FaceCropImageViewProps = PreviewableFace & { diff --git a/web/packages/new/photos/components/gallery/PeopleHeader.tsx b/web/packages/new/photos/components/gallery/PeopleHeader.tsx index de9b2c4264..713b3f965c 100644 --- a/web/packages/new/photos/components/gallery/PeopleHeader.tsx +++ b/web/packages/new/photos/components/gallery/PeopleHeader.tsx @@ -15,13 +15,14 @@ import log from "@/base/log"; import { deleteCGroup, renameCGroup, - suggestionsForPerson, + suggestionsAndChoicesForPerson, } from "@/new/photos/services/ml"; import { type CGroupPerson, type ClusterPerson, type Person, - type PersonSuggestion, + type PersonSuggestionsAndChoices, + type PreviewableCluster, } from "@/new/photos/services/ml/people"; import { wait } from "@/utils/promise"; import OverflowMenu from "@ente/shared/components/OverflowMenu/menu"; @@ -30,8 +31,9 @@ import AddIcon from "@mui/icons-material/Add"; import CheckIcon from "@mui/icons-material/Check"; import ClearIcon from "@mui/icons-material/Clear"; import EditIcon from "@mui/icons-material/Edit"; -import ListAltOutlined from "@mui/icons-material/ListAltOutlined"; -import MoreHoriz from "@mui/icons-material/MoreHoriz"; +import ListAltOutlinedIcon from "@mui/icons-material/ListAltOutlined"; +import MoreHorizIcon from "@mui/icons-material/MoreHoriz"; +import RestoreIcon from "@mui/icons-material/Restore"; import { Dialog, DialogActions, @@ -47,10 +49,12 @@ import { Typography, } from "@mui/material"; import { t } from "i18next"; -import React, { useEffect, useReducer } from "react"; +import React, { useEffect, useReducer, useState } from "react"; +import { isInternalUser } from "../../services/feature-flags"; import { useAppContext } from "../../types/context"; import { AddPersonDialog } from "../AddPersonDialog"; import { SpaceBetweenFlex } from "../mui"; +import { SuggestionFaceList } from "../PeopleList"; import { SingleInputDialog } from "../SingleInputForm"; import type { GalleryBarImplProps } from "./BarImpl"; import { GalleryItemsHeaderAdapter, GalleryItemsSummary } from "./ListHeader"; @@ -115,12 +119,17 @@ const CGroupPersonHeader: React.FC = ({ const cgroup = person.cgroup; const { showMiniDialog } = useAppContext(); + const [showReviewOption, setShowReviewOption] = useState(false); const { show: showNameInput, props: nameInputVisibilityProps } = useModalVisibility(); const { show: showSuggestions, props: suggestionsVisibilityProps } = useModalVisibility(); + useEffect(() => { + void isInternalUser().then((b) => setShowReviewOption(b)); + }, []); + const handleRename = (name: string) => renameCGroup(cgroup, name); const handleReset = () => @@ -152,7 +161,7 @@ const CGroupPersonHeader: React.FC = ({ /> } + triggerButtonIcon={} > } @@ -168,9 +177,9 @@ const CGroupPersonHeader: React.FC = ({ > {pt("Reset")} - {process.env.NEXT_PUBLIC_ENTE_WIP_CL /* TODO-Cluster */ && ( + {showReviewOption /* TODO-Cluster */ && ( } + startIcon={} centerAlign onClick={showSuggestions} > @@ -232,7 +241,7 @@ const ClusterPersonHeader: React.FC = ({ } + triggerButtonIcon={} > } @@ -268,33 +277,45 @@ interface SuggestionsDialogState { */ fetchFailed: boolean; /** - * List of clusters (suitably augmented for the UI display) which might - * belong to the person, and being offered to the user as suggestions. + * True if we should show the previously saved choice view instead of the + * new suggestions. */ - suggestions: PersonSuggestion[]; + showChoices: boolean; + /** Fetched choices. */ + choices: SCItem[]; + /** Fetched suggestions. */ + suggestions: SCItem[]; /** - * An entry corresponding to each clusters (suggestions) that the user has - * either explicitly accepted or rejected. + * An entry corresponding to each + * - saved choice for which the user has changed their mind. + * - suggestion that the user has either explicitly accepted or rejected. */ - markedSuggestionIDs: Map>; + marks: Map; } -type SuggestionMark = "yes" | "no" | undefined; +type SCItem = PreviewableCluster & { fixed?: boolean; accepted?: boolean }; type SuggestionsDialogAction = | { type: "fetch"; personID: string } | { type: "fetchFailed"; personID: string } - | { type: "fetched"; personID: string; suggestions: PersonSuggestion[] } - | { type: "mark"; suggestion: PersonSuggestion; value: SuggestionMark } + | { + type: "fetched"; + personID: string; + suggestionsAndChoices: PersonSuggestionsAndChoices; + } + | { type: "mark"; item: SCItem; value: boolean | undefined } | { type: "save" } + | { type: "toggleHistory" } | { type: "close" }; const initialSuggestionsDialogState: SuggestionsDialogState = { activity: undefined, personID: undefined, fetchFailed: false, + showChoices: false, + choices: [], suggestions: [], - markedSuggestionIDs: new Map(), + marks: new Map(), }; const suggestionsDialogReducer = ( @@ -304,11 +325,12 @@ const suggestionsDialogReducer = ( switch (action.type) { case "fetch": return { + ...initialSuggestionsDialogState, + choices: [], + suggestions: [], + marks: new Map(), activity: "fetching", personID: action.personID, - fetchFailed: false, - suggestions: [], - markedSuggestionIDs: new Map(), }; case "fetchFailed": if (action.personID != state.personID) return state; @@ -318,21 +340,27 @@ const suggestionsDialogReducer = ( return { ...state, activity: undefined, - suggestions: action.suggestions, + choices: action.suggestionsAndChoices.choices, + suggestions: action.suggestionsAndChoices.suggestions, }; case "mark": { - const markedSuggestionIDs = new Map(state.markedSuggestionIDs); - const id = action.suggestion.id; - if (action.value == "yes" || action.value == "no") { - markedSuggestionIDs.set(id, action.value); + const marks = new Map(state.marks); + const { item, value } = action; + if (item.accepted === undefined && value === undefined) { + // If this was a suggestion, prune marks created as a result of + // the user toggling the item back to its original unset state. + marks.delete(item.id); + } else if (item.accepted && value === item.accepted) { + // If this is a choice, prune marks which match the choice's + // accepted state. + marks.delete(item.id); } else { - markedSuggestionIDs.delete(id); + marks.set(item.id, value); } - return { - ...state, - markedSuggestionIDs, - }; + return { ...state, marks }; } + case "toggleHistory": + return { ...state, showChoices: !state.showChoices }; case "save": return { ...state, activity: "saving" }; case "close": @@ -360,7 +388,7 @@ const SuggestionsDialog: React.FC = ({ const isSmallWidth = useIsSmallWidth(); - const hasUnsavedChanges = state.markedSuggestionIDs.size > 0; + const hasUnsavedChanges = state.marks.size > 0; const resetPersonAndClose = () => { dispatch({ type: "close" }); @@ -380,10 +408,11 @@ const SuggestionsDialog: React.FC = ({ const go = async () => { try { - const suggestions = await suggestionsForPerson(person); - dispatch({ type: "fetched", personID, suggestions }); + const suggestionsAndChoices = + await suggestionsAndChoicesForPerson(person); + dispatch({ type: "fetched", personID, suggestionsAndChoices }); } catch (e) { - log.error("Failed to generate suggestions", e); + log.error("Failed to fetch suggestions and choices", e); dispatch({ type: "fetchFailed", personID }); } }; @@ -410,8 +439,8 @@ const SuggestionsDialog: React.FC = ({ resetPersonAndClose(); }; - const handleMark = (suggestion: PersonSuggestion, value: SuggestionMark) => - dispatch({ type: "mark", suggestion, value }); + const handleMark = (item: SCItem, value: boolean | undefined) => + dispatch({ type: "mark", item, value }); const handleSave = async () => { try { @@ -434,10 +463,48 @@ const SuggestionsDialog: React.FC = ({ fullScreen={isSmallWidth} PaperProps={{ sx: { minHeight: "80svh" } }} > - - {person.name && pt(`${person.name}?`)} - - + theme.colors.fill.faint + : "transparent", + }} + > + + + {state.showChoices + ? pt("Saved choices") + : pt("Review suggestions")} + + + {person.name ?? " "} + + + {state.choices.length > 1 && ( + dispatch({ type: "toggleHistory" })} + aria-label={ + !state.showChoices + ? pt("Saved suggestions") + : pt("Review suggestions") + } + sx={{ + backgroundColor: state.showChoices + ? (theme) => theme.colors.fill.muted + : "transparent", + }} + > + + + )} + + {state.activity == "fetching" ? ( @@ -448,6 +515,12 @@ const SuggestionsDialog: React.FC = ({ + ) : state.showChoices ? ( + ) : state.suggestions.length == 0 ? ( = ({ ) : ( - )} @@ -480,61 +553,74 @@ const SuggestionsDialog: React.FC = ({ color={"accent"} onClick={handleSave} > - {t("save")} + {hasUnsavedChanges ? pt("TODO Not impl") : t("save")} ); }; -type SuggestionsListProps = Pick< - SuggestionsDialogState, - "suggestions" | "markedSuggestionIDs" -> & { +interface SuggestionOrChoiceListProps { + items: SCItem[]; + marks: Map; /** - * Callback invoked when the user toggles the value associated with the - * given suggestion. + * Callback invoked when the user changes the value associated with the + * given suggestion or choice. */ - onMarkSuggestion: ( - suggestion: PersonSuggestion, - value: SuggestionMark, - ) => void; -}; + onMarkItem: (item: SCItem, value: boolean | undefined) => void; +} -const SuggestionsList: React.FC = ({ - suggestions, - markedSuggestionIDs, - onMarkSuggestion, +const SuggestionOrChoiceList: React.FC = ({ + items, + marks, + onMarkItem, }) => ( - - {suggestions.map((suggestion) => ( + + {items.map((item) => ( - {`${suggestion.previewFaces.length} faces ntaoheu naoehtu aosnehu asoenuh aoenuht`} - - onMarkSuggestion( - suggestion, - // Dance for TypeScript to recognize the type. - v == "yes" ? "yes" : v == "no" ? "no" : undefined, - ) - } - > - - - - - - - + + + {/* Use the face count as as stand-in for the photo count */} + {t("photos_count", { count: item.faces.length })} + + + + {!item.fixed && ( + onMarkItem(item, toItemValue(v))} + > + + + + + + + + )} ))} ); + +const fromItemValue = ( + item: SCItem, + marks: Map, +) => { + // Use the in-memory state if available. For choices, fallback to their + // original state. + const resolved = marks.has(item.id) ? marks.get(item.id) : item.accepted; + return resolved ? "yes" : resolved === false ? "no" : undefined; +}; + +const toItemValue = (v: unknown) => + // This dance is needed for TypeScript to recognize the type. + v == "yes" ? true : v == "no" ? false : undefined; diff --git a/web/packages/new/photos/services/ml/clip.ts b/web/packages/new/photos/services/ml/clip.ts index 773621602f..a3c6a22487 100644 --- a/web/packages/new/photos/services/ml/clip.ts +++ b/web/packages/new/photos/services/ml/clip.ts @@ -133,7 +133,7 @@ const normalized = (embedding: Float32Array) => { * The result can also be `undefined`, which indicates that the download for the * ML model is still in progress (trying again later should succeed). */ -export const clipMatches = async ( +export const _clipMatches = async ( searchPhrase: string, electron: ElectronMLWorker, ): Promise => { diff --git a/web/packages/new/photos/services/ml/cluster.ts b/web/packages/new/photos/services/ml/cluster.ts index fab47652ce..36a39b5e09 100644 --- a/web/packages/new/photos/services/ml/cluster.ts +++ b/web/packages/new/photos/services/ml/cluster.ts @@ -62,7 +62,7 @@ export type ClusterFace = Omit & { * other interactions with the worker (where this code runs) do not get stalled * while clustering is in progress. */ -export const clusterFaces = async ( +export const _clusterFaces = async ( faceIndexes: FaceIndex[], localFiles: EnteFile[], onProgress: (progress: ClusteringProgress) => void, diff --git a/web/packages/new/photos/services/ml/index.ts b/web/packages/new/photos/services/ml/index.ts index c19f32abe0..f3469a7dd0 100644 --- a/web/packages/new/photos/services/ml/index.ts +++ b/web/packages/new/photos/services/ml/index.ts @@ -794,5 +794,5 @@ export const deleteCGroup = async ({ id }: CGroup) => { * * The suggestion computation happens in a web worker. */ -export const suggestionsForPerson = async (person: CGroupPerson) => - worker().then((w) => w.suggestionsForPerson(person)); +export const suggestionsAndChoicesForPerson = async (person: CGroupPerson) => + worker().then((w) => w.suggestionsAndChoicesForPerson(person)); diff --git a/web/packages/new/photos/services/ml/people.ts b/web/packages/new/photos/services/ml/people.ts index b20988c074..16b36a0232 100644 --- a/web/packages/new/photos/services/ml/people.ts +++ b/web/packages/new/photos/services/ml/people.ts @@ -1,9 +1,11 @@ import { assertionFailed } from "@/base/assert"; import log from "@/base/log"; import type { EnteFile } from "@/media/file"; +import { shuffled } from "@/utils/array"; +import { ensure } from "@/utils/ensure"; import { getLocalFiles } from "../files"; import { savedCGroups, type CGroup } from "../user-entity"; -import type { FaceCluster } from "./cluster"; +import { type FaceCluster } from "./cluster"; import { savedFaceClusters, savedFaceIndexes } from "./db"; import { fileIDFromFaceID } from "./face"; import { dotProduct } from "./math"; @@ -322,28 +324,45 @@ export const filterNamedPeople = (people: Person[]): NamedPerson[] => { return namedPeople; }; -export interface PersonSuggestion { +export type PreviewableCluster = FaceCluster & { /** - * The ID of the suggestion. This is the same as the cluster's ID, - * duplicated here for the UI's convenience. - */ - id: string; - /** - * The underlying {@link FaceCluster} that is being offered as the - * suggestion. - */ - cluster: FaceCluster; - /** - * A list of up to 3 "preview" faces for this cluster, each annotated with + * A list of up to 3 "preview" faces for the cluster, each annotated with * the corresponding {@link EnteFile} that contains them. */ previewFaces: PreviewableFace[]; +}; + +export interface PersonSuggestionsAndChoices { + /** + * Previously saved choices. + * + * These are clusters (sorted by size) that the user had previously merged + * or explicitly ignored from the person under consideration. + * + * The {@link accepted} flag will true for the entries that correspond to + * accepted clusters, and false for those that the user had ignored. + * + * This array is guaranteed to be non-empty, and it is guaranteed that the + * first item is a merged cluster (i.e. a cluster for which accepted is + * true), even if there exists an ignored cluster with a larger size. The + * rest of the entries are intermixed and sorted by size normally. + * + * For convenience of the UI, teh first entry will have also have the + * {@link fixed} flag set. + */ + choices: (PreviewableCluster & { fixed?: boolean; accepted: boolean })[]; + /** + * New suggestions to offer to the user. + */ + suggestions: PreviewableCluster[]; } /** - * Returns suggestions for the given person. + * Returns suggestions and existing choices for the given person. */ -export const suggestionsForPerson = async (person: CGroupPerson) => { +export const _suggestionsAndChoicesForPerson = async ( + person: CGroupPerson, +): Promise => { const startTime = Date.now(); const personClusters = person.cgroup.data.assigned; @@ -371,6 +390,12 @@ export const suggestionsForPerson = async (person: CGroupPerson) => { .flat() .filter((e) => !!e); + // Randomly sample faces to limit the O(n^2) cost. + const sampledPersonFaceEmbeddings = shuffled(personFaceEmbeddings).slice( + 0, + 100, + ); + const suggestedClusters: FaceCluster[] = []; for (const cluster of clusters) { const { id, faces } = cluster; @@ -384,7 +409,7 @@ export const suggestionsForPerson = async (person: CGroupPerson) => { for (const fi of faces) { const ei = embeddingByFaceID.get(fi); if (!ei) continue; - for (const ej of personFaceEmbeddings) { + for (const ej of sampledPersonFaceEmbeddings) { const csim = dotProduct(ei, ej); if (csim >= 0.6) { suggest = true; @@ -397,41 +422,65 @@ export const suggestionsForPerson = async (person: CGroupPerson) => { if (suggest) suggestedClusters.push(cluster); } - suggestedClusters.sort((a, b) => b.faces.length - a.faces.length); - // Annotate the clusters with the information that the UI needs to show its // preview faces. const files = await getLocalFiles("normal"); const fileByID = new Map(files.map((f) => [f.id, f])); - const suggestions = suggestedClusters.map((cluster) => { - const previewFaces = cluster.faces - .slice(0, 3) - .map((faceID) => { - const fileID = fileIDFromFaceID(faceID); - if (!fileID) { - assertionFailed(); - return undefined; - } - const file = fileByID.get(fileID); - if (!file) { - // TODO-Cluster: This might be a hidden/trash file, so this - // assert is not appropriate, we instead need a "until 3". - // assertionFailed(); - return undefined; - } - return { file, faceID }; - }) - .filter((f) => !!f); + const toPreviewable = (cluster: FaceCluster) => { + const previewFaces: PreviewableFace[] = []; + for (const faceID of cluster.faces) { + const fileID = fileIDFromFaceID(faceID); + if (!fileID) { + assertionFailed(); + continue; + } - const id = cluster.id; - return { id, cluster, previewFaces }; - }); + const file = fileByID.get(fileID); + if (!file) { + // This might be a hidden/trash file, and it is thus not + // appropriate to use it as a preview file anyway. + continue; + } + + previewFaces.push({ file, faceID }); + + if (previewFaces.length == 4) break; + } + + return { ...cluster, previewFaces }; + }; + + const sortBySize = (entries: { faces: unknown[] }[]) => + entries.sort((a, b) => b.faces.length - a.faces.length); + + const acceptedChoices = personClusters + .map(toPreviewable) + .map((p) => ({ ...p, accepted: true })); + + sortBySize(acceptedChoices); + + const ignoredChoices = ignoredClusters + .map(toPreviewable) + .map((p) => ({ ...p, accepted: false })); + + // Ensure that the first item in the choices is not an ignored one, even if + // that is what we'd have ended up with if we sorted by size. + + const firstChoice = { ...ensure(acceptedChoices[0]), fixed: true }; + const restChoices = acceptedChoices.slice(1).concat(ignoredChoices); + sortBySize(restChoices); + + const choices = [firstChoice, ...restChoices]; + + sortBySize(suggestedClusters); + // Limit to the number of suggestions shown in a single go. + const suggestions = suggestedClusters.slice(0, 80).map(toPreviewable); log.info( `Generated ${suggestions.length} suggestions for ${person.id} (${Date.now() - startTime} ms)`, ); - return suggestions; + return { choices, suggestions }; }; diff --git a/web/packages/new/photos/services/ml/worker.ts b/web/packages/new/photos/services/ml/worker.ts index 906a7a2771..d8ebea44c0 100644 --- a/web/packages/new/photos/services/ml/worker.ts +++ b/web/packages/new/photos/services/ml/worker.ts @@ -18,14 +18,14 @@ import { type ImageBitmapAndData, } from "./blob"; import { + _clipMatches, clearCachedCLIPIndexes, clipIndexingVersion, - clipMatches, indexCLIP, type CLIPIndex, } from "./clip"; import { - clusterFaces, + _clusterFaces, reconcileClusters, type ClusteringProgress, } from "./cluster"; @@ -44,7 +44,7 @@ import { type RawRemoteMLData, type RemoteMLData, } from "./ml-data"; -import { suggestionsForPerson, type CGroupPerson } from "./people"; +import { _suggestionsAndChoicesForPerson, type CGroupPerson } from "./people"; import type { CLIPMatches, MLWorkerDelegate } from "./worker-types"; /** @@ -207,7 +207,7 @@ export class MLWorker { * Find {@link CLIPMatches} for a given normalized {@link searchPhrase}. */ async clipMatches(searchPhrase: string): Promise { - return clipMatches(searchPhrase, ensure(this.electron)); + return _clipMatches(searchPhrase, ensure(this.electron)); } private async tick() { @@ -326,7 +326,7 @@ export class MLWorker { * cgroups if needed. */ async clusterFaces(masterKey: Uint8Array) { - const clusters = await clusterFaces( + const clusters = await _clusterFaces( await savedFaceIndexes(), await getAllLocalFiles(), (progress) => this.updateClusteringProgress(progress), @@ -341,10 +341,10 @@ export class MLWorker { } /** - * Return suggestions for the given cgroup {@link person}. + * Return suggestions and choices for the given cgroup {@link person}. */ - async suggestionsForPerson(person: CGroupPerson) { - return suggestionsForPerson(person); + async suggestionsAndChoicesForPerson(person: CGroupPerson) { + return _suggestionsAndChoicesForPerson(person); } }