LycheeOrg / Lychee

A great looking and easy-to-use photo-management-system you can run on your server, to manage and share photos.
https://lycheeorg.github.io/
MIT License
3.41k stars 301 forks source link

[Enhancement] Be able to rotate image #471

Closed gardiol closed 4 years ago

gardiol commented 4 years ago

I know i am being a bit of invasive lately.... Sorry the covid stay at home is pushing me to work harder on my photos....

Anyway, is there a way to rotate images after they have been uploaded? I find sometimes some photos are rotated, specially from my analog film scanned, and it would be a nice feature to be able to rotate them in Lychee....

ildyria commented 4 years ago

While I see your point and see how this can be useful in some cases, adding this functionality would also be already stepping in the direction of being a picture editor. What I mean is that people will ask to add processing like converting .psd into .png, converting .cr2 and .cr3 into .jpg... Adding the ability to have multiple versions of a picture ( #441 ) etc.. And that is something we would like to avoid.

However for that specific one (rotations), I think we would be open for PR. :)

ildyria commented 4 years ago

I know i am being a bit of invasive lately....

Don't worry about that. :) This project has been sleepy over the last few months. :see_no_evil:

gardiol commented 4 years ago

Would it be viable to find the image id and manually rotate it from filesystem?

gardiol commented 4 years ago

i would like to help on this topic.. but i need some guidance i WAS a goot php developer up til 5.xy... since then i moved up the chain and i mostly manage nowadays.

ildyria commented 4 years ago

There is not much changes between php 5 and php 7. :) You can drop by the gitter if you have any questions: https://gitter.im/LycheeOrg/Lobby

A starting point to understand the current infrastructure of the project would be : https://github.com/LycheeOrg/Lychee-Laravel/wiki/Readings-about-Laravel

Would it be viable to find the image id and manually rotate it from filesystem?

That would be my go to option, like doing it via an exec command. You can also see to reuse the code in: https://github.com/LycheeOrg/Lychee-Laravel/tree/master/app/Image More specifically: https://github.com/LycheeOrg/Lychee-Laravel/blob/master/app/Image/ImagickHandler.php#L105

From a broad point of view, you would need to:

gardiol commented 4 years ago

I cannot promise anything i will try to give a try, will let you know!

gardiol commented 4 years ago

I am looking into this and i a doing some progress. Yes PHP has not changed that much. Anyway, can you help me understand where i can get the photo file path given the PhotoID?

ildyria commented 4 years ago

This snippet should help you a lot. :)

It is from https://github.com/LycheeOrg/Lychee-Laravel/blob/master/app/Http/Controllers/PhotoController.php#L536

$photo = Photo::find($photoID);

// Photo not found?
if ($photo == null) {
  Logs::error(__METHOD__, __LINE__, 'Could not find specified photo');

  return 'false';
}

$prefix_path = $photo->type == 'raw' ? 'raw/' : 'big/';
// determine the file based on given size
switch ($kind) {
  case 'FULL':
    $url = Storage::path($prefix_path . $photo->url);
    break;
  case 'LIVEPHOTOVIDEO':
    $url = Storage::path($prefix_path . $photo->livePhotoUrl);
    break;
  case 'MEDIUM2X':
    if (strpos($photo->type, 'video') !== 0) {
      $fileName = $photo->url;
    } else {
      $fileName = $photo->thumbUrl;
    }
    $fileName2x = explode('.', $fileName);
    $fileName2x = $fileName2x[0] . '@2x.' . $fileName2x[1];
    $url = Storage::path('medium/' . $fileName2x);
    break;
  case 'MEDIUM':
    if (strpos($photo->type, 'video') !== 0) {
      $url = Storage::path('medium/' . $photo->url);
    } else {
      $url = Storage::path('medium/' . $photo->thumbUrl);
    }
    break;
  case 'SMALL2X':
    if (strpos($photo->type, 'video') !== 0) {
      $fileName = $photo->url;
    } else {
      $fileName = $photo->thumbUrl;
    }
    $fileName2x = explode('.', $fileName);
    $fileName2x = $fileName2x[0] . '@2x.' . $fileName2x[1];
    $url = Storage::path('small/' . $fileName2x);
    break;
  case 'SMALL':
    if (strpos($photo->type, 'video') !== 0) {
      $url = Storage::path('small/' . $photo->url);
    } else {
      $url = Storage::path('small/' . $photo->thumbUrl);
    }
    break;
  case 'THUMB2X':
    $fileName2x = explode('.', $photo->thumbUrl);
    $fileName2x = $fileName2x[0] . '@2x.' . $fileName2x[1];
    $url = Storage::path('thumb/' . $fileName2x);
    break;
  case 'THUMB':
    $url = Storage::path('thumb/' . $photo->thumbUrl);
    break;
  default:
    Logs::error(__METHOD__, __LINE__,
      'Invalid kind ' . $request['kind']);

    return null;
}

