From 57ad12e43b121ad134f8a7696acc2d915057cde5 Mon Sep 17 00:00:00 2001 From: WithoutPants <53250216+WithoutPants@users.noreply.github.com> Date: Fri, 25 Nov 2022 11:14:50 +1100 Subject: [PATCH] Improve gallery performance (#3183) * Improve cover resolver performance * Deprecate and remove usage of slow gallery images --- graphql/documents/data/gallery.graphql | 3 - graphql/schema/types/gallery.graphql | 2 +- internal/api/resolver_model_gallery.go | 22 ++----- pkg/image/image.go | 12 ---- pkg/image/image_test.go | 34 ---------- pkg/image/query.go | 58 +++++++++++++++++ .../components/Galleries/GalleryViewer.tsx | 42 +++++++++++-- ui/v2.5/src/docs/en/Changelog/v0180.md | 1 + ui/v2.5/src/hooks/Lightbox/Lightbox.tsx | 48 +++++++++----- ui/v2.5/src/hooks/Lightbox/hooks.ts | 63 ++++++++++++++++--- 10 files changed, 188 insertions(+), 97 deletions(-) delete mode 100644 pkg/image/image.go delete mode 100644 pkg/image/image_test.go diff --git a/graphql/documents/data/gallery.graphql b/graphql/documents/data/gallery.graphql index f23e34b52..bb804047e 100644 --- a/graphql/documents/data/gallery.graphql +++ b/graphql/documents/data/gallery.graphql @@ -16,9 +16,6 @@ fragment GalleryData on Gallery { ...FolderData } - images { - ...SlimImageData - } cover { ...SlimImageData } diff --git a/graphql/schema/types/gallery.graphql b/graphql/schema/types/gallery.graphql index 2f7916a14..3716b9478 100644 --- a/graphql/schema/types/gallery.graphql +++ b/graphql/schema/types/gallery.graphql @@ -26,7 +26,7 @@ type Gallery { performers: [Performer!]! """The images in the gallery""" - images: [Image!]! # Resolver + images: [Image!]! @deprecated(reason: "Use findImages") cover: Image } diff --git a/internal/api/resolver_model_gallery.go b/internal/api/resolver_model_gallery.go index 72a061eb5..43c5b2221 100644 --- a/internal/api/resolver_model_gallery.go +++ b/internal/api/resolver_model_gallery.go @@ -123,6 +123,7 @@ func (r *galleryResolver) FileModTime(ctx context.Context, obj *models.Gallery) return nil, nil } +// Images is deprecated, slow and shouldn't be used func (r *galleryResolver) Images(ctx context.Context, obj *models.Gallery) (ret []*models.Image, err error) { if err := r.withReadTxn(ctx, func(ctx context.Context) error { var err error @@ -144,24 +145,9 @@ func (r *galleryResolver) Images(ctx context.Context, obj *models.Gallery) (ret func (r *galleryResolver) Cover(ctx context.Context, obj *models.Gallery) (ret *models.Image, err error) { if err := r.withReadTxn(ctx, func(ctx context.Context) error { - // doing this via Query is really slow, so stick with FindByGalleryID - imgs, err := r.repository.Image.FindByGalleryID(ctx, obj.ID) - if err != nil { - return err - } - - if len(imgs) > 0 { - ret = imgs[0] - } - - for _, img := range imgs { - if image.IsCover(img) { - ret = img - break - } - } - - return nil + // find cover.jpg first + ret, err = image.FindGalleryCover(ctx, r.repository.Image, obj.ID) + return err }); err != nil { return nil, err } diff --git a/pkg/image/image.go b/pkg/image/image.go deleted file mode 100644 index 00c8b3be2..000000000 --- a/pkg/image/image.go +++ /dev/null @@ -1,12 +0,0 @@ -package image - -import ( - "strings" - - "github.com/stashapp/stash/pkg/models" - _ "golang.org/x/image/webp" -) - -func IsCover(img *models.Image) bool { - return strings.HasSuffix(img.Path, "cover.jpg") -} diff --git a/pkg/image/image_test.go b/pkg/image/image_test.go deleted file mode 100644 index 3188a63d5..000000000 --- a/pkg/image/image_test.go +++ /dev/null @@ -1,34 +0,0 @@ -package image - -import ( - "fmt" - "path/filepath" - "testing" - - "github.com/stashapp/stash/pkg/models" - "github.com/stretchr/testify/assert" -) - -func TestIsCover(t *testing.T) { - type test struct { - fn string - isCover bool - } - - tests := []test{ - {"cover.jpg", true}, - {"covernot.jpg", false}, - {"Cover.jpg", false}, - {fmt.Sprintf("subDir%scover.jpg", string(filepath.Separator)), true}, - {"endsWithcover.jpg", true}, - {"cover.png", false}, - } - - assert := assert.New(t) - for _, tc := range tests { - img := &models.Image{ - Path: tc.fn, - } - assert.Equal(tc.isCover, IsCover(img), "expected: %t for %s", tc.isCover, tc.fn) - } -} diff --git a/pkg/image/query.go b/pkg/image/query.go index 36ed3a8c3..45f1cb687 100644 --- a/pkg/image/query.go +++ b/pkg/image/query.go @@ -7,6 +7,11 @@ import ( "github.com/stashapp/stash/pkg/models" ) +const ( + coverFilename = "cover.jpg" + coverFilenameSearchString = "%" + coverFilename +) + type Queryer interface { Query(ctx context.Context, options models.ImageQueryOptions) (*models.ImageQueryResult, error) } @@ -96,3 +101,56 @@ func FindByGalleryID(ctx context.Context, r Queryer, galleryID int, sortBy strin }, }, &findFilter) } + +func FindGalleryCover(ctx context.Context, r Queryer, galleryID int) (*models.Image, error) { + const useCoverJpg = true + img, err := findGalleryCover(ctx, r, galleryID, useCoverJpg) + if err != nil { + return nil, err + } + + if img != nil { + return img, nil + } + + // return the first image in the gallery + return findGalleryCover(ctx, r, galleryID, !useCoverJpg) +} + +func findGalleryCover(ctx context.Context, r Queryer, galleryID int, useCoverJpg bool) (*models.Image, error) { + // try to find cover.jpg in the gallery + perPage := 1 + sortBy := "path" + sortDir := models.SortDirectionEnumAsc + + findFilter := models.FindFilterType{ + PerPage: &perPage, + Sort: &sortBy, + Direction: &sortDir, + } + + imageFilter := &models.ImageFilterType{ + Galleries: &models.MultiCriterionInput{ + Value: []string{strconv.Itoa(galleryID)}, + Modifier: models.CriterionModifierIncludes, + }, + } + + if useCoverJpg { + imageFilter.Path = &models.StringCriterionInput{ + Value: coverFilenameSearchString, + Modifier: models.CriterionModifierEquals, + } + } + + imgs, err := Query(ctx, r, imageFilter, &findFilter) + if err != nil { + return nil, err + } + + if len(imgs) > 0 { + return imgs[0], nil + } + + return nil, nil +} diff --git a/ui/v2.5/src/components/Galleries/GalleryViewer.tsx b/ui/v2.5/src/components/Galleries/GalleryViewer.tsx index 3c3006310..868f63dc1 100644 --- a/ui/v2.5/src/components/Galleries/GalleryViewer.tsx +++ b/ui/v2.5/src/components/Galleries/GalleryViewer.tsx @@ -1,17 +1,49 @@ -import React from "react"; -import { useFindGallery } from "src/core/StashService"; +import React, { useMemo } from "react"; import { useLightbox } from "src/hooks"; import { LoadingIndicator } from "src/components/Shared"; import "flexbin/flexbin.css"; +import { + CriterionModifier, + useFindImagesQuery, +} from "src/core/generated-graphql"; interface IProps { galleryId: string; } export const GalleryViewer: React.FC = ({ galleryId }) => { - const { data, loading } = useFindGallery(galleryId); - const images = data?.findGallery?.images ?? []; - const showLightbox = useLightbox({ images, showNavigation: false }); + // TODO - add paging - don't load all images at once + const pageSize = -1; + + const currentFilter = useMemo(() => { + return { + per_page: pageSize, + sort: "path", + }; + }, [pageSize]); + + const { data, loading } = useFindImagesQuery({ + variables: { + filter: currentFilter, + image_filter: { + galleries: { + modifier: CriterionModifier.Includes, + value: [galleryId], + }, + }, + }, + }); + + const images = useMemo(() => data?.findImages?.images ?? [], [data]); + + const lightboxState = useMemo(() => { + return { + images, + showNavigation: false, + }; + }, [images]); + + const showLightbox = useLightbox(lightboxState); if (loading) return ; diff --git a/ui/v2.5/src/docs/en/Changelog/v0180.md b/ui/v2.5/src/docs/en/Changelog/v0180.md index 404e3e61a..a4855ca59 100644 --- a/ui/v2.5/src/docs/en/Changelog/v0180.md +++ b/ui/v2.5/src/docs/en/Changelog/v0180.md @@ -13,6 +13,7 @@ * Added tag description filter criterion. ([#3011](https://github.com/stashapp/stash/pull/3011)) ### 🎨 Improvements +* Improved performance viewing galleries with many images. ([#3183](https://github.com/stashapp/stash/pull/3183)) * Generated heatmaps now only show ranges within the duration of the scene. ([#3182](https://github.com/stashapp/stash/pull/3182)) * Added File Modification Time to File Info panels. ([#3054](https://github.com/stashapp/stash/pull/3054)) * Added counter to File Info tabs for objects with multiple files. ([#3054](https://github.com/stashapp/stash/pull/3054)) diff --git a/ui/v2.5/src/hooks/Lightbox/Lightbox.tsx b/ui/v2.5/src/hooks/Lightbox/Lightbox.tsx index cc6533882..a16a765cc 100644 --- a/ui/v2.5/src/hooks/Lightbox/Lightbox.tsx +++ b/ui/v2.5/src/hooks/Lightbox/Lightbox.tsx @@ -98,6 +98,8 @@ export const LightboxComponent: React.FC = ({ const [isSwitchingPage, setIsSwitchingPage] = useState(true); const [isFullscreen, setFullscreen] = useState(false); const [showOptions, setShowOptions] = useState(false); + const [imagesLoaded, setImagesLoaded] = useState(0); + const [navOffset, setNavOffset] = useState(); const oldImages = useRef([]); @@ -191,7 +193,6 @@ export const LightboxComponent: React.FC = ({ useEffect(() => { if (images !== oldImages.current && isSwitchingPage) { - oldImages.current = images; if (index === -1) setIndex(images.length - 1); setIsSwitchingPage(false); } @@ -220,30 +221,33 @@ export const LightboxComponent: React.FC = ({ } setResetPosition((r) => !r); - if (carouselRef.current) - carouselRef.current.style.left = `${index * -100}vw`; - if (indicatorRef.current) - indicatorRef.current.innerHTML = `${index + 1} / ${images.length}`; + oldIndex.current = index; + }, [index, images.length, lightboxSettings?.resetZoomOnNav]); + + const getNavOffset = useCallback(() => { + if (images.length < 2) return; + if (index === undefined || index === null) return; + if (navRef.current) { const currentThumb = navRef.current.children[index + 1]; if (currentThumb instanceof HTMLImageElement) { const offset = -1 * (currentThumb.offsetLeft - document.documentElement.clientWidth / 2); - navRef.current.style.left = `${offset}px`; - const previouslySelected = navRef.current.getElementsByClassName( - CLASSNAME_NAVSELECTED - )?.[0]; - if (previouslySelected) - previouslySelected.className = CLASSNAME_NAVIMAGE; - - currentThumb.className = `${CLASSNAME_NAVIMAGE} ${CLASSNAME_NAVSELECTED}`; + return { left: `${offset}px` }; } } + }, [index, images.length]); - oldIndex.current = index; - }, [index, images.length, lightboxSettings?.resetZoomOnNav]); + useEffect(() => { + // reset images loaded counter for new images + setImagesLoaded(0); + }, [images]); + + useEffect(() => { + setNavOffset(getNavOffset() ?? undefined); + }, [getNavOffset]); useEffect(() => { if (displayMode !== oldDisplayMode.current) { @@ -313,6 +317,7 @@ export const LightboxComponent: React.FC = ({ if (pageCallback) { pageCallback(-1); setIndex(-1); + oldImages.current = images; setIsSwitchingPage(true); } else setIndex(images.length - 1); } else setIndex((index ?? 0) - 1); @@ -334,6 +339,7 @@ export const LightboxComponent: React.FC = ({ // go to preview page, or loop back if no callback is set if (pageCallback) { pageCallback(1); + oldImages.current = images; setIsSwitchingPage(true); setIndex(0); } else setIndex(0); @@ -396,6 +402,15 @@ export const LightboxComponent: React.FC = ({ else document.exitFullscreen(); }, [isFullscreen]); + function imageLoaded() { + setImagesLoaded((loaded) => loaded + 1); + + if (imagesLoaded === images.length - 1) { + // all images are loaded - update the nav offset + setNavOffset(getNavOffset() ?? undefined); + } + } + const navItems = images.map((image, i) => ( = ({ role="presentation" loading="lazy" key={image.paths.thumbnail} + onLoad={imageLoaded} /> )); @@ -763,7 +779,7 @@ export const LightboxComponent: React.FC = ({ )} {showNavigation && !isFullscreen && images.length > 1 && ( -
+