Thumbnails size in file list doesn't match requested preview size

Steps to reproduce

Open folder containing images. It shows thumbnails in 64x64 size. But requests images of size 500x500 to display this thumbnails.

Expected behaviour

Images of size 64x64 are requested to display 64x64 thumbnails.

Actual behaviour

Images of size 500x500 are requested to display 64x64 thumbnails.

Server configuration

Operating system:
Ubuntu 18.10

Web server:
nginx + php-fpm

Database:
mysql

PHP version:
7.2

Nextcloud version: (see Nextcloud admin page)
15.0.2

Updated from an older Nextcloud/ownCloud or fresh install:
Updated

Hello! This is on purpose for a compatibility with the grid view :)
We had a discussion about it (performance wise) and decided it was still very decent.

(performance wise) and decided it was still very decent.

Well, it is not for me. I can't scroll folder containing several thousand of images with 500x500 previews, it will stop showing previews at all after showing around 50-100 first ones (looks like because server timings to get image gets up to 10 seconds). And I do have pregenerated 512x512 previews. If I change to 64x64 previews scroling works fine. This is on 8-core VM on top of 2xE5-2620v1 server.

Also, 500x500 previews are 50-100kb in size, just loading folder with 100 images requires ~10MB of data transfered.

Actually we fetch 250px images and they're about 20KB

I see https://xxx/index.php/core/preview?fileId=332354&c=1b49252159ee603963dd872140f8ab11&x=500&y=500&forceIcon=0 urls requested in developer tools.

server/apps/files/templates/list.php

Line 34 in 6a3f4e4

    
           <table id="filestable" class="list-container <?php p($_['showgridview'] ? 'view-grid' : '') ?>" data-allow-public-upload="<?php p($_['publicUploadEnabled'])?>" data-preview-x="250" data-preview-y="250">

data-preview-x="250" data-preview-y="250" :)

server/apps/files/js/filelist.js

Line 2106 in 0c8a000

urlSpec.x *= window.devicePixelRatio;

and I do have HiDPI monitor both on laptop and mobile.

@jancborchardt

Nice catch! I guess it's then a bit too much! Thanks :)
@jancborchardt shall we revert?

This is the first report we get about this, so not sure it warrants reverting?

Also, at this stage the grid view is not default yet, but it might be with Nextcloud 17 if we polish it more. And then we need the big thumbnails anyway. So I would say we should leave it as is.

I can confirm this at current 17.0.1 master my head is 51d3f98bfb83b632be6223a5d928d57e8e52e942

The thumbnail according to chrome inspector is 150x150 pixels.

the background url is http://172.18.0.1/core/preview?fileId=446&c=80c49cc440c54e05493e75f064aaa36e&x=325&y=325&forceIcon=0&asdfasd=adsfasdf23

however the response gives me a 1024x1024 image, considering i may have thousands - really saturating my network IO

1024-1024-crop.jpg: JPEG image data, JFIF standard 1.01, resolution (DPI), density 96x96, segment length 16, comment: "CREATOR: gd-jpeg v1.0 (using IJG JPEG v62), quality = 90", baseline, precision 8, 1024x1024, frames 3

Digging deeper I , when I access that URL see that Generator\getPreview() $width and $height are called with a value of 325 but when it reaches

		// Calculate the preview size
		list($width, $height) = $this->calculateSize($width, $height, $crop, $mode, $maxWidth, $maxHeight);

$width and $height become 1024 inside of calculateSize()

because in ce10f8b

This resulted still in a lot of possible images. Generating a lot of
server load and taking up a lot of space.
This moves it to previews to be powers of 4: 64, 256, 1024 and 4096
Also the first two powers are always skipped (4, 16) as it doesn't make
sense to generate previews for that.
We cache preview pretty agressively and I feel this is a better
tradeoff.

essentially - it gets rounded up to 1024 because it is not 325 and not near 256

In other words - whatever is setting the thubnail size preference is TOTALLY incompatible with the backend's ability to use scaled image sizing, the backend is limiting to a select size of thumbnails! either 256 or 1024 !!

I set in config.php ..

  'preview_max_x' => 325,
  'preview_max_y' => 325

but

		list($maxWidth, $maxHeight) = $this->getPreviewSize($maxPreview, $previewVersion);

still returns 4092, 3072

so the thumbnail sizing is a big steamy pile :/

.... This moves it to previews to be powers of 4: 64, 256, 1024 and 4096

