diff --git a/web/apps/photos/src/components/Collections/GalleryBarAndListHeader.tsx b/web/apps/photos/src/components/Collections/GalleryBarAndListHeader.tsx index 0e9c83dde4..882879e7ba 100644 --- a/web/apps/photos/src/components/Collections/GalleryBarAndListHeader.tsx +++ b/web/apps/photos/src/components/Collections/GalleryBarAndListHeader.tsx @@ -9,7 +9,6 @@ import { type CollectionsSortBy, type CollectionSummaries, } from "@/new/photos/types/collection"; -import { ensure } from "@/utils/ensure"; import { includes } from "@/utils/type-guards"; import { getData, @@ -95,7 +94,7 @@ export const GalleryBarAndListHeader: React.FC = ({ setActiveCollectionID, hiddenCollectionSummaries, people, - activePersonID, + activePerson, onSelectPerson, setCollectionNamerAttributes, setPhotoListHeader, @@ -173,10 +172,7 @@ export const GalleryBarAndListHeader: React.FC = ({ /> ) : ( p.id == activePersonID) ?? - people[0], - )} + person={activePerson} {...{ onSelectPerson, appContext }} /> ), @@ -190,7 +186,7 @@ export const GalleryBarAndListHeader: React.FC = ({ activeCollectionID, isActiveCollectionDownloadInProgress, people, - activePersonID, + activePerson, ]); if (shouldBeHidden) { @@ -205,7 +201,7 @@ export const GalleryBarAndListHeader: React.FC = ({ onChangeMode, activeCollectionID, people, - activePersonID, + activePerson, onSelectPerson, collectionsSortBy, }} diff --git a/web/apps/photos/src/components/PhotoFrame.tsx b/web/apps/photos/src/components/PhotoFrame.tsx index 59bb80b954..ccf0908a2b 100644 --- a/web/apps/photos/src/components/PhotoFrame.tsx +++ b/web/apps/photos/src/components/PhotoFrame.tsx @@ -8,7 +8,7 @@ import { PHOTOS_PAGES } from "@ente/shared/constants/pages"; import { CustomError } from "@ente/shared/error"; import useMemoSingleThreaded from "@ente/shared/hooks/useMemoSingleThreaded"; import { styled } from "@mui/material"; -import PhotoViewer from "components/PhotoViewer"; +import PhotoViewer, { type PhotoViewerProps } from "components/PhotoViewer"; import { useRouter } from "next/router"; import { GalleryContext } from "pages/gallery"; import PhotoSwipe from "photoswipe"; @@ -72,6 +72,7 @@ interface Props { isInHiddenSection?: boolean; setFilesDownloadProgressAttributesCreator?: SetFilesDownloadProgressAttributesCreator; selectable?: boolean; + onSelectPerson?: PhotoViewerProps["onSelectPerson"]; } const PhotoFrame = ({ @@ -95,6 +96,7 @@ const PhotoFrame = ({ isInHiddenSection, setFilesDownloadProgressAttributesCreator, selectable, + onSelectPerson, }: Props) => { const [open, setOpen] = useState(false); const [currentIndex, setCurrentIndex] = useState(0); @@ -580,6 +582,7 @@ const PhotoFrame = ({ setFilesDownloadProgressAttributesCreator={ setFilesDownloadProgressAttributesCreator } + onSelectPerson={onSelectPerson} /> ); diff --git a/web/apps/photos/src/components/PhotoViewer/FileInfo/index.tsx b/web/apps/photos/src/components/PhotoViewer/FileInfo/index.tsx index 20157caab7..a528aa1c2e 100644 --- a/web/apps/photos/src/components/PhotoViewer/FileInfo/index.tsx +++ b/web/apps/photos/src/components/PhotoViewer/FileInfo/index.tsx @@ -12,11 +12,19 @@ import { type ParsedMetadataDate, } from "@/media/file-metadata"; import { FileType } from "@/media/file-type"; -import { UnidentifiedFaces } from "@/new/photos/components/PeopleList"; +import { + AnnotatedFacePeopleList, + UnclusteredFaceList, +} from "@/new/photos/components/PeopleList"; import { PhotoDateTimePicker } from "@/new/photos/components/PhotoDateTimePicker"; import { photoSwipeZIndex } from "@/new/photos/components/PhotoViewer"; import { tagNumericValue, type RawExifTags } from "@/new/photos/services/exif"; -import { isMLEnabled } from "@/new/photos/services/ml"; +import { + AnnotatedFacesForFile, + getAnnotatedFacesForFile, + isMLEnabled, + type AnnotatedFaceID, +} from "@/new/photos/services/ml"; import { EnteFile } from "@/new/photos/types/file"; import { formattedByteSize } from "@/new/photos/utils/units"; import CopyButton from "@ente/shared/components/CodeBlock/CopyButton"; @@ -61,7 +69,7 @@ export interface FileInfoExif { parsed: ParsedMetadata | undefined; } -interface FileInfoProps { +export interface FileInfoProps { showInfo: boolean; handleCloseInfo: () => void; closePhotoViewer: () => void; @@ -73,6 +81,10 @@ interface FileInfoProps { fileToCollectionsMap?: Map; collectionNameMap?: Map; showCollectionChips: boolean; + /** + * Called when the user selects a person in the file info panel. + */ + onSelectPerson?: ((personID: string) => void) | undefined; } export const FileInfo: React.FC = ({ @@ -87,6 +99,7 @@ export const FileInfo: React.FC = ({ collectionNameMap, showCollectionChips, closePhotoViewer, + onSelectPerson, }) => { const { mapEnabled, updateMapEnabled, setDialogBoxAttributesV2 } = useContext(AppContext); @@ -97,6 +110,9 @@ export const FileInfo: React.FC = ({ const [exifInfo, setExifInfo] = useState(); const [openRawExif, setOpenRawExif] = useState(false); + const [annotatedFaces, setAnnotatedFaces] = useState< + AnnotatedFacesForFile | undefined + >(); const location = useMemo(() => { if (file) { @@ -106,6 +122,21 @@ export const FileInfo: React.FC = ({ return exif?.parsed?.location; }, [file, exif]); + useEffect(() => { + if (!file) return; + + let didCancel = false; + + void (async () => { + const result = await getAnnotatedFacesForFile(file); + !didCancel && setAnnotatedFaces(result); + })(); + + return () => { + didCancel = true; + }; + }, [file]); + useEffect(() => { setExifInfo(parseExifInfo(exif)); }, [exif]); @@ -129,6 +160,13 @@ export const FileInfo: React.FC = ({ getMapDisableConfirmationDialog(() => updateMapEnabled(false)), ); + const handleSelectFace = (annotatedFaceID: AnnotatedFaceID) => { + if (onSelectPerson) { + onSelectPerson(annotatedFaceID.personID); + closePhotoViewer(); + } + }; + return ( @@ -267,10 +305,17 @@ export const FileInfo: React.FC = ({ )} - {isMLEnabled() && ( + {isMLEnabled() && annotatedFaces && ( <> - {/* TODO-Cluster */} - + + )} diff --git a/web/apps/photos/src/components/PhotoViewer/index.tsx b/web/apps/photos/src/components/PhotoViewer/index.tsx index 10d2f310e2..b15f1e3f44 100644 --- a/web/apps/photos/src/components/PhotoViewer/index.tsx +++ b/web/apps/photos/src/components/PhotoViewer/index.tsx @@ -48,7 +48,7 @@ import { SetFilesDownloadProgressAttributesCreator } from "types/gallery"; import { pauseVideo, playVideo } from "utils/photoFrame"; import { PublicCollectionGalleryContext } from "utils/publicCollectionGallery"; import { getTrashFileMessage } from "utils/ui"; -import { FileInfo, type FileInfoExif } from "./FileInfo"; +import { FileInfo, type FileInfoExif, type FileInfoProps } from "./FileInfo"; import ImageEditorOverlay from "./ImageEditorOverlay"; import CircularProgressWithLabel from "./styledComponents/CircularProgressWithLabel"; import { ConversionFailedNotification } from "./styledComponents/ConversionFailedNotification"; @@ -98,7 +98,8 @@ const CaptionContainer = styled("div")(({ theme }) => ({ backgroundColor: theme.colors.backdrop.faint, backdropFilter: `blur(${theme.colors.blur.base})`, })); -interface Iprops { + +export interface PhotoViewerProps { isOpen: boolean; items: any[]; currentIndex?: number; @@ -115,9 +116,10 @@ interface Iprops { fileToCollectionsMap: Map; collectionNameMap: Map; setFilesDownloadProgressAttributesCreator: SetFilesDownloadProgressAttributesCreator; + onSelectPerson?: FileInfoProps["onSelectPerson"]; } -function PhotoViewer(props: Iprops) { +function PhotoViewer(props: PhotoViewerProps) { const galleryContext = useContext(GalleryContext); const appContext = useContext(AppContext); const publicCollectionGalleryContext = useContext( @@ -969,6 +971,7 @@ function PhotoViewer(props: Iprops) { refreshPhotoswipe={refreshPhotoswipe} fileToCollectionsMap={props.fileToCollectionsMap} collectionNameMap={props.collectionNameMap} + onSelectPerson={props.onSelectPerson} /> => { + // The derived UI state when we are in "people" mode. + // + // TODO: This spawns even more workarounds below. Move this to a + // reducer/store. + type DerivedState1 = { + filteredData: EnteFile[]; + galleryPeopleState: GalleryPeopleState | undefined; + }; + + const derived1: DerivedState1 = useMemoSingleThreaded(async () => { if ( !files || !user || @@ -536,35 +544,54 @@ export default function Gallery() { !hiddenFiles || !archivedCollections ) { - return; + return { filteredData: [], galleryPeopleState: undefined }; } if (activeCollectionID === TRASH_SECTION && !selectedSearchOption) { - return getUniqueFiles([ + const filteredData = getUniqueFiles([ ...trashedFiles, ...files.filter((file) => tempDeletedFileIds?.has(file.id)), ]); + return { filteredData, galleryPeopleState: undefined }; } let filteredFiles: EnteFile[] = []; + let galleryPeopleState: GalleryPeopleState; if (selectedSearchOption) { filteredFiles = await filterSearchableFiles( selectedSearchOption.suggestion, ); } else if (barMode == "people") { - const activePerson = ensure( - people.find((p) => p.id == activePersonID) ?? people[0], - ); - const pfSet = new Set(activePerson.fileIDs); + let filteredPeople = people; + if (tempDeletedFileIds?.size ?? tempHiddenFileIds?.size) { + // Prune the in-memory temp updates from the actual state to + // obtain the UI state. + filteredPeople = people + .map((p) => ({ + ...p, + fileIDs: p.fileIDs.filter( + (id) => + !tempDeletedFileIds?.has(id) && + !tempHiddenFileIds?.has(id), + ), + })) + .filter((p) => p.fileIDs.length > 0); + } + const activePerson = + filteredPeople.find((p) => p.id == activePersonID) ?? + filteredPeople[0]; + const pfSet = new Set(activePerson?.fileIDs ?? []); filteredFiles = getUniqueFiles( files.filter(({ id }) => { if (!pfSet.has(id)) return false; - // TODO-Cluster - // if (tempDeletedFileIds?.has(id)) return false; - // if (tempHiddenFileIds?.has(id)) return false; return true; }), ); + galleryPeopleState = { + activePerson, + activePersonID, + people: filteredPeople, + }; } else { const baseFiles = barMode == "hidden-albums" ? hiddenFiles : files; filteredFiles = getUniqueFiles( @@ -630,10 +657,10 @@ export default function Gallery() { } const sortAsc = activeCollection?.pubMagicMetadata?.data?.asc ?? false; if (sortAsc) { - return sortFiles(filteredFiles, true); - } else { - return filteredFiles; + filteredFiles = sortFiles(filteredFiles, true); } + + return { filteredData: filteredFiles, galleryPeopleState }; }, [ barMode, files, @@ -649,6 +676,36 @@ export default function Gallery() { activePersonID, ]); + const { filteredData, galleryPeopleState } = derived1 ?? { + filteredData: [], + galleryPeopleState: undefined, + }; + + // Calling setState during rendering is frowned upon for good reasons, but + // it is not verboten, and it has documented semantics: + // + // > React will discard the currently rendering component's output and + // > immediately attempt to render it again with the new state. + // > + // > https://react.dev/reference/react/useState + // + // That said, we should try to refactor this code to use a reducer or some + // other store so that this is not needed. + if (barMode == "people" && galleryPeopleState?.people.length === 0) { + log.info( + "Resetting gallery to all section since people mode is no longer valid", + ); + setBarMode("albums"); + setActiveCollectionID(ALL_SECTION); + } + + // Derived1 is async, leading to even more workarounds. + const resolvedBarMode = galleryPeopleState + ? barMode + : barMode == "people" + ? "albums" + : barMode; + const selectAll = (e: KeyboardEvent) => { // ignore ctrl/cmd + a if the user is typing in a text field if ( @@ -679,10 +736,14 @@ export default function Gallery() { count: 0, collectionID: activeCollectionID, context: - barMode == "people" && activePersonID - ? { mode: "people" as const, personID: activePersonID } + resolvedBarMode == "people" && + galleryPeopleState?.activePersonID + ? { + mode: "people" as const, + personID: galleryPeopleState.activePersonID, + } : { - mode: "albums" as const, + mode: resolvedBarMode as "albums" | "hidden-albums", collectionID: ensure(activeCollectionID), }, }; @@ -1058,7 +1119,12 @@ export default function Gallery() { // when the user clicks the "People" header in the search empty state (it // is guaranteed that this header will only be shown if there is at // least one person). - setActivePersonID(person?.id ?? ensure(people[0]).id); + setActivePersonID(person?.id ?? galleryPeopleState?.people[0]?.id); + setBarMode("people"); + }; + + const handleSelectFileInfoPerson = (personID: string) => { + setActivePersonID(personID); setBarMode("people"); }; @@ -1142,7 +1208,7 @@ export default function Gallery() { marginBottom: "12px", }} > - {barMode == "hidden-albums" ? ( + {resolvedBarMode == "hidden-albums" ? ( @@ -1163,15 +1229,16 @@ export default function Gallery() { )} {selected.count > 0 && @@ -1275,7 +1343,7 @@ export default function Gallery() { count={selected.count} ownCount={selected.ownCount} clearSelection={clearSelection} - barMode={barMode} + barMode={resolvedBarMode} activeCollectionID={activeCollectionID} selectedCollection={getSelectedCollection( selected.collectionID, @@ -1296,7 +1364,9 @@ export default function Gallery() { ?.type == "incomingShareViewer" } isInSearchMode={isInSearchMode} - isInHiddenSection={barMode == "hidden-albums"} + isInHiddenSection={ + resolvedBarMode == "hidden-albums" + } /> )} = ({ collectionsSortBy, onChangeCollectionsSortBy, people, - activePersonID, + activePerson, onSelectPerson, }) => { const isMobile = useIsMobileWidth(); @@ -194,11 +194,11 @@ export const GalleryBarImpl: React.FC = ({ ); break; case "people": - i = people.findIndex(({ id }) => id == activePersonID); + i = people.findIndex(({ id }) => id == activePerson?.id); break; } if (i != -1) listRef.current.scrollToItem(i, "smart"); - }, [mode, collectionSummaries, activeCollectionID, people, activePersonID]); + }, [mode, collectionSummaries, activeCollectionID, people, activePerson]); const itemData = useMemo( () => @@ -210,12 +210,9 @@ export const GalleryBarImpl: React.FC = ({ onSelectCollectionID, } : { - type: "people", + type: "people" as const, people, - activePerson: ensure( - people.find((p) => p.id == activePersonID) ?? - people[0], - ), + activePerson: ensure(activePerson), onSelectPerson, }, [ @@ -224,7 +221,7 @@ export const GalleryBarImpl: React.FC = ({ activeCollectionID, onSelectCollectionID, people, - activePersonID, + activePerson, onSelectPerson, ], ); diff --git a/web/packages/new/photos/components/Gallery/PeopleHeader.tsx b/web/packages/new/photos/components/Gallery/PeopleHeader.tsx index 616ae53acd..ad6e71925f 100644 --- a/web/packages/new/photos/components/Gallery/PeopleHeader.tsx +++ b/web/packages/new/photos/components/Gallery/PeopleHeader.tsx @@ -31,6 +31,34 @@ import { NameInputDialog } from "../NameInputDialog"; import type { GalleryBarImplProps } from "./BarImpl"; import { GalleryItemsHeaderAdapter, GalleryItemsSummary } from "./ListHeader"; +/** + * Derived UI state backing the gallery when it is in "people" mode. + * + * This may be different from the actual underlying state since there might be + * unsynced data (hidden or deleted that have not yet been synced with remote) + * that should be taken into account for the UI state. + */ +export interface GalleryPeopleState { + /** + * The ID of the currently selected person. + * + * We do not have an empty state currently, so this is guaranteed to be + * present whenever the gallery is in the "people" mode. + */ + activePersonID: string; + /** + * The currently selected person. + * + * This is a convenience property that contains a direct reference to the + * active {@link Person} from amongst {@link people}. + */ + activePerson: Person; + /** + * The list of people to show. + */ + people: Person[]; +} + type PeopleHeaderProps = Pick & { person: Person; appContext: NewAppContextPhotos; diff --git a/web/packages/new/photos/components/PeopleList.tsx b/web/packages/new/photos/components/PeopleList.tsx index 1f9862f308..ba7711af59 100644 --- a/web/packages/new/photos/components/PeopleList.tsx +++ b/web/packages/new/photos/components/PeopleList.tsx @@ -1,5 +1,6 @@ import { useIsMobileWidth } from "@/base/hooks"; -import { faceCrop, unidentifiedFaceIDs } from "@/new/photos/services/ml"; +import { pt } from "@/base/i18n"; +import { faceCrop, type AnnotatedFaceID } from "@/new/photos/services/ml"; import type { Person } from "@/new/photos/services/ml/people"; import type { EnteFile } from "@/new/photos/types/file"; import { Skeleton, Typography, styled } from "@mui/material"; @@ -25,7 +26,7 @@ export const SearchPeopleList: React.FC = ({ sx={{ justifyContent: people.length > 3 ? "center" : "start" }} > {people.slice(0, isMobileWidth ? 6 : 7).map((person) => ( - onSelectPerson(person)} > @@ -34,7 +35,7 @@ export const SearchPeopleList: React.FC = ({ enteFile={person.displayFaceFile} placeholderDimension={87} /> - + ))} ); @@ -49,7 +50,7 @@ const SearchPeopleContainer = styled("div")` margin-block-end: 15px; `; -const SearchPeopleButton = styled(UnstyledButton)( +const SearchPersonButton = styled(UnstyledButton)( ({ theme }) => ` width: 87px; height: 87px; @@ -66,88 +67,132 @@ const SearchPeopleButton = styled(UnstyledButton)( `, ); -const FaceChipContainer = styled("div")` +export interface AnnotatedFacePeopleListProps { + /** + * The {@link EnteFile} whose information we are showing. + */ + enteFile: EnteFile; + /** + * The list of faces in the file that are associated with a person. + */ + annotatedFaceIDs: AnnotatedFaceID[]; + /** + * Called when the user selects a face in the list. + */ + onSelectFace: (annotatedFaceID: AnnotatedFaceID) => void; +} + +/** + * Show the list of faces in the given file that are associated with a specific + * person. + */ +export const AnnotatedFacePeopleList: React.FC< + AnnotatedFacePeopleListProps +> = ({ enteFile, annotatedFaceIDs, onSelectFace }) => { + if (annotatedFaceIDs.length == 0) return <>; + + return ( + <> + + {t("people")} + + + {annotatedFaceIDs.map((annotatedFaceID) => ( + onSelectFace(annotatedFaceID)} + > + + + ))} + + + ); +}; + +const FileFaceList = styled("div")` display: flex; flex-wrap: wrap; justify-content: center; align-items: center; - margin-top: 5px; - margin-bottom: 5px; - overflow: auto; + gap: 5px; + margin: 5px; `; -const FaceChip = styled("div")<{ clickable?: boolean }>` +const AnnotatedFaceButton = styled(UnstyledButton)( + ({ theme }) => ` width: 112px; height: 112px; - margin: 5px; border-radius: 50%; overflow: hidden; - position: relative; - cursor: ${({ clickable }) => (clickable ? "pointer" : "normal")}; & > img { width: 100%; height: 100%; } -`; + :hover { + outline: 1px solid ${theme.colors.stroke.faint}; + outline-offset: 2px; + } +`, +); -export interface PhotoPeopleListProps { - file: EnteFile; - onSelect?: (person: Person, index: number) => void; -} - -export function PhotoPeopleList() { - return <>; -} - -interface UnidentifiedFacesProps { +export interface UnclusteredFaceListProps { + /** + * The {@link EnteFile} whose information we are showing. + */ enteFile: EnteFile; + /** + * The list of faces in the file that are not associated with a person. + */ + faceIDs: string[]; } /** - * Show the list of faces in the given file that are not linked to a specific - * person ("face cluster"). + * Show the list of faces in the given file that are not associated with a + * specific person. */ -export const UnidentifiedFaces: React.FC = ({ +export const UnclusteredFaceList: React.FC = ({ enteFile, + faceIDs, }) => { - const [faceIDs, setFaceIDs] = useState([]); - - useEffect(() => { - let didCancel = false; - - const go = async () => { - const faceIDs = await unidentifiedFaceIDs(enteFile); - !didCancel && setFaceIDs(faceIDs); - }; - - void go(); - - return () => { - didCancel = true; - }; - }, [enteFile]); - if (faceIDs.length == 0) return <>; return ( <> - {t("UNIDENTIFIED_FACES")} + {pt("Other faces")} + {/*t("UNIDENTIFIED_FACES") TODO-Cluster */} - + {faceIDs.map((faceID) => ( - + - + ))} - + ); }; +const UnclusteredFace = styled("div")` + width: 112px; + height: 112px; + margin: 5px; + border-radius: 50%; + overflow: hidden; + & > img { + width: 100%; + height: 100%; + } +`; + interface FaceCropImageViewProps { /** The ID of the face to display. */ faceID: string; diff --git a/web/packages/new/photos/services/ml/cluster.ts b/web/packages/new/photos/services/ml/cluster.ts index 4cd7e6850f..c0d81173b9 100644 --- a/web/packages/new/photos/services/ml/cluster.ts +++ b/web/packages/new/photos/services/ml/cluster.ts @@ -1,3 +1,4 @@ +import { assertionFailed } from "@/base/assert"; import { newNonSecureID } from "@/base/id-worker"; import log from "@/base/log"; import { ensure } from "@/utils/ensure"; @@ -204,18 +205,24 @@ const sortFacesNewestOnesFirst = ( const fileForFaceID = new Map( faces.map(({ faceID }) => [ faceID, - ensure(localFileByID.get(ensure(fileIDFromFaceID(faceID)))), + localFileByID.get(ensure(fileIDFromFaceID(faceID))), ]), ); - const fileForFace = ({ faceID }: { faceID: string }) => - ensure(fileForFaceID.get(faceID)); + // In unexpected scenarios, we might run clustering without having the + // corresponding EnteFile available locally. This shouldn't happen, so log + // an warning, but meanwhile let the clustering proceed by assigning such + // files an arbitrary creationTime. + const sortTimeForFace = ({ faceID }: { faceID: string }) => { + const file = fileForFaceID.get(faceID); + if (!file) { + assertionFailed(`Did not find a local file for faceID ${faceID}`); + return 0; + } + return file.metadata.creationTime; + }; - return faces.sort( - (a, b) => - fileForFace(b).metadata.creationTime - - fileForFace(a).metadata.creationTime, - ); + return faces.sort((a, b) => sortTimeForFace(b) - sortTimeForFace(a)); }; /** @@ -340,22 +347,22 @@ export const reconcileClusters = async ( const clusterByID = new Map(clusters.map((c) => [c.id, c])); // Get the existing remote cluster groups. - const cgroupEntities = await savedCGroups(); + const cgroups = await savedCGroups(); // Find the cgroups that have changed since we started. - const changedCGroupEntities = cgroupEntities - .map((cgroupEntity) => { - for (const oldCluster of cgroupEntity.data.assigned) { + const changedCGroups = cgroups + .map((cgroup) => { + for (const oldCluster of cgroup.data.assigned) { // The clustering algorithm does not remove any existing faces, it // can only add new ones to the cluster. So we can use the count as // an indication if something changed. const newCluster = ensure(clusterByID.get(oldCluster.id)); if (oldCluster.faces.length != newCluster.faces.length) { return { - ...cgroupEntity, + ...cgroup, data: { - ...cgroupEntity.data, - assigned: cgroupEntity.data.assigned.map(({ id }) => + ...cgroup.data, + assigned: cgroup.data.assigned.map(({ id }) => ensure(clusterByID.get(id)), ), }, @@ -367,19 +374,15 @@ export const reconcileClusters = async ( .filter((g) => !!g); // Update remote if needed. - if (changedCGroupEntities.length) { - await updateOrCreateUserEntities( - "cgroup", - changedCGroupEntities, - masterKey, - ); - log.info(`Updated ${changedCGroupEntities.length} remote cgroups`); + if (changedCGroups.length) { + await updateOrCreateUserEntities("cgroup", changedCGroups, masterKey); + log.info(`Updated ${changedCGroups.length} remote cgroups`); } // Find which clusters are part of remote cgroups. const isRemoteClusterID = new Set(); - for (const cgroupEntity of cgroupEntities) { - for (const cluster of cgroupEntity.data.assigned) + for (const cgroup of cgroups) { + for (const cluster of cgroup.data.assigned) isRemoteClusterID.add(cluster.id); } diff --git a/web/packages/new/photos/services/ml/index.ts b/web/packages/new/photos/services/ml/index.ts index ddb88dd69c..6de92a6205 100644 --- a/web/packages/new/photos/services/ml/index.ts +++ b/web/packages/new/photos/services/ml/index.ts @@ -5,7 +5,6 @@ import { isDesktop } from "@/base/app"; import { blobCache } from "@/base/blob-cache"; import { ensureElectron } from "@/base/electron"; -import { isDevBuild } from "@/base/env"; import log from "@/base/log"; import { masterKeyFromSession } from "@/base/session-store"; import type { Electron } from "@/base/types/ipc"; @@ -107,7 +106,7 @@ const worker = () => const createComlinkWorker = async () => { const electron = ensureElectron(); - const delegate = { workerDidUpdateStatus }; + const delegate = { workerDidUpdateStatus, workerDidUnawaitedIndex }; // Obtain a message port from the Electron layer. const messagePort = await createMLWorker(electron); @@ -313,30 +312,34 @@ export const mlSync = async () => { // Dependency order for the sync // - // files -> faces -> cgroups -> clusters + // files -> faces -> cgroups -> clusters -> people // - const w = await worker(); - // Fetch indexes, or index locally if needed. - await w.index(); + await (await worker()).index(); - // TODO-Cluster - if (await wipClusterEnable()) { - const masterKey = await masterKeyFromSession(); - - // Fetch existing cgroups from remote. - await pullUserEntities("cgroup", masterKey); - - // Generate or update local clusters. - await w.clusterFaces(masterKey); - } - - await updatePeople(); + await updateClustersAndPeople(); _state.isSyncing = false; }; +const workerDidUnawaitedIndex = () => void updateClustersAndPeople(); + +const updateClustersAndPeople = async () => { + if (!(await isInternalUser())) return; + + const masterKey = await masterKeyFromSession(); + + // Fetch existing cgroups from remote. + await pullUserEntities("cgroup", masterKey); + + // Generate or update local clusters. + await (await worker()).clusterFaces(masterKey); + + // Update the people shown in the UI. + await updatePeople(); +}; + /** * Run indexing on a file which was uploaded from this client. * @@ -361,14 +364,6 @@ export const indexNewUpload = (enteFile: EnteFile, uploadItem: UploadItem) => { void worker().then((w) => w.onUpload(enteFile, uploadItem)); }; -/** - * WIP! Don't enable, dragon eggs are hatching here. - * TODO-Cluster - */ -export const wipClusterEnable = async (): Promise => - (!!process.env.NEXT_PUBLIC_ENTE_WIP_CL && isDevBuild) || - (await isInternalUser()); - export type MLStatus = | { phase: "disabled" /* The ML remote flag is off */ } | { @@ -589,15 +584,65 @@ export const clipMatches = ( ): Promise => worker().then((w) => w.clipMatches(searchPhrase)); +/** A face ID annotated with the ID of the person to which it is associated. */ +export interface AnnotatedFaceID { + faceID: string; + personID: string; +} + /** - * Return the IDs of all the faces in the given {@link enteFile} that are not - * associated with a person cluster. + * List of faces found in a file + * + * It is actually a pair of lists, one annotated by the person ids, and one with + * just the face ids. */ -export const unidentifiedFaceIDs = async ( +export interface AnnotatedFacesForFile { + /** + * A list of {@link AnnotatedFaceID}s for all faces in the file that are + * also associated with a {@link Person}. + */ + annotatedFaceIDs: AnnotatedFaceID[]; + /* A list of the remaining face (ids). */ + otherFaceIDs: string[]; +} + +/** + * Return the list of faces found in the given {@link enteFile}. + */ +export const getAnnotatedFacesForFile = async ( enteFile: EnteFile, -): Promise => { +): Promise => { + const annotatedFaceIDs: AnnotatedFaceID[] = []; + const otherFaceIDs: string[] = []; + const index = await getFaceIndex(enteFile.id); - return index?.faces.map((f) => f.faceID) ?? []; + if (!index) return { annotatedFaceIDs, otherFaceIDs }; + + const people = _state.peopleSnapshot ?? []; + + const faceIDToPersonID = new Map(); + for (const person of people) { + let faceIDs: string[]; + if (person.type == "cgroup") { + faceIDs = person.cgroup.data.assigned.map((c) => c.faces).flat(); + } else { + faceIDs = person.cluster.faces; + } + for (const faceID of faceIDs) { + faceIDToPersonID.set(faceID, person.id); + } + } + + for (const { faceID } of index.faces) { + const personID = faceIDToPersonID.get(faceID); + if (personID) { + annotatedFaceIDs.push({ faceID, personID }); + } else { + otherFaceIDs.push(faceID); + } + } + + return { annotatedFaceIDs, otherFaceIDs }; }; /** diff --git a/web/packages/new/photos/services/ml/people.ts b/web/packages/new/photos/services/ml/people.ts index 6ad95d3fad..79e9152b86 100644 --- a/web/packages/new/photos/services/ml/people.ts +++ b/web/packages/new/photos/services/ml/people.ts @@ -1,4 +1,3 @@ -import { wipClusterEnable } from "."; import type { EnteFile } from "../../types/file"; import { getLocalFiles } from "../files"; import { savedCGroups, type CGroup } from "../user-entity"; @@ -138,8 +137,6 @@ export type Person = ( * reference. */ export const reconstructPeople = async (): Promise => { - if (!(await wipClusterEnable())) return []; - const files = await getLocalFiles("normal"); const fileByID = new Map(files.map((f) => [f.id, f])); @@ -247,10 +244,12 @@ export const reconstructPeople = async (): Promise => { }; }); - return cgroupPeople - .concat(clusterPeople) - .filter((c) => !!c) - .sort((a, b) => b.fileIDs.length - a.fileIDs.length); + const sorted = (ps: Interim) => + ps + .filter((c) => !!c) + .sort((a, b) => b.fileIDs.length - a.fileIDs.length); + + return sorted(cgroupPeople).concat(sorted(clusterPeople)); }; /** diff --git a/web/packages/new/photos/services/ml/worker-types.ts b/web/packages/new/photos/services/ml/worker-types.ts index b12598d52b..16153b3b2b 100644 --- a/web/packages/new/photos/services/ml/worker-types.ts +++ b/web/packages/new/photos/services/ml/worker-types.ts @@ -13,6 +13,19 @@ export interface MLWorkerDelegate { * indicating the indexing or clustering status to be updated. */ workerDidUpdateStatus: () => void; + /** + * Called when the worker indexes some files, but then notices that the main + * thread was not awaiting the indexing (e.g. it was not initiated by the + * main thread during a sync, but happened because of a live upload). + * + * In such cases, it uses this method to inform the main thread that some + * files were indexed, so that it can update any dependent state (e.g. + * clusters). + * + * It doesn't always call this because otherwise the main thread would need + * some extra code to avoid updating the dependent state twice. + */ + workerDidUnawaitedIndex: () => void; } /** diff --git a/web/packages/new/photos/services/ml/worker.ts b/web/packages/new/photos/services/ml/worker.ts index b9156e4106..42ee2a439f 100644 --- a/web/packages/new/photos/services/ml/worker.ts +++ b/web/packages/new/photos/services/ml/worker.ts @@ -109,7 +109,13 @@ export class MLWorker { private liveQ: IndexableItem[] = []; private idleTimeout: ReturnType | undefined; private idleDuration = idleDurationStart; /* unit: seconds */ - private onNextIdles: (() => void)[] = []; + /** Resolvers for pending promises returned from calls to {@link index}. */ + private onNextIdles: ((count: number) => void)[] = []; + /** + * Number of items processed since the last time {@link onNextIdles} was + * drained. + */ + private countSinceLastIdle = 0; /** * Initialize a new {@link MLWorker}. @@ -140,9 +146,12 @@ export class MLWorker { * During a backfill, we first attempt to fetch ML data for files which * don't have that data locally. If on fetching we find what we need, we * save it locally. Otherwise we index them. + * + * @return The count of items processed since the last last time we were + * idle. */ index() { - const nextIdle = new Promise((resolve) => + const nextIdle = new Promise((resolve) => this.onNextIdles.push(resolve), ); this.wakeUp(); @@ -225,17 +234,23 @@ export class MLWorker { // Use the liveQ if present, otherwise get the next batch to backfill. const items = liveQ.length ? liveQ : await this.backfillQ(); - const allSuccess = await indexNextBatch( - items, - ensure(this.electron), - this.delegate, - ); - if (allSuccess) { - // Everything is running smoothly. Reset the idle duration. - this.idleDuration = idleDurationStart; - // And tick again. - scheduleTick(); - return; + this.countSinceLastIdle += items.length; + + // If there is items remaining, + if (items.length > 0) { + // Index them. + const allSuccess = await indexNextBatch( + items, + ensure(this.electron), + this.delegate, + ); + if (allSuccess) { + // Everything is running smoothly. Reset the idle duration. + this.idleDuration = idleDurationStart; + // And tick again. + scheduleTick(); + return; + } } // We come here in three scenarios - either there is nothing left to do, @@ -255,8 +270,16 @@ export class MLWorker { // Resolve any awaiting promises returned from `index`. const onNextIdles = this.onNextIdles; + const countSinceLastIdle = this.countSinceLastIdle; this.onNextIdles = []; - onNextIdles.forEach((f) => f()); + this.countSinceLastIdle = 0; + onNextIdles.forEach((f) => f(countSinceLastIdle)); + + // If no one was waiting, then let the main thread know via a different + // channel so that it can update the clusters and people. + if (onNextIdles.length == 0 && countSinceLastIdle > 0) { + this.delegate?.workerDidUnawaitedIndex(); + } } /** Return the next batch of items to backfill (if any). */ @@ -321,13 +344,13 @@ export class MLWorker { expose(MLWorker); /** - * Find out files which need to be indexed. Then index the next batch of them. + * Index the given batch of items. * - * Returns `false` to indicate that either an error occurred, or there are no - * more files to process, or that we cannot currently process files. + * Returns `false` to indicate that either an error occurred, or that we cannot + * currently process files since we don't have network connectivity. * - * Which means that when it returns true, all is well and there are more - * things pending to process, so we should chug along at full speed. + * Which means that when it returns true, all is well and if there are more + * things pending to process, we should chug along at full speed. */ const indexNextBatch = async ( items: IndexableItem[], @@ -342,9 +365,6 @@ const indexNextBatch = async ( return false; } - // Nothing to do. - if (items.length == 0) return false; - // Keep track if any of the items failed. let allSuccess = true;