// Check the file actually exists
// TODO: USE STORAGE FACADE HERE
if (!file_exists($url)) {
  Logs::error(__METHOD__, __LINE__, 'File is missing: ' . $url . ' (' . $photoID . ')');

  return null;
}
gardiol commented 4 years ago

Ok, thank you, i think i have the server code ready for testing, can you give me some hints on where to go onthe frontend side? I want to add two icons on the picture page which trigger the new /api/PhotoEditor::rotate

API i am adding...

I added: Route::post('/api/PhotoEditor::rotate', 'PhotoEditorController@rotate')->middleware('upload')

to routes/web.php

ildyria commented 4 years ago

For the front end it here: https://github.com/LycheeOrg/Lychee-front :)

gardiol commented 4 years ago

I am studying it. It seems i need to work on the photo.js and on the view.js. I would like to add two buttons on the #buttonBar item, still not clear to me how to do that... Sorry for these basic questions... but maybe after all i might help also on other things... I really love Lychee. The php side is brilliantly clear and concise.

ildyria commented 4 years ago

You may want to have a look at: https://github.com/LycheeOrg/Lychee-front/blob/master/scripts/main/header.js

Also this one will be of interest (for adding the buttons): https://github.com/LycheeOrg/Lychee-Laravel/blob/master/resources/views/gallery.blade.php

Please note that some of the icons may already be available see here: https://raw.githubusercontent.com/LycheeOrg/Lychee-front/master/images/iconic.svg

And this will help you too: https://github.com/LycheeOrg/Lychee-Laravel/wiki/Build

gardiol commented 4 years ago

Hi! I did quite some progress... Now there are two things i am missing out:

  1. how to add new icons? I need two for rotations and doesnt seems to me there are already
  2. Hw do i connect the front end to the back end? I have done most of it i think but i cannot undestand how to expose the backend endpoint (es /api/PhotoEditor) to the bit in javascript:

header.dom('#button_rotleft') .on(eventName, function() { photoEditor.rotate([ photo.getID(), 1 ]) });

and one last thing, to i have to rotate all the copies of the image as listed in your previous post or just the "full size/raw" one and the thumbnals are regenerated automatically?

thanks

ildyria commented 4 years ago

Hi! I did quite some progress... Now there are two things i am missing out:

  1. how to add new icons? I need two for rotations and doesnt seems to me there are already

Indeed I did not find rotations icons too. You can find some svg icons here: https://icons.getbootstrap.com/ Add them here: https://github.com/LycheeOrg/Lychee-front/tree/master/images

You will need to add a line here: https://github.com/LycheeOrg/Lychee-front/blob/master/gulpfile.js#L323 In order to make sure they are exported when compiling the front-end.

  1. Hw do i connect the front end to the back end? I have done most of it i think but i cannot understand how to expose the backend endpoint (es /api/PhotoEditor) to the bit in javascript:

header.dom('#button_rotleft') .on(eventName, function() { photoEditor.rotate([ photo.getID(), 1 ]) });

If you look into the public folder, you will find the Lychee-front repository in it. From there (public/Lychee-front), you can run npm install (once or more if you added external npm dependencies) and then npm run compile. This will generate public/dist/main.js and other files.

This main.js basically contain all the the .js files from scripts/main/' merged together.

Also from a UX/UI perspective, I would not place this functionalities in the header, but add a small overlay with the two rotation icons at the bottom center of the page (a bit like the Exif overlay which is on the left). Additionally I would add a setting whether to show this overlay or not in order to make sure to not click on it by mistake but togglable with a key binding for example. R (as rotate) would have been nice for similarities with Lightroom but it is used for rename, so I would suggest T as Transform. :)

Have a look at: https://github.com/LycheeOrg/Lychee-front/blob/master/scripts/main/build.js#L269 https://github.com/LycheeOrg/Lychee-front/blob/master/scripts/main/build.js#L325

and one last thing, to i have to rotate all the copies of the image as listed in your previous post or just the "full size/raw" one and the thumbnails are regenerated automatically?

you can take either approach:

gardiol commented 4 years ago

Good, more steps forward. Now i have two issues.

I think i set up the front end correctly now, still on the header, but first i want to make it work then improve the UI part.

When i hit the "rotate" button i get the following error in the browser console:

description: "Server error or API not found."
params: {…}
direction: 1
function: "PhotoEditor::rotate"
photoID: "15859207754229"
__proto__: Object { … }
response: "Internal Server Error"

But i have this in my "routes/web.php:

Route::post('/api/PhotoEditor::rotate', 'PhotoEditorController@rotate')->middleware('upload');

And i have the file i created: app/Http/Controllers/PhotoEditorController.php which contains the code for the new controller.php:


/** @noinspection PhpUndefinedClassInspection */

namespace App\Http\Controllers;

use App\Album;
use App\Photo;

class PhotoEditController extends Controller
{
        /**
         */
        public function __construct(
        ) {
        }

        /**
         * Given a photoID and a direction (+1: 90° clockwise, -1: 90° counterclockwise) rotate an image
         *
         * @param Request $request
         *
         * @return boolean
         */
        public function rotate(Request $request)
        {
                $request->validate([
                        'photoID' => 'string|required',
                        'direction' => 'integer|required',
                ]);

                $photo = Photo::with('album')->find($request['photoID']);

                // Photo not found?
                if ($photo == null) {
                        Logs::error(__METHOD__, __LINE__, 'Could not find specified photo');
                if ($photo == null) {
                        Logs::error(__METHOD__, __LINE__, 'Could not find specified photo');
                        return false;
                }

                try {   
                        $img_path =  "big/".$photo->url

                        $image = new \Imagick();
                        $image->readImage($path);

                        $orientation = $image->getImageOrientation();

                        $rotate = true;
                        if ( $direction == 1 ) {
                                $image->rotateImage(new \ImagickPixel(), 90);
                        } else if ( $direction == -1 {
                                $image->rotateImage(new \ImagickPixel(), -90);
                        }
                        $image->write();
                        $image->clear();
                        $image->destroy();

                        return true;
                } catch (ImagickException $exception) {
                        Logs::error(__METHOD__, __LINE__, $exception->getMessage());

                        return false;
                }
        }
}

Is there something obvious i am missing?

The second thing is... i cannot see the two new SVG i copied into frontend images (and added to the gulp file!) They are not copied under the public/img folder after i run npm run compile.

ildyria commented 4 years ago
                // Photo not found?
                if ($photo == null) {
                        Logs::error(__METHOD__, __LINE__, 'Could not find specified photo');
                if ($photo == null) {

Is this duplication intended?

Additionally you don't need the ::with('album') if you are not going to use the album object. :)

                        $orientation = $image->getImageOrientation();

wrong that is not the path. Use the Storage facade for that:

$img_path =  Storage::path("big/".$photo->url);

Additionally, you are using $path instead of $image_path when you call:

$image->readImage($path);

you are likely to get an error.

Also you are missing a ; after $img_path = "big/".$photo->url

Hope this helps. :)

gardiol commented 4 years ago

Oh shit, yes, a ton of basic errors. Is there a way to proof check the files with this Laravel approach? I will fix later and try again. Thank you once more

ildyria commented 4 years ago

If you use an editor like phpstorm or visual code, those includes modules to parse your language and check for simple errors (such as missing ; and unused variables etc.)

If you are editing on a remote server (or a VM like me), I found visual code more efficient than phpstorm, however on local files I found the syntax completion of phpstorm better.

Additionally we have a formatting script for the php files: make formatting. It will make sure your indentations etc are compliant with the code on the repo.

ildyria commented 4 years ago

Also, I would make sure that the solution is Imagick agnostic, meaning it can also be used if you don't have it. We have something similar here: https://github.com/LycheeOrg/Lychee-Laravel/tree/master/app/Image

gardiol commented 4 years ago

Ah ah.. I work from a remote server using VI. I am that old fashioned. Anyway, i will use the make formatting and some more manual checking. Strange tough, usually i get the PHP syntax errors in the server logs with this i dont. Let me have it all working first then i will improve by fixing the imagick stuff and the overlay icons too.

ildyria commented 4 years ago

With VS Code, I'm literally connecting to the server via SSH and letting it handle the rest. :)

gardiol commented 4 years ago

Yep you are right... I will work with Kate or Geany via SSH.... Sorry, i don't like Microsoft stuff if i can avoid it.

ildyria commented 4 years ago

:'D That one is actually free. Also... Microsoft owns Github... :roll_eyes:

gardiol commented 4 years ago

Yeah and also most of google stuff is free... and they track you and such.

Yes i know ms bought github. I host my repos on my own server, or i would have moved to gitlab I guess...

I don'ht question vs from technical point.

There are alternatives. I like to support open source and smaller realities. But i am diverging from the topic so i stop here.

d7415 commented 4 years ago

Ah ah.. I work from a remote server using VI. I am that old fashioned. Anyway, i will use the make formatting and some more manual checking. Strange tough, usually i get the PHP syntax errors in the server logs with this i dont. Let me have it all working first then i will improve by fixing the imagick stuff and the overlay icons too.

Plenty of vim plugins too :wink:

gardiol commented 4 years ago

[2020-04-16 11:50:05] local.ERROR: Target class [App\Http\Controllers\PhotoEditorController] does not exist.

The file is in the correct pth alongsize PhotoController...

Do i need to register the class in some way beside the routes/web.php? now the code is correct and make formtting sais it's all good.

gardiol.org@gardiol ~/subdomains/photos/htdocs $ grep -ri PhotoEditorController *
routes/web.php:Route::post('/api/PhotoEditor::rotate', 'PhotoEditorController@rotate')->middleware('upload');
vendor/composer/autoload_classmap.php:    'App\\Http\\Controllers\\PhotoEditController' => $baseDir . '/app/Http/Controllers/PhotoEditorController.php',
vendor/composer/autoload_static.php:        'App\\Http\\Controllers\\PhotoEditController' => __DIR__ . '/../..' . '/app/Http/Controllers/PhotoEditorController.php',

I grepped for PhotoController and copied what was done fot it... Something is missing?

ildyria commented 4 years ago
class PhotoEditController extends Controller

:stuck_out_tongue:

gardiol commented 4 years ago

Oh no... This is what you get when doing coding while kids are doing homework nearby.

gardiol commented 4 years ago

Fixed that.

[2020-04-16 11:57:08] local.ERROR: Class App\Http\Controllers\Request does not exist {"exception":"[object] (ReflectionException(code: 0): Class App\\Http\\Controllers\\Request does not exist at /home/web/gardiol.org/subdomains/photos/Lychee-GIT/vendor/laravel/framework/src/Illuminate/Routing/RouteSignatureParameters.php:25)

This baffles me

Now my code is even simpler:

<?php

/** @noinspection PhpUndefinedClassInspection */

namespace App\Http\Controllers;

use App\Logs;
use App\Photo;
use Storage;

class PhotoEditorController extends Controller
{       
        public function __construct()
        {
        }

        /**
         * Given a photoID and a direction (+1: 90° clockwise, -1: 90° counterclockwise) rotate an image.
         *
         * @param Request $request
         *
         * @return bool
         */
        public function rotate(Request $request)
        {       
                $request->validate([ 
                        'photoID' => 'string|required',
                        'direction' => 'integer|required',
                ]);

                $photo = Photo::find($request['photoID']);

                // Photo not found? 
                if ($photo == null) {
                        Logs::error(__METHOD__, __LINE__, 'Could not find specified photo');

                        return false;
                }
                $img_path = Storage::path('big/' . $photo->url);

                $image = new \Imagick();
                $image->readImage($img_path);

                $rotate = true;
                if ($direction == 1) {
                        $image->rotateImage(new \ImagickPixel(), 90);
                } elseif ($direction == -1) {
                } elseif ($direction == -1) {
                        $image->rotateImage(new \ImagickPixel(), -90);
                }
                $image->write();
                $image->clear();
                $image->destroy();
                return true;
        }
}
ildyria commented 4 years ago

add

use Illuminate\Http\Request;
gardiol commented 4 years ago

Yes!!! It worked!!!

Ahahahah i am so happy!!!

Now i can fix the details. Thank you all guys... will ask more i am sure!

I will open a pull request when i am satisfied with the results.

gardiol commented 4 years ago

Long time, but i managed to get some free time only recently to work on this. I have two more questions:

ildyria commented 4 years ago

Would you accept a not too polished solution (only imagemagick support, and icons in the header of image view)? i am quite sure this is the maximum extent i can get to reach at the moment.

Can you add an advanced setting to enable-disable this rotation functionality ? :) We will gladly merge it then, also this will allow you to disable it when imagick is not available.

  • How do i trigger a gallery refresh after the rotation is complete? Right now i am refreshing the entire page to see the rotated image, but this is of course not desirable.

I don't know if you are doing it in the photo view or album view or both. In the photo view, you should be able to access the image src directly, in the case of the album view, you should have the id of the image and be able to retrieve the image dom path and update the src. This will be relevent for you to trigger a reload of the picture: https://stackoverflow.com/a/1077051/3981163

gardiol commented 4 years ago

Of course yes, can you point me on how to start adding an advanced setting?

ildyria commented 4 years ago

php artisan make:migration <migration-descriptive-name> This will create a new file here: https://github.com/LycheeOrg/Lychee/tree/master/database/migrations which will be executed on : php artisan migrate You need to be able to rollback: php artisan migrate:rollback

As follow is an example of migration which added the advanced setting prefer_available_xmp_metadata https://github.com/LycheeOrg/Lychee/blob/master/database/migrations/2020_05_19_174233_config_prefer_available_xmp_metadata.php

You can later access this setting by using:

Configs::get_value('prefer_available_xmp_metadata', '0');

Confidentiality of level 2 is enough for you (if I'm not mistaken).

gardiol commented 4 years ago

I am almost done.

Two things (as it seems, it's always two things...)

I flagged the "editor_enabled" config boolean so that the PhotoEditorController does not perform when it's false, but i want to remove the rotate icons from the header too. What is the correct way to do it? Do i access the config from the front-end and disable the icons? Do i check the config in the gallery.blade.php, but this means that upon config change the frontend must be reloaded? It's not clear to me.

Rotation works reliably now, but rotating the thumbnails causes an issue with the display size in the album page. Basically, the rotated sizes are not rotated.. a vertical thumb stays vertical. (note: i rotate all the existing small/medium/big/thumb images).

EDIT - To be more clear... If the image is a vertical rectangle, when i rotate it becomes an horizontal rectangle. Well, the thumbnail in the album page is still a vertical rectangle with the horizontal image cropped inside.

ildyria commented 4 years ago

I flagged the "editor_enabled" config boolean so that the PhotoEditorController does not perform when it's false, but i want to remove the rotate icons from the header too. What is the correct way to do it? Do i access the config from the front-end and disable the icons? Do i check the config in the gallery.blade.php, but this means that upon config change the frontend must be reloaded? It's not clear to me.

It should be sent in the front end via on the GET php/index.php See if it is not already provided, you will get access to the setting here: https://github.com/LycheeOrg/Lychee-front/blob/master/scripts/main/lychee.js#L166 Have a look here for how we remove buttons: https://github.com/LycheeOrg/Lychee-front/blob/c3be513d1be3b1dbb934313defd265d2a8f3a58f/scripts/main/lychee.js#L522

Rotation works reliably now, but rotating the thumbnails causes an issue with the display size in the album page. Basically, the rotated sizes are not rotated.. a vertical thumb stays vertical. (note: i rotate all the existing small/medium/big/thumb images).

EDIT - To be more clear... If the image is a vertical rectangle, when i rotate it becomes an horizontal rectangle. Well, the thumbnail in the album page is still a vertical rectangle with the horizontal image cropped inside.

Do you change the small; small2x; medium; and medium2x data of a photo on rotation ? They provide the "shape" of the place holder.

gardiol commented 4 years ago

No i did not change the small; small2x; medium; and medium2x data of the photo ... Can you point to some example?

ildyria commented 4 years ago

Those data are passed to the front end: 2020-06-04_1920x1080_15:28:22 https://github.com/LycheeOrg/Lychee/blob/master/app/Photo.php#L293 https://github.com/LycheeOrg/Lychee/blob/master/app/ModelFunctions/PhotoFunctions.php#L756

gardiol commented 4 years ago

How do you get notified when the user has changed a config from the advanced config editor in the front end?

I can read the setting and remove the buttons. But after i change the setting from the front end i need to reload the page to see the changed setting...

ildyria commented 4 years ago

How do you get notified when the user has changed a config from the advanced config editor in the front end?

I can read the setting and remove the buttons. But after i change the setting from the front end i need to reload the page to see the changed setting...

Changed settings => reloads. it is that simple. :stuck_out_tongue:

gardiol commented 4 years ago

If you mean manually reload page, ok. If you mean automatic reload, it's not happening in my firefox. In either case, you answered my question.

gardiol commented 4 years ago

I managed to apply the new sizes to the database, but it seems i need to apply a gallery update after that?

ildyria commented 4 years ago

I managed to apply the new sizes to the database, but it seems i need to apply a gallery update after that?

Most likely as the front end is not aware of the change, just reloading the album should be enough though.

gardiol commented 4 years ago

How do i trigger that from the front-end code?

ildyria commented 4 years ago

window.location.reload() is a possibility, lyche.load(...), lychee.goto(...) is also possible... or create a method to query the album / photo again and reload the proper elements...

gardiol commented 4 years ago

I know how to reload a page. Ihoped there was a way without.

ildyria commented 4 years ago

You know the ID of the picture you are modifying so you can just modify the data in the album json, ~modify the dom of the said picture~ and then call view.album.content.justify();

https://github.com/LycheeOrg/Lychee-front/blob/master/scripts/main/view.js#L350

gardiol commented 4 years ago

I tried to reflow the album view with code like this:

     let sel_div = 'div[data-id='+photoID+']';
     let div_w = $( sel_div ).width();
     let div_h = $( sel_div ).height();
     $( sel_div ).width( div_h );
     $( sel_div ).height( div_w );
     view.album.content.justify();

If i DO NOT call the justify(), i get the album image swapped correctly, but the page has not "reflow" to adapt.

If i DO call the justify(), the rotated thumbnail will just be cropped in the same old size.

I checkd that in the database the new sizes are correctly stored.

ildyria commented 4 years ago

If i DO NOT call the justify(), i get the album image swapped correctly, but the page has not "reflow" to adapt. If i DO call the justify(), the rotated thumbnail will just be cropped in the same old size. I checkd that in the database the new sizes are correctly stored.

You need to update the album.json.photos array, that is where the data for each photo is stored! Look at the justify code: https://github.com/LycheeOrg/Lychee-front/blob/master/scripts/main/view.js#L350 you will understand why it still crops. Changing the dom is a possibility but it is better to change the json array.