From 85cf1de12cb332b3913c232bd3fe666a39a6624c Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Tue, 21 Jan 2025 16:28:35 +0530 Subject: [PATCH] All current os/arch combinations are handled --- desktop/src/main/services/image.ts | 12 ++---- desktop/src/types/ipc.ts | 10 ----- .../src/services/upload/upload-service.ts | 29 +------------- web/packages/base/types/ipc.ts | 40 +++---------------- web/packages/gallery/utils/convert.ts | 21 +--------- 5 files changed, 13 insertions(+), 99 deletions(-) diff --git a/desktop/src/main/services/image.ts b/desktop/src/main/services/image.ts index e533a49274..1dc37d9726 100644 --- a/desktop/src/main/services/image.ts +++ b/desktop/src/main/services/image.ts @@ -2,7 +2,7 @@ import fs from "node:fs/promises"; import path from "node:path"; -import { CustomErrorMessage, type ZipItem } from "../../types/ipc"; +import { type ZipItem } from "../../types/ipc"; import { execAsync, isDev } from "../utils/electron"; import { deleteTempFileIgnoringErrors, @@ -45,9 +45,6 @@ const convertToJPEGCommand = ( case "linux": case "win32": - // The bundled binary is for x86 and arm64. - if (process.arch != "x64" && process.arch != "arm64") - throw new Error(CustomErrorMessage.NotAvailable); return [ imageMagickPath(), "convert", @@ -58,7 +55,7 @@ const convertToJPEGCommand = ( ]; default: - throw new Error(CustomErrorMessage.NotAvailable); + throw new Error("Not available on the current OS/arch"); } }; @@ -138,9 +135,6 @@ const generateImageThumbnailCommand = ( case "linux": case "win32": - // The bundled binary is for x86 and arm64. - if (process.arch != "x64" && process.arch != "arm64") - throw new Error(CustomErrorMessage.NotAvailable); return [ imageMagickPath(), "convert", @@ -158,6 +152,6 @@ const generateImageThumbnailCommand = ( ]; default: - throw new Error(CustomErrorMessage.NotAvailable); + throw new Error("Not available on the current OS/arch"); } }; diff --git a/desktop/src/types/ipc.ts b/desktop/src/types/ipc.ts index f4985bfc71..f61cd71a2a 100644 --- a/desktop/src/types/ipc.ts +++ b/desktop/src/types/ipc.ts @@ -32,13 +32,3 @@ export interface PendingUploads { filePaths: string[]; zipItems: ZipItem[]; } - -/** - * See: [Note: Custom errors across Electron/Renderer boundary] - * - * Note: this is not a type, and cannot be used in preload.js; it is only meant - * for use in the main process code. - */ -export const CustomErrorMessage = { - NotAvailable: "This feature in not available on the current OS/arch", -}; diff --git a/web/apps/photos/src/services/upload/upload-service.ts b/web/apps/photos/src/services/upload/upload-service.ts index 2e20395906..beeb0bfc89 100644 --- a/web/apps/photos/src/services/upload/upload-service.ts +++ b/web/apps/photos/src/services/upload/upload-service.ts @@ -5,7 +5,6 @@ import { ensureElectron } from "@/base/electron"; import { basename, nameAndExtension } from "@/base/file-name"; import type { PublicAlbumsCredentials } from "@/base/http"; import log from "@/base/log"; -import { CustomErrorMessage } from "@/base/types/ipc"; import { extractVideoMetadata } from "@/gallery/services/ffmpeg"; import { detectFileTypeInfoFromChunk, @@ -1208,23 +1207,6 @@ const readImageOrVideo = async ( return withThumbnail(uploadItem, fileTypeInfo, fileStream); }; -// TODO(MR): Merge with the uploader -class ModuleState { - /** - * This will be set to true if we get an error from the Node.js side of our - * desktop app telling us that native image thumbnail generation is not - * available for the current OS/arch combination. - * - * That way, we can stop pestering it again and again (saving an IPC - * round-trip). - * - * Note the double negative when it is used. - */ - isNativeImageThumbnailGenerationNotAvailable = false; -} - -const moduleState = new ModuleState(); - /** * Augment the given {@link dataOrStream} with thumbnail information. * @@ -1247,12 +1229,9 @@ const withThumbnail = async ( let hasStaticThumbnail = false; const electron = globalThis.electron; - const notAvailable = - fileTypeInfo.fileType == FileType.image && - moduleState.isNativeImageThumbnailGenerationNotAvailable; // 1. Native thumbnail generation using items's (effective) path. - if (electron && !notAvailable && !(uploadItem instanceof File)) { + if (electron && !(uploadItem instanceof File)) { try { thumbnail = await generateThumbnailNative( electron, @@ -1260,11 +1239,7 @@ const withThumbnail = async ( fileTypeInfo, ); } catch (e) { - if (e.message.endsWith(CustomErrorMessage.NotAvailable)) { - moduleState.isNativeImageThumbnailGenerationNotAvailable = true; - } else { - log.error("Native thumbnail generation failed", e); - } + log.error("Native thumbnail generation failed", e); } } diff --git a/web/packages/base/types/ipc.ts b/web/packages/base/types/ipc.ts index 31888169e5..0a6fe53483 100644 --- a/web/packages/base/types/ipc.ts +++ b/web/packages/base/types/ipc.ts @@ -271,13 +271,9 @@ export interface Electron { /** * Try to convert an arbitrary image into JPEG using native layer tools. * - * The behaviour is OS dependent. On macOS we use the `sips` utility, and on - * some Linux architectures we use an ImageMagick executable bundled with - * our desktop app. - * - * In other cases (primarily Windows), where native JPEG conversion is not - * yet possible, this function will throw an error with the - * {@link CustomErrorMessage.NotAvailable} message. + * The behaviour is OS dependent. On macOS we use the `sips` utility, while + * on Linux and Windows we an ImageMagick executable bundled with our + * desktop app. * * @param imageData The raw image data (the contents of the image file). * @@ -288,13 +284,9 @@ export interface Electron { /** * Generate a JPEG thumbnail for the given image. * - * The behaviour is OS dependent. On macOS we use the `sips` utility, and on - * some Linux architectures we use an ImageMagick executable bundled with - * our desktop app. - * - * In other cases (primarily Windows), where native thumbnail generation is - * not yet possible, this function will throw an error with the - * {@link CustomErrorMessage.NotAvailable} message. + * The behaviour is OS dependent. On macOS we use the `sips` utility, while + * on Linux and Windows we an ImageMagick executable bundled with our + * desktop app. * * @param dataOrPathOrZipItem The file whose thumbnail we want to generate. * It can be provided as raw image data (the contents of the image file), or @@ -610,26 +602,6 @@ export interface ElectronMLWorker { computeFaceEmbeddings: (input: Float32Array) => Promise; } -/** - * Errors that have special semantics on the web side. - * - * [Note: Custom errors across Electron/Renderer boundary] - * - * If we need to identify errors thrown by the main process when invoked from - * the renderer process, we can only use the `message` field because: - * - * > Errors thrown throw `handle` in the main process are not transparent as - * > they are serialized and only the `message` property from the original error - * > is provided to the renderer process. - * > - * > - https://www.electronjs.org/docs/latest/tutorial/ipc - * > - * > Ref: https://github.com/electron/electron/issues/24427 - */ -export const CustomErrorMessage = { - NotAvailable: "This feature in not available on the current OS/arch", -}; - /** * Data passed across the IPC bridge when an app update is available. */ diff --git a/web/packages/gallery/utils/convert.ts b/web/packages/gallery/utils/convert.ts index 4d5db902fd..67a571f3f5 100644 --- a/web/packages/gallery/utils/convert.ts +++ b/web/packages/gallery/utils/convert.ts @@ -1,21 +1,11 @@ import { isDesktop } from "@/base/app"; import log from "@/base/log"; -import { CustomErrorMessage } from "@/base/types/ipc"; import { workerBridge } from "@/base/worker/worker-bridge"; import { isHEICExtension, needsJPEGConversion } from "@/media/formats"; import { heicToJPEG } from "@/media/heic-convert"; import { convertToMP4 } from "../services/ffmpeg"; import { detectFileTypeInfo } from "./detect-type"; -/** - * This will be set to false if we get an error from the Node.js side of our - * desktop app telling us that native JPEG conversion is not available for the - * current OS/arch combination. - * - * That way, we can stop pestering it again and again, saving an IPC round-trip. - */ -let _isNativeJPEGConversionAvailable = true; - /** * Return a new {@link Blob} containing an image's data in a format that the * browser (likely) knows how to render (in an img tag, or on the canvas). @@ -62,18 +52,11 @@ export const renderableImageBlob = async ( // If we're running in our desktop app, see if our Node.js layer can // convert this into a JPEG using native tools. - if (isDesktop && _isNativeJPEGConversionAvailable) { + if (isDesktop) { try { return await nativeConvertToJPEG(imageBlob); } catch (e) { - if ( - e instanceof Error && - e.message.endsWith(CustomErrorMessage.NotAvailable) - ) { - _isNativeJPEGConversionAvailable = false; - } else { - log.error("Native conversion to JPEG failed", e); - } + log.error("Native conversion to JPEG failed", e); } }