at ce10f8b it is changed from rounding to a size of powers of 2 to powers of 4...

reverting that commit atleast gives me a 245 pixel image, instead of a 1024 image, although it's still loaded in a 130x130 pixel box......

in Drupal and Wordpress you just define an image size for the thumbnails, and then it's created once when required. no one cares if they get a slightly lower or slightly higher resolution thumbnail.

Whatever is doing the fancy JS for detecting DPI etc and then requesting a certain size thumbnail , in my opinion is a total waste of time, it should just dish up a general thumbnail FAST, this would entirely remove the all this wierd code about setting power of 2 or power of 4 pixel sized limit images, cut down on CPU and Disk IO just use a default size.

then on image request just get the one size.

there, fast, and you have less data to worry about

https://github.com/nextcloud/server/blame/0c8a0007a98d4b8df4b53298451d822292605be6/apps/files/js/filelist.js#L2106

this concept of caring about the DPI for thumbnails is 6 years old! I really think it needs to be thrown out, no one cares about getting exactly an amazing thumbnail at the tradeoff of having dozens of open issues here on github about slow thumbnail generation because it's making 6 useless thumbnails per image

remove the DPI stuff for thumbnails, I have never seen this in other systems, it adds too much complexity and dependencies on different thumbnail sizes
have a default thumbnail size which is clearly documented and configurable, 1 size will fit 85% of people, and the others can just tweak a well defined setting.
stop all the pain with people running cron scripts to pregenerate thumbnails and gallery images which are never used!

total madness! simpler is better!

@skjnldsv

Thanks for digging 👍

I remember this high dpi stuff (even for thumbnails) from many projects. Customers always complain if something looks blurry on their new MacBook Pro Retina ;)

I guess @skjnldsv and @jancborchardt are waiting for more input. If you think the high dpi logic is superfluous nowdays feel free to open a pull request with your suggested changes. I'm happy to help (e.g. with breaking tests).

Sometimes a change will improve the situation for a special case but not in general (e.g. #17028). Just to let you know 🙈

this concept of caring about the DPI for thumbnails is 6 years old!

It's probably even older because 6 years ago they move the code to this place 🤣

@dgtlmoon

@dgtlmoon putting together a PR can be time consuming, so I would like to warn you. Be ready for your PR to be ignored as the gallery is clearly not a priority here.

It is awesome to contribute and I don't want to discourage you but be prepared to be ignored, you might not even get a response back.

You probably already know https://ownyourbits.com/2019/10/16/a-new-architecture-for-nextcloud/, #14131 and #14953. We're getting a bit off topic here. Feel free to add your findings to the other issues as additional context.

@kesselb

@kesselb thanks for that, the only thing I can say is - ColdFusion also had an event loop model and look how that went!

I dont feel that new architecture article addresses what I feel is the elephant in the room here, that there's slabs of custom code and the bootstrap time is 200ms... really strange, this is not normal. just saying "yeah! event loops!" is not really the way forwards.

Introducing more custom built code to solve a problem that has been solved by dozens of other existing frameworks that can be introduced? even Drupal 7 dropped it's custom code that it was using for over a decade and went with Symfony in version 8.

back to topic - I'm having a hack here https://github.com/dgtlmoon/server/tree/refactor-thumbnail-files-integration the main thing is that I'm working towards each interface (grid of thumbs, main view of image etc) issuing a type of image it wants ie &imageGroup=thumbnail and then just roll with that, instead of this bizzare overcomplicated way of inferring the DPI and weird ^n configuration (which is what is sending the wrong size image, as per this topic)

but after spending time inside the code, looking at the slabs of custom framework going on and dealing with the slowness caused by an insane bootstrap sequence, i no longer feel the motivation :( :(

@kesselb

@kesselb @skjnldsv I don't want to hijack this thread. I apologize if it sounded more negative than I intended to, but I am just stating a fact that a hypothetical PR will most likely not be taken into account because the gallery is not a priority for NC and that is ok. Still I think it's fair to warn before hand to a potential contributor.

Particular users get NC for free and we should be grateful, and clearly our interests do not align necessarily with the ones from NC as a company, but sometimes we fall under the illusion that because it's open source we can build this together, where this is not the case in reality. That being said, if the gallery actually worked fine it would be huge for particular users and it would take NC from "well, it's the only FOSS alternative even though it doesn't work well" to "I don't miss my proprietary services, this is awesome".

Let's leave it there, as I said I don't want to hijack the thread, and I admire people that try to fix stuff in open source to help themselves and others, just warning them.

PS. Not talking about myself, but about other people that have tried to help with the Gallery, some of them send me private emails in deep frustration like I can do anything about it, just because I wrote an article or two. Ofc my own PRs when related to the gallery didn't stir much movement as I would have wanted to but that doesn't bother me too much since I (and NCP users) can still benefit from my work so it didn't go to waste.

@nachoparker

@nachoparker Nextcloud is first and foremost a community project. Everyone is invited, welcome and in fact necessary to make this work. :)

What is a priority is decided by how many people are willing to work on improving it. We have hundreds of people in the Nextcloud organization here on Github who can review and 👍 pull requests, not only people who work for Nextcloud the company )which would create a huge bottleneck).

(Also, this issue is not about the Gallery at all, but about the file list and/or grid view.)

With that, let’s go back to the topic of the pull request. :)

❤️

Except that it's rounding up images to 1024pixels if the image size is greater than 256pixels... Because of the old ^n logic as above. This needs to be removed and some preset sizes put in. If you look at my branch as linked above you can see some progress.

…

On Monday, November 18, 2019, Gatak ***@***.***> wrote: One solution to the problem of file sizes is to provide a UI section in the server admin page. Something like this 1. select what thumbnail and preview dimensions should be used. Here the UI can have a predifined list or allow manual input. Explain what default sizes are used on different pages and apps, including the mobile apps. 2. select if displayed previews be loaded from the nearest below or above dimension. Give examples so the user understands the relation to point 1) 3. jpeg quality setting 0-100% I believe this would help for the time being. The issue about backend performance is another thing to be solved long term. Oh one thing. @nachoparker <https://github.com/nachoparker> optimizations for multicore rendering should be merged in nc base/server IMHO. https://ownyourbits.com/2019/06/29/understanding-and- improving-nextcloud-previews/ Thank you. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#13709?email_source=notifications&email_token=AACDEONAPMCUAESI35LR3B3QUKA5LA5CNFSM4GRJ6J72YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEKHNGQ#issuecomment-554989210>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACDEOPRUWDLDOE5OD5EKQ3QUKA5LANCNFSM4GRJ6J7Q> .

@Gatak

@Gatak

Yes, I wrote that article (I think you know that).

I played around the "using more cores" idea, just to be able to benchmark some numbers and stir some movement towards improving preview generation times, but it didn't receive much traction. I wanted to attract visibility to the issue to see if it would help.

So I played with both the generator app and the server code

nachoparker#1
#16158

nachoparker/previewgenerator#1
nextcloud/previewgenerator#166

So I don't understand should I use Preview Generator with default settings or what settings should I use to benefit from pregenerated thumbnails? As i understand from this discussion it is now not possible to pregenerate thumbnails?

@montvid

@montvid yes, pregenerating previews is recommended.

Those are the recommended settings, afaik:

This is a screenshot from:
https://ownyourbits.com/2019/06/29/understanding-and-improving-nextcloud-previews/

There was a nice PR for the viewer recently nextcloud/viewer#899 but I suppose it doesn't solve this issue?

Navigation Menu

Repository navigation

Thumbnails size in file list doesn't match requested preview size #13709

Steps to reproduce

Expected behaviour

Actual behaviour

Server configuration

8 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Thumbnails size in file list doesn't match requested preview size #13709

Description

Steps to reproduce

Expected behaviour

Actual behaviour

Server configuration

Activity

skjnldsv commented on Apr 30, 2019

bazuchan commented on Apr 30, 2019

skjnldsv commented on Apr 30, 2019

bazuchan commented on Apr 30, 2019

skjnldsv commented on Apr 30, 2019

bazuchan commented on Apr 30, 2019

skjnldsv commented on Apr 30, 2019

jancborchardt commented on Apr 30, 2019

dgtlmoon commented on Nov 16, 2019

kesselb commented on Nov 16, 2019

nachoparker commented on Nov 16, 2019

8 remaining items

kesselb commented on Nov 17, 2019

dgtlmoon commented on Nov 17, 2019

dgtlmoon commented on Nov 17, 2019

nachoparker commented on Nov 18, 2019

jancborchardt commented on Nov 18, 2019

dgtlmoon commented on Nov 18, 2019

nachoparker commented on Nov 19, 2019

montvid commented on Dec 27, 2020

szaimen commented on Dec 28, 2020

szaimen commented on Jun 18, 2021

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions