Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Photos 2.0 - Slow speeds, navigation, ordering issues #1195

Closed
valdearg opened this issue Aug 31, 2022 · 32 comments
Closed

Photos 2.0 - Slow speeds, navigation, ordering issues #1195

valdearg opened this issue Aug 31, 2022 · 32 comments
Labels
0. Needs triage Pending approval or rejection. This issue is pending approval. bug Something isn't working

Comments

@valdearg
Copy link
Contributor

Describe the bug
Upgrading to the latest beta version 15.0.3 introduced the new version of Photos.

Currently I'm running into three different issues when viewing the overall apps/photos/ view which shows all images on the system.

  1. Performance on my system is very slow to load. I suspect this may be because the initial dav query requests 1000 items. I'm using a separate PostgreSQL instance which contains a LOT of images and can be quite slow. If possible, the previous version requested a smaller number of images (I think it was around 15) which then performed another paginated search for a further number as you scrolled down the page. It would be amazing to get this part back, or perhaps customisable?

  2. Ordering of images has changed, but I'm not entirely sure the exact ordering. Previously the view would display items in the most recently modified/recently added.

The DAV query seems to be working correctly. For example, the first items are:

<d:response>
        <d:href>/<redacted>/01_TD_1.jpg</d:href>
        <d:propstat>
            <d:prop>
                <oc:fileid>14270471</oc:fileid>
                <d:getlastmodified>Sat, 27 Aug 2022 13:40:59 GMT</d:getlastmodified>
                <d:getetag>&quot;c71bd8b70533133097c425313a719546&quot;</d:getetag>
                <d:getcontenttype>image/jpeg</d:getcontenttype>
                <d:getcontentlength>2110878</d:getcontentlength>
                <nc:has-preview>true</nc:has-preview>
                <nc:file-metadata-size>{&quot;width&quot;:1365,&quot;height&quot;:2048}</nc:file-metadata-size>
                <oc:favorite>0</oc:favorite>
                <d:resourcetype/>
            </d:prop>
            <d:status>HTTP/1.1 200 OK</d:status>
        </d:propstat>
        <d:propstat>
            <d:prop>
                <nc:realpath/>
                <nc:face-detections/>
            </d:prop>
            <d:status>HTTP/1.1 404 Not Found</d:status>
        </d:propstat>
    </d:response>
    <d:response>
        <d:href>/<redacted>//<redacted>/.jpg</d:href>
        <d:propstat>
            <d:prop>
                <oc:fileid>14270419</oc:fileid>
                <d:getlastmodified>Sat, 27 Aug 2022 13:40:46 GMT</d:getlastmodified>
                <d:getetag>&quot;76a3ca3a325df858b4cfacd6944b266d&quot;</d:getetag>
                <d:getcontenttype>image/jpeg</d:getcontenttype>
                <d:getcontentlength>1762165</d:getcontentlength>
                <nc:has-preview>true</nc:has-preview>
                <nc:file-metadata-size>{&quot;width&quot;:1306,&quot;height&quot;:1935}</nc:file-metadata-size>
                <oc:favorite>0</oc:favorite>
                <d:resourcetype/>
            </d:prop>
            <d:status>HTTP/1.1 200 OK</d:status>
        </d:propstat>
        <d:propstat>
            <d:prop>
                <nc:realpath/>
                <nc:face-detections/>
            </d:prop>
            <d:status>HTTP/1.1 404 Not Found</d:status>
        </d:propstat>
    </d:response>

However, in the photos view, we're getting these files first. Suggesting perhaps it's ordering the wrong way around maybe?

