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

Server support for full HiDPI #92

Merged
merged 2 commits into from Feb 28, 2019
Merged

Server support for full HiDPI #92

merged 2 commits into from Feb 28, 2019

Conversation

kamil4
Copy link
Contributor

@kamil4 kamil4 commented Feb 27, 2019

This merge request addresses #198.

It's a pretty large patch so allow me to provide an overview/motivation for why I coded it the way I did. I can of course make any modifications you deem necessary to make it acceptable for merging.

Lychee currently supports HiDPI ("retina") for the square thumbnails only, by creating @2x files on the server side during image upload and assuming on the frontend side that those files are always present (the @2x names are unconditionally used in srcset when generating HTML).

To keep things configurable, I added three new options to the configs table: thumb_2x, small_2x, and medium_2x. Only the first one defaults to 1, to mimic the current behavior. The others need to be set to 1 manually to activate the generation of @2x versions of small and medium images.

Depending on the input image size, some of the image variants, especially the medium@2x, may end up not getting generated. I decided to extend the photos table with additional columns medium2x, small2x, and for the sake of completeness also thumb2x to store that info. The server API needed to be extended to pass the data on 2x variants to the frontend.

In image view mode, as well as in justified album view, the images may not be rendered at their natural resolution but may be resized. In those cases, passing density value of 2x in the srcset tag would be incorrect. Instead, the best solution is to pass the widths of each image variant (using the <width>w syntax in srcset) and the expected rendered size in the sizes tag. But we need to know those widths, and they are not readily available in the frontend. The server knows them when it generates the images, so I changed the small/small2x/medium/medium2x columns in the photos table to store <width>x<height> strings for each variant, or an empty string if that variant is not available. I kept thumb2x as a 0/1 since that has a fixed size of 400x400. Again, the server API needed to be extended to pass this additional data (in small_dim/medium_dim/small2x_dim/medium2x_dim fields).

Given the size of the patch, I have no intention of backporting it to Lychee v3; however, the patched frontend will also work with an unpatched server.

With that lengthy preface out of the way, I can now discuss parts of the patch itself:

database/migrations/* verifies for each image that the thumb2x variant actually exists and also extracts the sizes of small/medium variants using GD.

Image/ handlers were extended to return the width/height of generated images via two arguments passed by reference. If this syntax is too ugly to tolerate, please recommend an alternative (I've never coded in PHP before).

In ModelFunctions/PhotoFunctions.php, I made the generation of the thumb2x variant in createThumb() conditional on the input image size to match what was already done for small/medium. I added the generation of 2x variants to add() and createMedium(), conditional on the config variables.

There are two areas that I wasn't sure of:

I believe I did the right thing for Console/Commands/medium.php and small.php but I didn't test because I don't know how to use these things -- any hint? Also, should I add support for generating 2x variants there?

I wasn't sure what to do about getRandom() in app/Http/Controllers/PhotoController.php because I couldn't figure out how to get that Frame mode to work. Though I feel that that API is needlessly restrictive and it shouldn't be the server's job to pick big vs medium variant. Why not simply return a whole Photo object and let the frontend pick whatever it needs out of it -- there's already code for that?

@ildyria
Copy link
Member

ildyria commented Feb 27, 2019

I have no intention of backporting it to Lychee v3; however, the patched frontend will also work with an unpatched server.

That's what I like to read and this has already been my philosophy for quite a while, there is a reason why there is no multi user support and sub albums in Lychee v3. 😃 👍

@ildyria
Copy link
Member

ildyria commented Feb 27, 2019

because I couldn't figure out how to get that Frame mode to work.

Go to advanced settings, set Mod_frame to 1 and then go to https://example.com/frame (yes it is that simple. 😃 )

@ildyria ildyria added the enhancement New feature or request label Feb 27, 2019
@ildyria ildyria added this to the v4.0.0 milestone Feb 27, 2019
@ildyria
Copy link
Member

ildyria commented Feb 27, 2019

By the way, @kamil4 you might want to join on gitter: https://gitter.im/LycheeOrg/Lobby (you can log in directly with your github id)

@hermzz
Copy link
Contributor

hermzz commented Feb 28, 2019

Looks good, but I just tested it by setting thumb2x, small2x, and medium2x to 1 and the medium image failed to get created. Could you double check this is working?

> select * from configs where `key` like '%2x';
+----+-----------+-------+--------+
| id | key       | value | cat    |
+----+-----------+-------+--------+
| 37 | thumb_2x  | 1     | Config |
| 38 | small_2x  | 1     | Config |
| 39 | medium_2x | 1     | Config |
+----+-----------+-------+--------+
> select medium, medium2x, small, small2x, thumb2x from photos where id = 15513143310778;
+-----------+----------+---------+----------+---------+
| medium    | medium2x | small   | small2x  | thumb2x |
+-----------+----------+---------+----------+---------+
| 1624x1080 |          | 541x360 | 1083x720 |       1 |
+-----------+----------+---------+----------+---------+

@kamil4
Copy link
Contributor Author

kamil4 commented Feb 28, 2019

@hermzz, what was the size of the image you uploaded? With default settings, medium2x will be generated only if either the width of the original image is above 3840 or the height is above 2160...

@ildyria
Copy link
Member

ildyria commented Feb 28, 2019

Seems to be working here.

@ildyria ildyria merged commit 0a43348 into LycheeOrg:master Feb 28, 2019
@kamil4 kamil4 deleted the hidpi branch March 2, 2019 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants