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

Uploading bigger SVG files blocks the UI #1489

Closed
benzkji opened this issue Aug 16, 2024 · 23 comments · Fixed by #1490
Closed

Uploading bigger SVG files blocks the UI #1489

benzkji opened this issue Aug 16, 2024 · 23 comments · Fixed by #1490

Comments

@benzkji
Copy link
Contributor

benzkji commented Aug 16, 2024

Users think the system is broken. I've had this issue several times, with architecture projects.

I wonder if the old "create 4 thumbnails for every image right during/after upload" system is still in place? This could be replaced by just creating ONE thumbnail, during rendering of the change list? Thus taking of quite a bit of load, during upload, and making everything a bit more reactive?

@fsbraun
Copy link
Member

fsbraun commented Aug 19, 2024

@jrief Do I remember correctly that currently no thumbnails are generated for SVG files?

@benzkji
Copy link
Contributor Author

benzkji commented Aug 19, 2024

there is https://github.com/django-cms/django-filer/blob/master/filer/settings.py#L41

I can confirm that I have these 4 sizes on my system. Further, I get a 40x40px version in the changelist, that is generated as well, somewhen.

Without SVG support (ie reportlab), SVGs are File objects and don't have a thumbnail. But that's not an option in my current project.

@benzkji
Copy link
Contributor Author

benzkji commented Aug 19, 2024

...so, one could just set FILER_ADMIN_ICON_SIZES, to contain only one size. I'll check this, it could reduce processing time a lot. It might still take twice the time as needed (40x40, plus FILER_ADMIN_ICON_SIZES size - one would be enough). If it's already sketchy, twice/half the time is worth a lot ;)

@fsbraun
Copy link
Member

fsbraun commented Aug 19, 2024

I'll try to look into this, too. Later though...

@fsbraun
Copy link
Member

fsbraun commented Aug 19, 2024

Also, I wonder if "do not create thumbnails upon upload but only upon request" may be the better strategy if and only if there is a separate request for each thumbnail needed:

  • Potentially fewer thumbnails created
  • Main requests (upload, changelist, ...) do not get blocked

@fsbraun
Copy link
Member

fsbraun commented Aug 19, 2024

OK, what I found so far:

  • filer creates “thumbnails” for SVG files which are identical to the original and only differ in the size attributes (width and height attribute)
  • I'll have to check, but I doubt it is useful to create those files. The size can probably be overwritten by an <img> tag.
  • @benzkji likely needs thumbnails as rendered PNG/WEBP (bitmap with transparency) (if the bitmap image size is smaller than the original SVG) to avoid large data transfers and browser rendering times
  • I have not confirmed, but I think the thumbnails created automatically are not those actually needed by the current admin.

@benzkji
Copy link
Contributor Author

benzkji commented Aug 19, 2024

filer creates “thumbnails” for SVG files which are identical to the original and only differ in the size attributes (width and height attribute)

that is probably true. the issue is the generation of those, as it takes a long time to read xml/svg of large files, with thousands of path elements

I'll have to check, but I doubt it is useful to create those files. The size can probably be overwritten by an tag.

I guess not, yes. I could live with a version that creates ONE thumbnail. As all thumbnails in filer are quadratic, it would make sense (that is what easy-thumbnail does: modify "viewBox" of the SVG).

@benzkji likely needs thumbnails as rendered PNG/WEBP (bitmap with transparency) (if the bitmap image size is smaller than the original SVG) to avoid large data transfers and browser rendering times

It's hard to decide, I'm not really sure. Without a crop, we could always use the original SVG. As we did before. But since SVGs are images in filer, it has become different ;) Sure, a few 100'000 path SVGs in the changelist does not make it rendered faster...

I have not confirmed, but I think the thumbnails created automatically are not those actually needed by the current admin.

That's what I was thinking as well...

@fsbraun
Copy link
Member

fsbraun commented Aug 19, 2024

I can imagine not using thumbnails for SVG in the admin, but doing some CSS magic (object-fit etc.) instead. This way, no thumbnails would need to be created for SVG files unless you use those thumbnails elsewhere.

If you then also had an option to not create thumbnails upon upload - especially if they are not used - (the last point of my previous comment, sort of), you might be fine.

Finally, one could set a max image size for svg thumbnails and just show an icon for very large svg files. I assume a few 100k paths don't render so clearly on 40x40 or 150x150?

@benzkji
Copy link
Contributor Author

benzkji commented Aug 19, 2024

I would be fine with any of these solutions, but would prefer your first, most eleganct one, IMHO. Max image size could be difficult, there are SVGs with binary data (raster images) withing with only a few paths, that are 2mb, etc etc.

By the way, you could also drop the quadratic thumbnails, and use object-fit: contain, to still have a quadrat. I've had many people complain about those - you are not able to distinguish similar image when the change is cropped away ;). But that's another issue.

@fsbraun
Copy link
Member

fsbraun commented Aug 20, 2024

@benzkji Can you confirm my observations on thumbnail generation upon upload within the bundled filer admin?

  • A 180x180 thumbnail is generated for each upload. This is used for preview in the image widget.
  • No thumbnails are generated for ('16', '32', '48', '64') currently. Would it be possible you have leftovers from earlier filer versions?
  • A 40x40 thumbnail is generated upon listing the newly uploaded file in the folder table view, or a 160x160 thumbnail is created in the folder thumbnail view. This only happens when the images are requested, not upon upload.

If this were true, then the 180x180 thumbnail can be fully disabled. The widget might request its preview, so the thumbnail is only created on demand. Maybe it could also be 160x160?

It might need to be decided that the ('16', '32', '48', '64') setting can be deprecated.

I can start working on an admin view that does not generate thumbnails for SVG at all.

@benzkji
Copy link
Contributor Author

benzkji commented Aug 20, 2024

Will do so, thanks! @fsbraun

@fsbraun
Copy link
Member

fsbraun commented Aug 20, 2024

Also, you can try

pip install git+https://github.com/fsbraun/django-filer@fix/no-admin-svg-thumbnails

This should streamline thumbnails in the admin, and avoid them altogether for SVG files. (Existing thumbnails do not disappear!)

@benzkji
Copy link
Contributor Author

benzkji commented Aug 20, 2024

Hi @fsbraun . Thanks. I notice that the change list becomes very very unresponsive, if many big SVGs are listed, this seems unusable, even when not using thumbnails at all (100'000 paths problem). My browser tab required 3gb of RAM!

  • render PNG/WEBP for normal thumbnails of SVGs (admin, web frontend)
  • use the original SVG if an SVG is really needed (heavy zoom in, whatever) - without thumbnailer

this is the conclusion, for me. I am not sure if PNGs can be generated from SVGs, though?!

@benzkji
Copy link
Contributor Author

benzkji commented Aug 20, 2024

I can confirm that no 8x8/16x16/etc thumbnails are generated, when using the default filer uploading mechanisms.

@benzkji
Copy link
Contributor Author

benzkji commented Aug 20, 2024

THUMBNAIL_EXTENSION = "webp"
THUMBNAIL_PRESERVE_EXTENSIONS = ("gif",)

SVGs are still thumbnailed as SVGs. So I guess this would need to be adressed in easy-thumbnails?

@benzkji
Copy link
Contributor Author

benzkji commented Aug 20, 2024

I really like the ability to have SVGs in FilerImageFields. But if I knew what it means in a real world, I'm not so sure anymore ;)

@benzkji
Copy link
Contributor Author

benzkji commented Aug 20, 2024

I'll probably just deny SVGs bigger than 200kb or so. That's what it was meant for...fair enough.

The PNG/WEBP thumbnail solution would be nice, but yea, honestly this is a nice to have (and an easy-thumbnails thing).

@fsbraun thanks for all the efforts and investigations. If we get a change list without thumbnailed SVGs, nice, but for me the problem ist "SVG" in general ;). At least we have another setting to deprecate :)

@fsbraun
Copy link
Member

fsbraun commented Aug 20, 2024

Just to make sure I understand: The changelist remains "very, very unresponsive". It was not more responsive with thumbnailed SVG?

I conclude that we should look into thumbnailing SVG to webp or something (no idea, if that is possible).

@benzkji
Copy link
Contributor Author

benzkji commented Aug 20, 2024

Yes, it remains unresponsive in both cases (FF and Chrome on Linux). Thumbnailing only changes the viewBox attribute in the svg, the whole XML needs to be parsed, always.

@fsbraun
Copy link
Member

fsbraun commented Aug 20, 2024

OK, for now, we save the storage space and the need for the backend to parse the SVG. Maybe have a fallback icon for large SVG?

@benzkji
Copy link
Contributor Author

benzkji commented Aug 20, 2024

Fallback: Yes, until we can thumbnail to PNG/WEBP?

@fsbraun
Copy link
Member

fsbraun commented Aug 20, 2024

OK, I implemented the fallback: static/filer/icons/file-picture.svg is shown if the SVG is larger than 1MB (configurable)

@benzkji
Copy link
Contributor Author

benzkji commented Aug 20, 2024

Hero, thanks alot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants