A client had requested a change where they wanted all the default crops to be automatically generated after uploading a file (rather than having to go into the vertical tabs ui and click each crop individually). In case anyone finds this useful, I've added a patch to the module which adds this functionality, plus config to toggle it (default is off).

This issue is necessary to implement queue /automated import with mandatory crop, it could also ease multi-upload process.

CommentFileSizeAuthor
#67 interdiff_49-67.txt1.81 KBneerajsingh
#67 2865396-67.patch2.6 KBneerajsingh
#63 2865396-63.patch1.91 KBweseze
#54 Screen Shot 2020-04-28 at 12.21.41 PM.png86.48 KBjoshua.boltz
#49 image_widget_crop-apply_default_crop-2865396-44.patch2.32 KBArkener
#43 apply_default_crop-2865396-43.patch2.36 KBgoodDenis
#42 apply_default_crop-2865396-42.patch2.25 KBmaestro888
#41 2_2_fix-2865396-41.patch2.7 KBrrotari
#37 2865396-36-apply-default-crop.diff2.81 KBKingdutch
#35 interdiff.txt864 bytesbramtenhove
#35 2865396-35-apply-default-crop.diff2.53 KBbramtenhove
#28 2865396-26-apply-default-crop.diff2.23 KBjochemvn
#26 2865396-25-apply-default-crop.diff1.61 KBjochemvn
#23 2865396-always-crop.patch1.88 KBKeenegan
#19 crop-automatically.txt1.69 KBq__nt_n
#2 automatically_apply-2865396-2.patch4.22 KBchrishks
autocrop.patch4.22 KBchrishks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chrishks created an issue. See original summary.

chrishks’s picture

FileSize
4.22 KB
woprrr’s picture

Hi @chrishks

Thank to your feeback and patch !!

This needed are already treated and aren't in scope of this module (this is why I don't have implement rotation, zoom, automatic crop, focal point or other effects or behavior).

For automatic crop feature that is work of another module I created to provide automatic crop with a service to implement Google vision api or/and image widget crop integration.

Conceptually automatic crop aren't in scop of this module because image widget crop are a interface to use CROP API on few contexts BUT we already and you have a needed to purpose a usefull integration with all crop ecosystem modules.

Actually I'm on my phone to respond you more speed as possible but I review your patch to take feedback (but usage of form alter are generally a bad idea we do prevent all new options in image_crop element directly).

chrishks’s picture

I should point out the patch doesn't make things fully automated (like your other module). It just saves some clicks :-)

The idea is you can still use the widget after to change the crop if you didn't like what the defaults were.

Anyway, thanks for taking a look! If its out of scope then no problem, close the issue, just thought it might be something that might be useful to others.

woprrr credited slashrsm.

woprrr’s picture

The idea is you can still use the widget after to change the crop if you didn't like what the defaults were.

Yes and this issue are a good start to support automated_crop like "imce" compatibility. If automated_crop are detected we can enable an option at the widget basis to automatically create a crop with effects preferences and come back adjust it if the automated cropped zone are wrong :) At the beginin I just think automated_crop to works with Image_widget_crop and purpose exactly same feature describe by you. The advantage is decoupled responsability of Image Widget Crop and use automated_crop manager to automatically applies crops when you upload an image with Image Widget Crop +AND "automated_crop" option on widget are enable.

I should point out the patch doesn't make things fully automated (like your other module). It just saves some clicks :-)

Yes, But with automated_crop we have only an image style to set up and combine all effects (manual_crop + automated_crop + scale or resize) you have choice :) Just with an boolean to active/desactive in widget onto widget (IWC) configuration.

Anyway, thanks for taking a look! If its out of scope then no problem, close the issue, just thought it might be something that might be useful to others.

It's not the subject :) I don't talk to refuse your feature request just have a better usage of Crop ecosystem modules that permit to have a real modularity with all features. If Image Widget Crop assume a lot of feature that risk to be a THON module hard to refactor or move. The Idea is like media / commerce having an ecosystem of modules (small) and responsable of just one job and permit to use what ever combos to permit you make a crop more easily possible.

Conclusion : We need to create a brige in Image Widget Crop to use automated crop API and having our feature finally. Normally these feature are already planned to use it by IWC.

What you think about this ?

woprrr’s picture

Version: 8.x-1.4 » 8.x-2.x-dev
Issue tags: +D8Media, +automated crop, +Media Initiative

This feature aren't in 1.x scope but more for post RC stable of 2.x branch.

q__nt_n’s picture

This patch doesn't work for me.
I'm on branch 2.x-dev (ref bcc23af35d0f7fc9f355aafed89bc77ca66a3945).
In my field's widget, I don't have the checkbox who should enable automatic cropping.

@chrishks This patch still working for you ?

martin.davidson’s picture

I would find this extremely useful.

woprrr’s picture

Status: Active » Postponed (maintainer needs more info)

Image widget crop can't assume this kind of behavior alone because Crop ecosystem are designed like micro service and Image Widget Crop are only an interface to crop images from image field widget. To having automated cropping tools please @see https://www.drupal.org/project/crop/issues/2830768#comment-12466067 that permit you to define crop effect having a fallback effect using "automated crop" service specialized for define crop area automatically. This service does provide a Google api or whatever other services to analize and provide automatic crops based on lot of criteria (like face/objects...).

In Image Widget Crop we can use automated crop factory to purpose a pre defined crop area on file upload by example ?

Kingdutch’s picture

It seems that this was previously implemented for the 1.x version of this module in #2658652: Default crop does not apply

Kingdutch’s picture

Status: Postponed (maintainer needs more info) » Active

Forgot to move this back to active after providing new information.

woprrr’s picture

Status: Active » Postponed (maintainer needs more info)

I think we are moving away from the original topic here.

Existing behavior in version 1.x is fonctional in 2.x but I think it's more a misunderstanding on how the default crop is to bring. Currently this option is not the one to automatically crop but more to define a crop area by default and requires MANDATORY a click in crop area to validate the pre-select zone.

The automatic crop on file the upload will be brought by this issue once the bridge with "automated_crop" or the feature of automated crop developed and will not be brought by a complexification of the existing js because it is not a good solution of all the ways.

It seems that this was previously implemented for the 1.x version of this module in

Yes and that's already works => upload a file and you can seen a pre-selected crop area you just need to click in crop area to validate this pre-selected area purposed by js.

conclusion this issue need to wait automated crop service out to exist and provide a more powerfull & flexible feature.

mariusdiacu’s picture

The author of the issue pointed a common use case where automatic crop is needed.
If you have, for example, a profile picture field with crop enabled (I use image_widget_crop) and you need it displayed as a circle thumbnail (though a square), the automatic crop shown on edit form is not applied until you click on the crop (to set it). In this case, the generated thumbnail is not a square, has no crop applied and, for the end user, its confusing. You see the cropped image widget but it is not active until you click on it.

O'Briat’s picture

Woppr is right, I think it's just a wording issue :

  • this issue is about applying the "cropperjs default crop" without users having to click on cropping area (that's why he called it "automatic")
  • Woppr talks about a much more advanced feature: the "magic" cropping using external api (google) to recognise image contents and crop them "automatically"

Just rename this issue into "apply default crops on file upload" and "voila".

I know that @q__nt_n has a php working patch for it.

Kingdutch’s picture

I indeed mean what @mariusdiacu also means. It's also how I read the initial issue summary.

It's very confusing for users that they see the crop in the preview but it's not applied unless a user clicks it. We've had a few bug reports about this behaviour from clients already.

It seems that this would be as easy as implementing a croparea.click() after the initial crop area is determined?

mariusdiacu’s picture

I've already tried many methods to force a click() method, but no success. The event that should be dispatched by the jQuery cropper js is not working (at least for me).

q__nt_n’s picture

FileSize
1.69 KB

Hi @O'Briat !

I did that : https://gist.github.com/QuentinFonteneau/91fd0fea12e8bab3a2394fb3a0b4ed11
This function is call when I create /edit my bundle image (on submit and publish action)
It's work quite well :)

woprrr’s picture

Hi all I can confirm that @q___nt_n method are the best way to do that because we have already speak about that in Drupal Slack. Finally We only need to make exactly same logic like Automated Crop but customized to work with IWC. IT's totally (must have) correct to make a new Class to applies your own logic and purpose an automated crop during managed_file process. IMHO the "Mediator" Design pattern match with that needed to make this class by the way.

Kingdutch’s picture

Status: Postponed (maintainer needs more info) » Active

I'm moving this back to active. I think there's enough people for whom this solves a real issue (myself included).

I think the issue would then have the following steps:

  • Create a widget setting "Always apply crop" "Applies the default crops even if the user did not select any"
  • Based on q__nt_n's function in #19 create behaviour where the default crop is used if none was selected in the field widget's submit handler.

This ensures that the default behaviour for this module doesn't change but elegantly solves the problem without requiring any other modules.

If this is truly something that won't be fixed in this module then this issue should probably (sadly) be closed as "Won't fix".

Kingdutch’s picture

Title: Automatically apply crops on file upload » Provide option to apply default crop if user doesn't select any
Issue tags: +Needs issue summary update

Rename issue based on comments and the actions described in my issue. We'll also need to update the issue summary for anyone that finds the issue at this point.

Keenegan’s picture

Status: Active » Needs review
FileSize
1.88 KB

I have the same needs for a project so I've created a patch with q__nt_n answer.
I just added the automatic crop, not the configuration to enable/disable automatic crop, I just want to know if it was the right way to do.

Kingdutch’s picture

Status: Needs review » Needs work

From what I can see we only need to set the properties here (like is done when a crop is applied by the user). The actual 'cropping' is done by submit handlers in the image_widget_crop.module file which uses the properties that were set.

We could also move this out to a function defaultCropProperties($crop_type, $image) to make it a bit more readable.

You should also move the $properties['crop_applied'] = 1; out of the if ($width_ratio > $height_ratio) { so it also applies if $height_ratio is bigger than $width_ratio. We should probably also set $edit = TRUE;

Enabling/disabling could be done by changing the else into else if (!empty($element['#apply_default_crop']) and adding that property as a setting in the ImageCropWidget.

O'Briat’s picture

Issue summary: View changes

Add multi-upload, bulk edit and queue job support.

jochemvn’s picture

Hey Keenegan

I've made some changes to your patch. As suggested by Kingdutch it's not needed to call the service (which should be injected btw), since the submit handlers take care of this.
I've taken the $properties['crop_applied'] out of the if...then...else and added $edit = TRUE.

Could you take a look and confirm it's working?

Status: Needs review » Needs work

The last submitted patch, 26: 2865396-25-apply-default-crop.diff, failed testing. View results

jochemvn’s picture

jochemvn’s picture

Keenegan’s picture

Hello jochemvn, sorry for the delay.

I tried your last patch, but it didn't work in my test.

When I uploaded an image, I had the same behaviour than before (no crop applied).

The form_state_values variable was an empty array, so the rest of the code wasn't executed.
if ($form_state_values) {...}

Maybe (probably) am I missing something ?

ribel’s picture

I have tested last patch and crop works correctly now.

woprrr’s picture

+++ b/src/Element/ImageCrop.php
@@ -205,14 +205,39 @@ class ImageCrop extends FormElement {
+            else {
+              list ($width_ratio, $height_ratio) = explode(':', $crop_type->aspect_ratio);
+
+              // Define properties according to dimensions and ratio.
+              $height = $image->getHeight();
+              $width = $image->getWidth();
+              if ($width_ratio > $height_ratio) {
+                $properties['width'] = $width;
+                $properties['height'] = $width * $height_ratio / $width_ratio;
+                $properties['x'] = 0;
+                $properties['y'] = ($height - $properties['height']) / 2;
+              }
+              else {
+                $properties['height'] = $height;
+                $properties['width'] = $height * $width_ratio / $height_ratio;
+                $properties['x'] = ($width - $properties['width']) / 2;
+                $properties['y'] = 0;
+              }

This part can be extracted from another method or calculaction Class.

I think we can reduce the complexity of that if/else.

woprrr’s picture

I have different feedback about this subject.

My first opinion about that subject it’s not really the job of the IWC widget to predict every possible use case made by the widgets. It is probably easier and more convenient to extend the widget and adapt it yourself even if it does not exactly predict what you are imagining. It is also IWC’s responsibility to correctly cut out the key parts in order to better interact with them. In the idea IWC was made to simply provide a Crop API crop area interface. Many feature requests are made more complex to meet the needs. The automatic crop in the absence of manual harvesting by the user comes under a large and complex feature and it is the job of automated crops to provide an automated crop harvesting service based on many parameters that are rules of trade.

That’s why from the beginning I try to interest people on the implementation of automated crops instead of looking for and distorting the manual harvest widget (even if he can do the job).

Regarding the patch as I said it seems to me that it adds complexity in the code and this complexity will further dig the debt accumulate on IWC and that we will soon tackle head for a more scalable version. The idea of automated crop is, in my opinion, the most adapted to your needs because it will be a third-party service (Google vision or your?) Who will do the analysis of the image and will harvest the crop area for IWC . IWC will just allow to be able to alter this crop area to perform automatically and the option will just activate the harvest delegation from the crop area to the tier service. (SRP)

Conclusion : we can use this patch or change it to provide a "fallback" when any automated crop service is enabled and options of automated crop is needed but we can’t again add more complexity directly into this widget.

What we need about that ?

O'Briat’s picture

I don't know how you want to interface IWC with "automated crop" services, but this "fallback" could be bundle with IWC as the default "automated crop" and it could provide a nice example for peoples who want to create their own "automated crop" services using custom code or third-party services.

I still think that a lot of people are asking for this features either because their users are "lazy" or because they have to use automated workflow : bulk upload or data migration...

bramtenhove’s picture

We used the patch from #26, but encountered an issue with a square crop style.

Patch is updated with a check for square crop styles so the crop doesn't go beyond the image boundaries.

woprrr’s picture

Status: Needs work » Closed (won't fix)

Hii Here !

I think definitely we are wrong here. As I explained it will never be within the scope of the IWC module to carry the responsibility of automatic crop. I really thank you for looking for a solution to the automatic crop tool! Thank you for following the topic the most advance on the issue https://www.drupal.org/project/crop/issues/2830768#comment-12466067

We need to move the responsibility from Crop API to support 'fallback' and Automated crop Via Plugins for code fallback calculation.

If we need in the correct issue I can make a Diagram to explain better.

Kingdutch’s picture

Just uploading a patch for anyone that can use it until the solution in the Crop module is done.

This is a patch for version 2.1 (2.2 makes some changes in the ImageCrop widget which will stop this patch from applying).

This fixes an issue where we used the incorrect logic for figuring out what to do with square crops for tall images.

woprrr’s picture

Hi @Kingdutch

Good job ! Thank you normally, today I will finish the first part of combo with Crop Api -> Automated Crop.

woprrr’s picture

Status: Closed (won't fix) » Postponed
Parent issue: » #2830768: Automated crop integration

I re-open this issue to having best visibility about changes from Automated crop side.

IMHO when the feature are released we can create new issue to adapt/update wording&doc.

In current state with https://www.drupal.org/project/crop/issues/2830768#comment-12678267 actually we can have the AutomatedCrop feature usable. We just need to transform AutomatedCrop Factory from a more "Drupal" Factory (PLUGIN) to provide best flexibility.

woprrr’s picture

Status: Postponed » Closed (won't fix)

All use cases are now covered by https://www.drupal.org/project/crop/issues/2830768#comment-12686405

I create a follow-up issue to edit all docs and Project page to how using the automatic crop with IWC. All help/review/feedback/opinion are WELCOME in #2830768: Automated crop integration issue or Automated crop issue queue.

rrotari’s picture

maestro888’s picture

Status: Closed (won't fix) » Needs review
FileSize
2.25 KB
goodDenis’s picture

Thanks for the patch!

Image crop is still required for me.
I added

$element['crop_wrapper'][$type]['crop_container']['values']['crop_applied']['#value'] = 1

to fix it.

MrPaulDriver’s picture

@woppr I'm with the others looking for a default crop on image upload. Thank you for the latest patch @goodDenis

I tried automated_crop and was getting unpredictable and unwanted results.

c7bamford’s picture

I'm joining this dogpile. The default crop area is shown to users. It's not intuitive to not apply it if the user decides that it is what they want. If it is not selecting the correct area, the user can adjust it, but if it is, it needs to be accepted by the system.

c7bamford’s picture

Status: Needs review » Reviewed & tested by the community

The patch at #43 works for me.

jantoine’s picture

Status: Reviewed & tested by the community » Needs work

This still needs some work! The patch in #43 applies the crop automatically when uploading new files, but if an entity was saved using an image style that didn't include IWC and is updated to use an image style that includes an IWC step, the crop is not automatically applied when editing the image.

Lukas von Blarer’s picture

Status: Needs work » Reviewed & tested by the community

@jantoine This sounds like an edge case for me. You can create a follow-up of this issue.

Setting the status back to RTBC.

Arkener’s picture

Thanks everyone for the patches. I fixed a small issue we encountered while using the patch in #43. The ratio of the end result wasn't taken into consideration when determining if the image should be cropped horizontal or vertical, which sometimes resulted in black bars on the cropped result.

goodDenis’s picture

Thanks, #44 works for me.

Traverus’s picture

Hey there. Sharing my thoughts that I believe that this isn't an implementation of 'automatic cropping' it's purely a user interface concern. The user sees a selection on the screen when they look at the widget, if they do nothing, they would assume that the selection in question is in fact the crop. I think allowing for 'default crop selection' to be possible would be the correct user interface choice. Thanks for everyone's work on this and I'll be using a patch.

Lukas von Blarer’s picture

The patch in #49 works for me. Thank you!

woprrr’s picture

Status: Needs review » Closed (works as designed)

Hi there, Feelfree to continue to use this patch but it's out of scope for IWC no additional feature are planned and you already have a robust solution with automated crop feel free to use it or this patch but this issue will not be merge sorry this is a team chose, we will think about it for 9.x banches.

BTW : Everyone seem's to not understand the problems about calculation in PHP side and JS ... ALL calculation are assumed by js component not by php.

best regards.

joshua.boltz’s picture

It seems like this issue was automatically closed, but I've tried many of the patches in this issue and am still experiencing the issue where the default crop area is not being recognized when saving the image.

In diving into the module code a bit, i can see that if i do not touch the crop handles, it results in a "crop_applied" value of 0.
If i do use the crop handles and perform my own crop of the image, i see "crop_applied" set to 1.

It seems like if the crop is not manually performed by the user and the default crop is used, that the module's logic just uses the width and height dimensions from the originally uploaded image, whereas i would think the script should re-calculate those dimensions based on the default crop area.

dotpex’s picture

Status: Closed (works as designed) » Needs review

@woppr I'm with the others looking for this feature. I tried Automated Crop but there is no option to manually change crop.
Every client like Image Widget Crop and all of them asking for automatic crop.
In my opinion Image Widget Crop is a perfect place to have an option/checkbox 'Apply crop automatically'

dotpex’s picture

Status: Needs review » Needs work
dotpex’s picture

I think everybody will agree about a simple automatic crop: "no need to click on image to apply crop"

O'Briat’s picture

Again, everyone wants this option to be implemented in the widget whatever it's a php or js calculation. It seems a obvious feature, but @woprr refuse to implement it for 3 years now. For the record, we use the @q__nt_n patch for 2 years on a major website without any problem or complain.

scotwith1t’s picture

Pretty surprised this would be such a confusing and contentious issue. I've been using this patch for a while but recently noticed that this doesn't work in the media library modal so ended up back here again. I know @phenaproxima is a maintainer as well as @slashrsm, so maybe one of them will consider looking at this issue. Just to add to the chorus, seems like a no-brainer to have the crops, which look like they are already applied (automatically expanded, etc.) be used/saved whether the user interacts with the cropper or not - even in a modal, hopefully :)

gslexie’s picture

I have run into this on multiple projects and I don't really believe the fix belongs in either IWC or Automated Crop.

Crop API provides tools for defining and executing constraints on image size and shape, but does not reliably enforce those constraints. If, for example, I set up a crop type and image style and spec it for 16:9 images, I really should be able to expect to render 16:9 images, but instead I get random shapes that can break the frontend design if nobody has specified a crop. If there would be any fix, I would think that it should extend the "Manual Crop" image effect to make it predictable on its own.

That said, tools to provide default crops exist in Core!

When defining your image styles, use two effects. First, Manual Crop effect. Second, apply the core Scale and Crop effect so as to match the aspect defined in your Crop Type.

If a crop is not defined, Scale and Crop will provide the fallback behavior. If a crop is defined, then it already matches the aspect ratio the second effect becomes meaningless.

This also seems to match the the presentational default that misleads people so often in IWC.

For crops with variable aspect ratio but size limitations, I combinations of other image effects can probably be used to achieve the equivalent behavior.

My hunch was that this is what the Crop API developers intended. It's a pretty robust approach, because you can decide for yourself how to apply defaults on a per-image-style basis. Portraits can default to top anchored, while backgrounds can default crop to center. Or whatever you want.

But this all is not obvious, has caught many of us off guard, and is cumbersome to define for numerous image styles. Maybe the option of a simplified fallback would still applicable within the "Manual Crop" image effect just for DX. Even moreso because Automated Crop is still not production ready and does so much more than just enforce crop definitions.

I also began looking into writing a Javascript patch for IWC to apply the crop that is presented without requiring an extra click, but I gave up because it seemed either confusing or complicated when there are multiple crop types available. It would either imply incorrectly to users that _all_ crop types should be initialized, or, risk performance issues by actually mass-initializing a cropper for each type on load.

pfrenssen’s picture

When defining your image styles, use two effects. First, Manual Crop effect. Second, apply the core Scale and Crop effect so as to match the aspect defined in your Crop Type.

If a crop is not defined, Scale and Crop will provide the fallback behavior. If a crop is defined, then it already matches the aspect ratio the second effect becomes meaningless.

This approach has a serious disadvantage: it will no longer make it possible to do a pure ratio based crop which is one of the biggest selling points of the Crop API module. Scale and Crop only works on pixels, and what makes it even worse is that it will upscale images uploaded with a lower resolution.

pfrenssen’s picture

I see that cropperJS has a method to retrieve the data from the JS side: getCropBoxData().

weseze’s picture

Patch from #44 isn't working in media library modal. The reason is that the entire ImageCrop Element class isn't compatible with media library modal because of broken form_state checks. Everything that is not related to form_state does seem to be working, so that's why most stuff actually does work.

I have attached a new patch, based on #44 that works on the setting "Show default crop area". If that is checked and there are no default loaded properties from an existing crop, a default crop is set. I have not addressed the broken form_state code for media_library modals since this check bypasses that entirely to make it work.

Important to note that this is that this patch is not a "correct" solution, just a quickfix to get us going.

The proper fix would be to use "automated_crop", but currently that module has a dev dependency on "crop", which make it unusable for me.

@pfrenssen: I was also very interested in trying to make it work through JS, but couldn't figure that one out. I think it would be a better (temporary) solution then the patch I made.

jonathanshaw’s picture

The autocrop module looked complicated, bit I installed it and configured it very simply and it solved my use case perfectly. User pictures were cropped to square as soon as they were uploaded, even before the user was saved.

Danny Englander’s picture

I tested the patch in #63 and it works in the media library modal in the sense that it does apply a default crop and indeed the vertical tabs show that cropping has been applied. However, on the front-end, no cropping is being applied. I realize based on the comments in #63 that this patch will need some work for it to fully work but I just thought I would provide some feedback anyway. Thank you.

Danny Englander’s picture

Status: Needs work » Needs review

With regard to my comment in #65, I learned that you need to set the media library form display at /admin/structure/media/manage/image/form-display/media_library to use cropping as well. Thereafter, the default crops showed up on the front end after simply uploading a new image without doing anything else.

neerajsingh’s picture

Re-rolling, patch from #49.

Scenario - If there a crop type's "Aspect Ratio" is left empty for arbitrary aspect ratio. The below line of code does not work:
list($width_ratio, $height_ratio) = explode(':', $crop_type->aspect_ratio);
resulting error in the below line:
if ($width / $width_ratio > $height / $height_ratio) {

Re-rolling the patch to address this issue.

yepa’s picture

Patch #67 works fine on 8.x-2.3.
Thanks folks.