about summary refs log tree commit diff
path: root/src
diff options
context:
space:
mode:
authordan <dan.abramov@gmail.com>2024-11-04 21:28:27 +0000
committerGitHub <noreply@github.com>2024-11-04 21:28:27 +0000
commit174988bc5ab00774d200a882312985f55d903d81 (patch)
treeaf52d6f05093ceeea3e3293db0cbab3d5cf43156 /src
parentac9d910e1e77c559eff8b32cd8412335f41074f1 (diff)
downloadvoidsky-174988bc5ab00774d200a882312985f55d903d81.tar.zst
Unify dimensions cache between lightbox and feed (#6047)
* Remove useless memo

* Use explicit values when useImageAspectRatio doesn't know

It's not very good that you can't distingiush when we haven't loaded vs when we're certain. This shifts the burden of dealing with missing values to the caller.

* Check cache early

* Handle src change

* Rewrite image-sizes.fetch to avoid mixing async styles

* Make image-sizes LRU

Code is copy paste from useImageDimensions.ts

* Rm unused fields

* Derive aspect on the fly

* Factor useImageDimensions out of useImageAspectRatio

* Move useImageDimensions into image-sizes

* Make lightbox use the same cache

* Wire up known dimensions to the lightbox

* Handle division by zero in the hook

* Use safe aspect for lightbox calculations
Diffstat (limited to 'src')
-rw-r--r--src/lib/media/image-sizes.ts100
-rw-r--r--src/state/lightbox.tsx2
-rw-r--r--src/view/com/lightbox/ImageViewing/@types/index.ts7
-rw-r--r--src/view/com/lightbox/ImageViewing/components/ImageItem/ImageItem.android.tsx17
-rw-r--r--src/view/com/lightbox/ImageViewing/components/ImageItem/ImageItem.ios.tsx16
-rw-r--r--src/view/com/lightbox/ImageViewing/hooks/useImageDimensions.ts93
-rw-r--r--src/view/com/lightbox/Lightbox.tsx10
-rw-r--r--src/view/com/profile/ProfileSubpageHeader.tsx12
-rw-r--r--src/view/com/util/images/AutoSizedImage.tsx61
-rw-r--r--src/view/com/util/post-embeds/index.tsx2
10 files changed, 144 insertions, 176 deletions
diff --git a/src/lib/media/image-sizes.ts b/src/lib/media/image-sizes.ts
index 7a1555688..8eaa9467f 100644
--- a/src/lib/media/image-sizes.ts
+++ b/src/lib/media/image-sizes.ts
@@ -1,35 +1,93 @@
+import {useEffect, useState} from 'react'
 import {Image} from 'react-native'
 
 import type {Dimensions} from '#/lib/media/types'
 
-const sizes: Map<string, Dimensions> = new Map()
+type CacheStorageItem<T> = {key: string; value: T}
+const createCache = <T>(cacheSize: number) => ({
+  _storage: [] as CacheStorageItem<T>[],
+  get(key: string) {
+    const {value} =
+      this._storage.find(({key: storageKey}) => storageKey === key) || {}
+    return value
+  },
+  set(key: string, value: T) {
+    if (this._storage.length >= cacheSize) {
+      this._storage.shift()
+    }
+    this._storage.push({key, value})
+  },
+})
+
+const sizes = createCache<Dimensions>(50)
 const activeRequests: Map<string, Promise<Dimensions>> = new Map()
 
 export function get(uri: string): Dimensions | undefined {
   return sizes.get(uri)
 }
 
-export async function fetch(uri: string): Promise<Dimensions> {
-  const Dimensions = sizes.get(uri)
-  if (Dimensions) {
-    return Dimensions
+export function fetch(uri: string): Promise<Dimensions> {
+  const dims = sizes.get(uri)
+  if (dims) {
+    return Promise.resolve(dims)
+  }
+  const activeRequest = activeRequests.get(uri)
+  if (activeRequest) {
+    return activeRequest
+  }
+  const prom = new Promise<Dimensions>((resolve, reject) => {
+    Image.getSize(
+      uri,
+      (width: number, height: number) => {
+        const size = {width, height}
+        sizes.set(uri, size)
+        resolve(size)
+      },
+      (err: any) => {
+        console.error('Failed to fetch image dimensions for', uri, err)
+        reject(new Error('Could not fetch dimensions'))
+      },
+    )
+  }).finally(() => {
+    activeRequests.delete(uri)
+  })
+  activeRequests.set(uri, prom)
+  return prom
+}
+
+export function useImageDimensions({
+  src,
+  knownDimensions,
+}: {
+  src: string
+  knownDimensions: Dimensions | null
+}): [number | undefined, Dimensions | undefined] {
+  const [dims, setDims] = useState(() => knownDimensions ?? get(src))
+  const [prevSrc, setPrevSrc] = useState(src)
+  if (src !== prevSrc) {
+    setDims(knownDimensions ?? get(src))
+    setPrevSrc(src)
   }
 
-  const prom =
-    activeRequests.get(uri) ||
-    new Promise<Dimensions>(resolve => {
-      Image.getSize(
-        uri,
-        (width: number, height: number) => resolve({width, height}),
-        (err: any) => {
-          console.error('Failed to fetch image dimensions for', uri, err)
-          resolve({width: 0, height: 0})
-        },
-      )
+  useEffect(() => {
+    let aborted = false
+    if (dims !== undefined) return
+    fetch(src).then(newDims => {
+      if (aborted) return
+      setDims(newDims)
     })
-  activeRequests.set(uri, prom)
-  const res = await prom
-  activeRequests.delete(uri)
-  sizes.set(uri, res)
-  return res
+    return () => {
+      aborted = true
+    }
+  }, [dims, setDims, src])
+
+  let aspectRatio: number | undefined
+  if (dims) {
+    aspectRatio = dims.width / dims.height
+    if (Number.isNaN(aspectRatio)) {
+      aspectRatio = undefined
+    }
+  }
+
+  return [aspectRatio, dims]
 }
diff --git a/src/state/lightbox.tsx b/src/state/lightbox.tsx
index eb5a88864..53409e3ec 100644
--- a/src/state/lightbox.tsx
+++ b/src/state/lightbox.tsx
@@ -3,6 +3,7 @@ import type {MeasuredDimensions} from 'react-native-reanimated'
 import {AppBskyActorDefs} from '@atproto/api'
 
 import {useNonReactiveCallback} from '#/lib/hooks/useNonReactiveCallback'
+import {Dimensions} from '#/lib/media/types'
 
 type ProfileImageLightbox = {
   type: 'profile-image'
@@ -14,6 +15,7 @@ type ImagesLightboxItem = {
   uri: string
   thumbUri: string
   alt?: string
+  dimensions: Dimensions | null
 }
 
 type ImagesLightbox = {
diff --git a/src/view/com/lightbox/ImageViewing/@types/index.ts b/src/view/com/lightbox/ImageViewing/@types/index.ts
index 8fdc3f364..f5ab8bba9 100644
--- a/src/view/com/lightbox/ImageViewing/@types/index.ts
+++ b/src/view/com/lightbox/ImageViewing/@types/index.ts
@@ -16,4 +16,9 @@ export type Position = {
   y: number
 }
 
-export type ImageSource = {uri: string; thumbUri: string; alt?: string}
+export type ImageSource = {
+  uri: string
+  thumbUri: string
+  alt?: string
+  dimensions: Dimensions | null
+}
diff --git a/src/view/com/lightbox/ImageViewing/components/ImageItem/ImageItem.android.tsx b/src/view/com/lightbox/ImageViewing/components/ImageItem/ImageItem.android.tsx
index 3de15b379..487acf931 100644
--- a/src/view/com/lightbox/ImageViewing/components/ImageItem/ImageItem.android.tsx
+++ b/src/view/com/lightbox/ImageViewing/components/ImageItem/ImageItem.android.tsx
@@ -12,8 +12,8 @@ import Animated, {
 } from 'react-native-reanimated'
 import {Image} from 'expo-image'
 
+import {useImageDimensions} from '#/lib/media/image-sizes'
 import type {Dimensions as ImageDimensions, ImageSource} from '../../@types'
-import useImageDimensions from '../../hooks/useImageDimensions'
 import {
   applyRounding,
   createTransform,
@@ -52,7 +52,10 @@ const ImageItem = ({
   isScrollViewBeingDragged,
 }: Props) => {
   const [isScaled, setIsScaled] = useState(false)
-  const imageDimensions = useImageDimensions(imageSrc)
+  const [imageAspect, imageDimensions] = useImageDimensions({
+    src: imageSrc.uri,
+    knownDimensions: imageSrc.dimensions,
+  })
   const committedTransform = useSharedValue(initialTransform)
   const panTranslation = useSharedValue({x: 0, y: 0})
   const pinchOrigin = useSharedValue({x: 0, y: 0})
@@ -119,12 +122,12 @@ const ImageItem = ({
     candidateTransform: TransformMatrix,
   ) {
     'worklet'
-    if (!imageDimensions) {
+    if (!imageAspect) {
       return [0, 0]
     }
     const [nextTranslateX, nextTranslateY, nextScale] =
       readTransform(candidateTransform)
-    const scaledDimensions = getScaledDimensions(imageDimensions, nextScale)
+    const scaledDimensions = getScaledDimensions(imageAspect, nextScale)
     const clampedTranslateX = clampTranslation(
       nextTranslateX,
       scaledDimensions.width,
@@ -248,7 +251,7 @@ const ImageItem = ({
     .numberOfTaps(2)
     .onEnd(e => {
       'worklet'
-      if (!imageDimensions) {
+      if (!imageDimensions || !imageAspect) {
         return
       }
       const [, , committedScale] = readTransform(committedTransform.value)
@@ -260,7 +263,6 @@ const ImageItem = ({
       }
 
       // Try to zoom in so that we get rid of the black bars (whatever the orientation was).
-      const imageAspect = imageDimensions.width / imageDimensions.height
       const screenAspect = SCREEN.width / SCREEN.height
       const candidateScale = Math.max(
         imageAspect / screenAspect,
@@ -363,11 +365,10 @@ const styles = StyleSheet.create({
 })
 
 function getScaledDimensions(
-  imageDimensions: ImageDimensions,
+  imageAspect: number,
   scale: number,
 ): ImageDimensions {
   'worklet'
-  const imageAspect = imageDimensions.width / imageDimensions.height
   const screenAspect = SCREEN.width / SCREEN.height
   const isLandscape = imageAspect > screenAspect
   if (isLandscape) {
diff --git a/src/view/com/lightbox/ImageViewing/components/ImageItem/ImageItem.ios.tsx b/src/view/com/lightbox/ImageViewing/components/ImageItem/ImageItem.ios.tsx
index f21953498..a96a1c913 100644
--- a/src/view/com/lightbox/ImageViewing/components/ImageItem/ImageItem.ios.tsx
+++ b/src/view/com/lightbox/ImageViewing/components/ImageItem/ImageItem.ios.tsx
@@ -19,8 +19,8 @@ import Animated, {
 import {Image} from 'expo-image'
 
 import {useAnimatedScrollHandler} from '#/lib/hooks/useAnimatedScrollHandler_FIXED'
-import {Dimensions as ImageDimensions, ImageSource} from '../../@types'
-import useImageDimensions from '../../hooks/useImageDimensions'
+import {useImageDimensions} from '#/lib/media/image-sizes'
+import {ImageSource} from '../../@types'
 
 const SWIPE_CLOSE_OFFSET = 75
 const SWIPE_CLOSE_VELOCITY = 1
@@ -47,7 +47,10 @@ const ImageItem = ({
   const scrollViewRef = useAnimatedRef<Animated.ScrollView>()
   const translationY = useSharedValue(0)
   const [scaled, setScaled] = useState(false)
-  const imageDimensions = useImageDimensions(imageSrc)
+  const [imageAspect, imageDimensions] = useImageDimensions({
+    src: imageSrc.uri,
+    knownDimensions: imageSrc.dimensions,
+  })
   const maxZoomScale = imageDimensions
     ? (imageDimensions.width / SCREEN.width) * MAX_ORIGINAL_IMAGE_ZOOM
     : 1
@@ -99,7 +102,7 @@ const ImageItem = ({
     const willZoom = !scaled
     if (willZoom) {
       nextZoomRect = getZoomRectAfterDoubleTap(
-        imageDimensions,
+        imageAspect,
         absoluteX,
         absoluteY,
       )
@@ -179,7 +182,7 @@ const styles = StyleSheet.create({
 })
 
 const getZoomRectAfterDoubleTap = (
-  imageDimensions: ImageDimensions | null,
+  imageAspect: number | undefined,
   touchX: number,
   touchY: number,
 ): {
@@ -188,7 +191,7 @@ const getZoomRectAfterDoubleTap = (
   width: number
   height: number
 } => {
-  if (!imageDimensions) {
+  if (!imageAspect) {
     return {
       x: 0,
       y: 0,
@@ -199,7 +202,6 @@ const getZoomRectAfterDoubleTap = (
 
   // First, let's figure out how much we want to zoom in.
   // We want to try to zoom in at least close enough to get rid of black bars.
-  const imageAspect = imageDimensions.width / imageDimensions.height
   const screenAspect = SCREEN.width / SCREEN.height
   const zoom = Math.max(
     imageAspect / screenAspect,
diff --git a/src/view/com/lightbox/ImageViewing/hooks/useImageDimensions.ts b/src/view/com/lightbox/ImageViewing/hooks/useImageDimensions.ts
deleted file mode 100644
index 8b5bc1b87..000000000
--- a/src/view/com/lightbox/ImageViewing/hooks/useImageDimensions.ts
+++ /dev/null
@@ -1,93 +0,0 @@
-/**
- * Copyright (c) JOB TODAY S.A. and its affiliates.
- *
- * This source code is licensed under the MIT license found in the
- * LICENSE file in the root directory of this source tree.
- *
- */
-
-import {useEffect, useState} from 'react'
-import {Image, ImageURISource} from 'react-native'
-
-import {Dimensions, ImageSource} from '../@types'
-
-const CACHE_SIZE = 50
-
-type CacheStorageItem = {key: string; value: any}
-
-const createCache = (cacheSize: number) => ({
-  _storage: [] as CacheStorageItem[],
-  get(key: string): any {
-    const {value} =
-      this._storage.find(({key: storageKey}) => storageKey === key) || {}
-
-    return value
-  },
-  set(key: string, value: any) {
-    if (this._storage.length >= cacheSize) {
-      this._storage.shift()
-    }
-
-    this._storage.push({key, value})
-  },
-})
-
-const imageDimensionsCache = createCache(CACHE_SIZE)
-
-const useImageDimensions = (image: ImageSource): Dimensions | null => {
-  const [dimensions, setDimensions] = useState<Dimensions | null>(null)
-
-  const getImageDimensions = (
-    image: ImageSource,
-  ): Promise<Dimensions | null> => {
-    return new Promise(resolve => {
-      if (image.uri) {
-        const source = image as ImageURISource
-        const cacheKey = source.uri as string
-        const imageDimensions = imageDimensionsCache.get(cacheKey)
-        if (imageDimensions) {
-          resolve(imageDimensions)
-        } else {
-          Image.getSizeWithHeaders(
-            // @ts-ignore
-            source.uri,
-            source.headers,
-            (width: number, height: number) => {
-              if (width > 0 && height > 0) {
-                imageDimensionsCache.set(cacheKey, {width, height})
-                resolve({width, height})
-              } else {
-                resolve(null)
-              }
-            },
-            () => {
-              resolve(null)
-            },
-          )
-        }
-      } else {
-        resolve(null)
-      }
-    })
-  }
-
-  let isImageUnmounted = false
-
-  useEffect(() => {
-    // eslint-disable-next-line @typescript-eslint/no-shadow
-    getImageDimensions(image).then(dimensions => {
-      if (!isImageUnmounted) {
-        setDimensions(dimensions)
-      }
-    })
-
-    return () => {
-      // eslint-disable-next-line react-hooks/exhaustive-deps
-      isImageUnmounted = true
-    }
-  }, [image])
-
-  return dimensions
-}
-
-export default useImageDimensions
diff --git a/src/view/com/lightbox/Lightbox.tsx b/src/view/com/lightbox/Lightbox.tsx
index 891be3f9c..83ea2e941 100644
--- a/src/view/com/lightbox/Lightbox.tsx
+++ b/src/view/com/lightbox/Lightbox.tsx
@@ -32,7 +32,15 @@ export function Lightbox() {
     return (
       <ImageView
         images={[
-          {uri: opts.profile.avatar || '', thumbUri: opts.profile.avatar || ''},
+          {
+            uri: opts.profile.avatar || '',
+            thumbUri: opts.profile.avatar || '',
+            dimensions: {
+              // It's fine if it's actually smaller but we know it's 1:1.
+              height: 1000,
+              width: 1000,
+            },
+          },
         ]}
         initialImageIndex={0}
         thumbDims={opts.thumbDims}
diff --git a/src/view/com/profile/ProfileSubpageHeader.tsx b/src/view/com/profile/ProfileSubpageHeader.tsx
index 785080c4b..d5b7c2b68 100644
--- a/src/view/com/profile/ProfileSubpageHeader.tsx
+++ b/src/view/com/profile/ProfileSubpageHeader.tsx
@@ -72,7 +72,17 @@ export function ProfileSubpageHeader({
     ) {
       openLightbox({
         type: 'images',
-        images: [{uri: avatar, thumbUri: avatar}],
+        images: [
+          {
+            uri: avatar,
+            thumbUri: avatar,
+            dimensions: {
+              // It's fine if it's actually smaller but we know it's 1:1.
+              height: 1000,
+              width: 1000,
+            },
+          },
+        ],
         index: 0,
         thumbDims: null,
       })
diff --git a/src/view/com/util/images/AutoSizedImage.tsx b/src/view/com/util/images/AutoSizedImage.tsx
index a9bfc1c96..ce2389ce2 100644
--- a/src/view/com/util/images/AutoSizedImage.tsx
+++ b/src/view/com/util/images/AutoSizedImage.tsx
@@ -5,7 +5,7 @@ import {AppBskyEmbedImages} from '@atproto/api'
 import {msg} from '@lingui/macro'
 import {useLingui} from '@lingui/react'
 
-import * as imageSizes from '#/lib/media/image-sizes'
+import {useImageDimensions} from '#/lib/media/image-sizes'
 import {Dimensions} from '#/lib/media/types'
 import {isNative} from '#/platform/detection'
 import {useLargeAltBadgeEnabled} from '#/state/preferences/large-alt-badge'
@@ -14,44 +14,24 @@ import {ArrowsDiagonalOut_Stroke2_Corner0_Rounded as Fullscreen} from '#/compone
 import {MediaInsetBorder} from '#/components/MediaInsetBorder'
 import {Text} from '#/components/Typography'
 
-export function useImageAspectRatio({
+function useImageAspectRatio({
   src,
-  dimensions,
+  knownDimensions,
 }: {
   src: string
-  dimensions: Dimensions | undefined
+  knownDimensions: Dimensions | null
 }) {
-  const [raw, setAspectRatio] = React.useState<number>(
-    dimensions ? calc(dimensions) : 1,
-  )
-  // this basically controls the width of the image
-  const {isCropped, constrained, max} = React.useMemo(() => {
+  const [raw] = useImageDimensions({src, knownDimensions})
+  let constrained: number | undefined
+  let max: number | undefined
+  let isCropped: boolean | undefined
+  if (raw !== undefined) {
     const ratio = 1 / 2 // max of 1:2 ratio in feeds
-    const constrained = Math.max(raw, ratio)
-    const max = Math.max(raw, 0.25) // max of 1:4 in thread
-    const isCropped = raw < constrained
-    return {
-      isCropped,
-      constrained,
-      max,
-    }
-  }, [raw])
-
-  React.useEffect(() => {
-    let aborted = false
-    if (dimensions) return
-    imageSizes.fetch(src).then(newDim => {
-      if (aborted) return
-      setAspectRatio(calc(newDim))
-    })
-    return () => {
-      aborted = true
-    }
-  }, [dimensions, setAspectRatio, src])
-
+    constrained = Math.max(raw, ratio)
+    max = Math.max(raw, 0.25) // max of 1:4 in thread
+    isCropped = raw < constrained
+  }
   return {
-    dimensions,
-    raw,
     constrained,
     max,
     isCropped,
@@ -125,7 +105,7 @@ export function AutoSizedImage({
     isCropped: rawIsCropped,
   } = useImageAspectRatio({
     src: image.thumb,
-    dimensions: image.aspectRatio,
+    knownDimensions: image.aspectRatio ?? null,
   })
   const cropDisabled = crop === 'none'
   const isCropped = rawIsCropped && !cropDisabled
@@ -222,14 +202,16 @@ export function AutoSizedImage({
           a.rounded_md,
           a.overflow_hidden,
           t.atoms.bg_contrast_25,
-          {aspectRatio: max},
+          {aspectRatio: max ?? 1},
         ]}>
         {contents}
       </Pressable>
     )
   } else {
     return (
-      <ConstrainedImage fullBleed={crop === 'square'} aspectRatio={constrained}>
+      <ConstrainedImage
+        fullBleed={crop === 'square'}
+        aspectRatio={constrained ?? 1}>
         <Pressable
           onPress={onPress}
           onLongPress={onLongPress}
@@ -244,10 +226,3 @@ export function AutoSizedImage({
     )
   }
 }
-
-function calc(dim: Dimensions) {
-  if (dim.width === 0 || dim.height === 0) {
-    return 1
-  }
-  return dim.width / dim.height
-}
diff --git a/src/view/com/util/post-embeds/index.tsx b/src/view/com/util/post-embeds/index.tsx
index 19769bcd7..13e4d12e0 100644
--- a/src/view/com/util/post-embeds/index.tsx
+++ b/src/view/com/util/post-embeds/index.tsx
@@ -145,7 +145,7 @@ export function PostEmbeds({
         uri: img.fullsize,
         thumbUri: img.thumb,
         alt: img.alt,
-        aspectRatio: img.aspectRatio,
+        dimensions: img.aspectRatio ?? null,
       }))
       const _openLightbox = (
         index: number,