<d:response>
        <d:href>/<redacted>/Red_x5000-450x634.jpg</d:href>
        <d:propstat>
            <d:prop>
                <oc:fileid>14245079</oc:fileid>
                <d:getlastmodified>Fri, 26 Aug 2022 10:20:43 GMT</d:getlastmodified>
                <d:getetag>&quot;51ace037a58e2be631948ce5fb76e319&quot;</d:getetag>
                <d:getcontenttype>image/jpeg</d:getcontenttype>
                <d:getcontentlength>149554</d:getcontentlength>
                <nc:has-preview>true</nc:has-preview>
                <nc:file-metadata-size>{&quot;width&quot;:450,&quot;height&quot;:634}</nc:file-metadata-size>
                <oc:favorite>0</oc:favorite>
                <d:resourcetype/>
            </d:prop>
            <d:status>HTTP/1.1 200 OK</d:status>
        </d:propstat>
        <d:propstat>
            <d:prop>
                <nc:realpath/>
                <nc:face-detections/>
            </d:prop>
            <d:status>HTTP/1.1 404 Not Found</d:status>
        </d:propstat>
    </d:response>
    <d:response>
        <d:href>/<redacted>/Red_x5000-501x705.jpg</d:href>
        <d:propstat>
            <d:prop>
                <oc:fileid>14245080</oc:fileid>
                <d:getlastmodified>Fri, 26 Aug 2022 10:20:42 GMT</d:getlastmodified>
                <d:getetag>&quot;e3c1239146083d387ac22dd32cec6919&quot;</d:getetag>
                <d:getcontenttype>image/jpeg</d:getcontenttype>
                <d:getcontentlength>177274</d:getcontentlength>
                <nc:has-preview>true</nc:has-preview>
                <nc:file-metadata-size>{&quot;width&quot;:501,&quot;height&quot;:705}</nc:file-metadata-size>
                <oc:favorite>0</oc:favorite>
                <d:resourcetype/>
            </d:prop>
            <d:status>HTTP/1.1 200 OK</d:status>
        </d:propstat>
        <d:propstat>
            <d:prop>
                <nc:realpath/>
                <nc:face-detections/>
            </d:prop>
            <d:status>HTTP/1.1 404 Not Found</d:status>
        </d:propstat>
    </d:response>
  1. I suspect this final issue will be difficult to replicate and could be due to the speed issues previously and it would be great to get some further users testing this. If you scroll down the Photos page, say after 50/100 images and view an image, if you click off the image (so outside the image on the right or left), sometimes this reverts to the top of the page. It's intermittent for me but does seem to happen after I've done this a few times.

It's a bit difficult to send screenshots I'm afraid as it's a lot of family pictures currently at the top. For the moment I've reverted to the previous beta release due to the performance.

@valdearg valdearg added 0. Needs triage Pending approval or rejection. This issue is pending approval. bug Something isn't working labels Aug 31, 2022
@meichthys
Copy link

What method did you use to install the 2.0 version?

@valdearg
Copy link
Contributor Author

This was included in the latest beta of version 15 according to the release notes: nextcloud/server#33674

I noticed a fairly significant change to the UI, though I think the version number may have appeared as the same in the apps view. May just be because it's beta.

@meichthys
Copy link

So you also running the nextcloud server beta?

@valdearg
Copy link
Contributor Author

Yes, beta 2 currently.

@marcelklehr
Copy link
Member

cc @artonge

@artonge
Copy link
Collaborator

artonge commented Sep 6, 2022

N°1 addressed in #1209

@valdearg
Copy link
Contributor Author

3 can be ignored, I think this was an issue with the theme I was using.

Updated to the latest beta 6 and the performance does seem to be much better.

So just the ordering of images now!

@valdearg
Copy link
Contributor Author

Any updates on the ordering issues @artonge? Just aware that it was touted as a big feature upgrade and it's a bit unusable when you have a good number of images.

@artonge
Copy link
Collaborator

artonge commented Oct 11, 2022

Any updates on the ordering issues @artonge? Just aware that it was touted as a big feature upgrade and it's a bit unusable when you have a good number of images.

In the forum, I pointed @AndyXheli to a possible starting point to investigate the issue.

Here it is again:

I can't find sortFilesByTimestamp anymore. Either it does not exist, and the .sort is not using it. Or it exists but the logic is wrong.

.forEach(month => filesByMonth[month].sort(this.sortFilesByTimestamp))

@valdearg Any chances you could open a PR for that ?

@valdearg
Copy link
Contributor Author

Hrm, does this help at all? I'm afraid I'm not an expert:

sortFilesByTimestamp(fileId1, fileId2) {

It looks like that used to be included in the FilesPicker.vue, but no longer.

@valdearg
Copy link
Contributor Author

#1330 maybe, good luck.

@AndyXheli
Copy link

sorry @artonge bit a little busy. didnt mean to ignore you.

@AndyXheli
Copy link

@valdearg did you try to manually apply this patch ? did it help ?

@valdearg
Copy link
Contributor Author

@valdearg did you try to manually apply this patch ? did it help ?

Have to confess, I've never setup a NextCloud development process, I get an error about versions of Photos so probably need to do something else: "The files of the app Photos (photos) were not replaced correctly. Make sure it is a version compatible with the server."

Main website is on beta2 so doubt it's a drop in replacement!

If anyone is able to test it, that'd be great!

@AndyXheli
Copy link

I can test. Can you give me a scenario to try before and after the test please

@artonge
Copy link
Collaborator

artonge commented Oct 11, 2022

"The files of the app Photos (photos) were not replaced correctly. Make sure it is a version compatible with the server."

Both 'photos' and 'server' needs to be on the same branch, either master or stable25, else you will have this king of error :)

@valdearg
Copy link
Contributor Author

I can test. Can you give me a scenario to try before and after the test please

Thanks! Simple test is that the images should be in the order they were added to the system, so if you add a new image to a folder NOW, it should show at the top. All images should then be in the order they were added.

@AndyXheli
Copy link

@artonge Hmm so question i though i can just modify the file but i cant so do i have to bulid a test app and then install it on the server ? if so, how do i go about doing that ? unless @valdearg you already have the app complied and ready for install ?

@artonge
Copy link
Collaborator

artonge commented Oct 11, 2022

If you want to test on a live server, you could compile on your dev machine and move the build files to the server.
You could also install the build tools on the server and compile on it directly, but that might be difficult or unwanted.

@AndyXheli
Copy link

I only have a test server my windows machine only run github desktop or on my linux "ubuntu' then i have my test VM running Nc 25 RC3

@valdearg
Copy link
Contributor Author

Haha, I have a broken app so far I'm afraid! It just redirects to the Files currently.

@AndyXheli
Copy link

Haha oh boy. I'm working on creating a dev setup

@szaimen
Copy link
Contributor

szaimen commented Oct 11, 2022

@valdearg @AndyXheli if you have docker installed locally, you can check the PR out and test it easily with:

docker run -it \
--name nextcloud-easy-test \
-p 127.0.0.1:8443:443 \
-e PHOTOS_BRANCH=valdearg:patch-2 \
--volume="nextcloud_easy_test_npm_cache_volume:/var/www/.npm" \
ghcr.io/szaimen/nextcloud-easy-test:latest

@AndyXheli
Copy link

@szaimen Thank you! I'll give that a try :)

@AndyXheli
Copy link

I get an error as soon as i click on photos app see admin @valdearg how about you ?

@szaimen
Copy link
Contributor

szaimen commented Oct 11, 2022

Ah, that is probably my fault because the viewer app is missing.

Please try this one:

docker run -it \
--name nextcloud-easy-test \
-p 127.0.0.1:8443:443 \
-e PHOTOS_BRANCH=valdearg:patch-2 \
-e VIEWER_BRANCH=master \
--volume="nextcloud_easy_test_npm_cache_volume:/var/www/.npm" \
ghcr.io/szaimen/nextcloud-easy-test:latest

@AndyXheli
Copy link

No worries. Trying it again, Thank you!

@AndyXheli
Copy link

Looks good to me :)

@valdearg
Copy link
Contributor Author

OK, from my testing, thanks to the fantastic help from @szaimen there, I can also confirm it seems to be working too.

Just testing some other oddities as well.

@valdearg
Copy link
Contributor Author

OK, think that's all set! That patch seems to resolve it, thanks for everyone's help with this!

Not sure the procedure for fully approving the fix and including it in where it needs to be fully, but it should be good.

@szaimen
Copy link
Contributor

szaimen commented Oct 12, 2022

All done now? :)

@artonge
Copy link
Collaborator

artonge commented Oct 12, 2022

Let's close this one, feel free to open another more specific issue if needed :)

@artonge artonge closed this as completed Oct 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. Needs triage Pending approval or rejection. This issue is pending approval. bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants