jonom / silverstripe-focuspoint Goto Github PK

I am trying to debug and find a solution for this issue. Currently i have a 2880x1800 image with a focus point ( values Focus X = 0.8333333333333333, Focus Y = -0.6000000000000001) for some reason it is not generating an image cropped around the focus point. In reality it is resizing the image and setting the cropped image to the following, Top, Left, Width, Height (20-0-320-180) values.

Have you ever met this issue?

Silverstripe 3.1.6 added $force_resample to Image - which allows you to - as the name suggests, force an image to be resampled, even if it otherwise wouldn't be. I was going to add in a pull request, but it would have forced the minimum requirement up, so thought I'd just post an issue instead.

You can see the variable being added to the framework here: silverstripe/silverstripe-framework@1d86fe4

The focus point gets saved, but the UI resets to the center of the picture on load each time. It doesn't load back the focus point. That probably means that you if you want to change other image properties like the title, you have to set again the focus point or it will be saved in the center.

Tested on OSX Safari.

When working with images in the CMS outside of the Files section (e.g. HTMLEditorField insert), the editing form is disabled for some reason. I am guessing the SS4 team ran out of time to make this work in SS4.0, but expect it may be added in in a later release, as inline editing was available in SS3 in (e.g. on UploadFields).

Editing the focus point currently requires a trip to the Files section, which is a poor UX, but consistent with editing other file properties. Hopefully the native file editing experience will be improved in SS allowing all file properties (including focus point) to be edited outside of the Files admin.

Hi Jonom,

Thanks for your excellent modules! I've just published an a module that adds cropping to your FocusPoint module: https://github.com/micschk/silverstripe-focuspointcropper/

in CMS's asset manager, when i click on an image, i got ' Injector.get(): Component FocusPointField does not exist' on the right column.

I've had focuspoint on our site for a while. It's stopped working with a composer update when it jumped to 2.4

Error is

[User Warning] DataObject::__construct passed The value 'assets/article-images/_resampled/CroppedFocusedImageWyI1NzQiLCIzMDUiLCJ5Iiw2Ml0/stairs.jpg'. It's supposed to be passed an array, taken straight from the database. Perhaps you should use DataList::create()->First(); instead?

I've set focuspoint in composer.json to 2.3 and 2.2.1 - same error.

Maybe silverstripe updated and now isn't compatible. Where should I start to look / Is this a bug?

See #73 (comment)

Since 3.1.3, if the migration is executed during the same dev/build as when the Image_Live and Image_Versions tables are created, the migration fails with

ERROR [Emergency]: Uncaught SilverStripe\ORM\Connect\DatabaseException: Couldn't run query:

UPDATE "Image_Live"
 SET "FocusPointX" = "FocusX", "FocusPointY" = "FocusY" * -1

Unknown column 'FocusX' in 'field list'

It's possible the migration is triggered before the extra versioning related tables are created.

Hi, I can't get the backend focal point selector to function, the changes all appear on the site once the module has been installed, but all that appears on the image info is the area to select the focal point and a blank text box. Nothing happens once a focal point has been selected.

Any tips? - It would be great to put your work to use on this specific client project.

Thanks,
George.

Instead of after merge in to master.

When trying to use this fantastic focuspoint plugin with Silverstripe 4.3 and Elemental 4, got this error, even if trying to edit images from the Files UI directly:

[2019-01-31 15:48:04] error-log.ERROR: Uncaught Exception Error: "Call to a member function getX() on null" at /var/www/html/vendor/jonom/focuspoint/src/Forms/FocusPointField.php line 91 {"exception":"[object] (Error(code: 0): Call to a member function getX() on null at /var/www/html/vendor/jonom/focuspoint/src/Forms/FocusPointField.php:91)"} []

At first it seemed to work, then something bad happened. Tried to uninstall and reinstall it, dev/build?flush=all, composer vendor-expose and so on.

We recently upgraded one of our sites to use PHP8 and Silverstripe 4.10.0 - it appears that something has changed and silverstripe-focuspoint is no longer compatible. No errors occurred in the build process but when navigating to an individual file in the CMS the following error displays in the console: Error: Injector.get(): Component FocusPointField does not exist.

I'm flagging this early into my investigation and will continue to look into this - will update here if I discover anything.

Hi @jonom,

first of all thanks for the great module.
I noticed a weird error wich I get in the cms when opening a single view of a file Call to a member function getX() on null. When you create the FocusPointField you pass in 3 parameters, $name, $title and $image. On the title you pass in $image->fieldLabel('FocusPoint') which translates to "Focus point".

Later you $this->setName

Somehow the name doesn't get set properly (check how FieldGroup handles it) and later on getSchemaStateDefaults when you do $this->getName() it returns Focuspoint but it actually should return FocusPoint

That's why it gets a null reference and fails when trying to getX() on it. Let me know if you have any more questions.

Best regards

Including the Injector yml for method replacement seems to prevent the Image table from being added during dev/build. Commenting out the yml allows the table to be built, then it can be re-added. Not a nice solution though.

The javascript hasn't been included in all locations where focuspoint appears by default. The "file selector" dialog on an UploadField is missing this functionality, the FocusPointField appears, but you cannot alter the x,y position.

Lead's to a tedious double handling scenario

#ss4

I've been trying to use both focuspoint and versionedfiles, and I've noticed that versionedfiles shows errors when attempting to re-generate the formatted images made by focuspoint.

Additionally, when trying to run the /dev/tasks/RegenerateCachedImagesTask, I run into the same problems, so the issue does not lie in versionedfiles.

The errors:

[Warning] array_merge(): Argument '#2 is not an array
GET /dev/tasks/RegenerateCachedImagesTask
Line 925 in ~/framework/model/Image.php

[Warning] call_user_func_array() expects parameter 2 to be array, null given
GET /dev/tasks/RegenerateCachedImagesTask
Line 926 in ~/framework/model/Image.php

I've looked into the problem in depth, and it seems that Image::$extra_methods is not properly being populated with the focuspoint methods.

Any plans for a version for Silverstripe 3.2?
CroppedImage has been replaced by Fill....

Putting all of this outside of the $image->manipulateImage(function() { ... closure means that even if the resampled copy exists already, focuspoint will still check the dimensions of the image. The getWidth()/getHeight() calls can result in expensive operations - intervention loads the entire image into memory to check the dimensions.

The saving grace here is that the dimensions are cached by SilverStripe once they’ve first been discovered, but if that cache is lost (e.g. after a dev/build or a deploy) then focuspoint will trigger re-loading the image into memory to check dimensions even when the resampled copy already exists. Even when the cached dimensions do exist, this adds unnecessary cache reads to each page load.

I had a quick look at refactoring this to move the getWidth()/getHeight() calls inside the manipulateImage() closure. However the width/height are passed to calculateCrop(), and the resulting crop data is then used in the variant name. The only way I can spot to fix this would be to adjust the variant name to not use crop data - I think using the requested dimensions and focuspoint x/y co-ordinates should be enough to guarantee uniqueness though...

CroppedFocusedImage with focuspoint 2.4.* and SilverStripe 3.6.*@dev on PHP 7.1.3 throws a [Notice] like: A non well formed numeric value encountered. This happens on line [email protected]

$widthRatio = $this->owner->width / $width;

A possible fix is to sanitize parameters like:

$width = intval($width);
$height = intval($height);

Probable other methods need this too. Would you accept a PR with the above fix? See also silverstripe/silverstripe-framework@ae0fe75

First off, thanks for the module, i like it alot.
But something strange is going on, i just moved a website to the live server, and de focuspoint plugin does not seem to work. I'm getting no errors whats so ever but each time i pick a focus point and save, the focus point gets resetted. The weird thing is that it works on our local server. And other projects on the save live server do not have the same problem.
Im using silverstripe 3.5.0.

Not sure where else to put this but the picker 'seems' active when viewing an image in view-mode (form disabled). Clients try to alter the focus point but this doesn't change as it's disabled. Only after clicking the 'Details' button, the focuspoint can actually be altered.

To make this more obvious, I'd proposte we grey-out & dim the image, just like the other disabled inputs of this form;

Achieved with some basic CSS:

#Form_fileSelectForm .focuspoint-picker {
  filter: grayscale(100%);
  opacity: .6;
}

Upon updating a project from Silverstripe 3 to 4 (yeah I know) all focuspoints seemed a bit off vertically. It only now occurred to me that the Y axis has been swapped not just in the UI element but altogether in the whole module (which is good, otherwise it'd be a bug).

So the axis is not necessarily wrong I'd say but it is indeed inverted. But this could use some kind of migration task to migrate the existing focuspoint values between silverstripe 3 & 4. PR coming up...

FocusPoint is adding a dash to the filename. This breaks the Silverstripe convention of prepending resampling information using just alphanumerics and ending with a dash. Silverstripe's File::find() expects the dash to denote the start of the original filename. It's also breaking the SecureAssets component, which returns 404 on all focuspoint images.

The problem code appears to be line 113 in FocusPointImage.php:
$focusHash = $this->PercentageX() . '-' . $this->PercentageY();

Just remove the dash and it seems to work fine.

When a user changes the position of the focuspoint in the CMS fileSelectForm modal and then publishes the file, the UI returns to read-only mode (greyed & dimmed preview). But at this point the FP picker's position is not updated to reflect the new value (remains at its previous position).

Upon closing & reopening this modal, the picker does reflect the new FP position (so the new value gets saved correctly). Probably something to do with the "graphql context not being updated" or so...

When selecting focus point on Image in the panel Y Axis is returning wrong values. From the jquery focus point axis you get https://github.com/jonom/jquery-focuspoint/raw/master/demos/img/grid.png that top image should return Y=1, bottom Y=-1 but at the moment it is opposite.

Regarding this the module does not work out of the box with jquery focuspoint

i suggest that this should be noted in the documentation

Just starting a new project with Silberstripe's latest stable release v4.4.1 but I think FocusPoint isn't compatible — it's crashing the file viewer/editor and I'm getting Error: Injector.get(): Component FocusPointField does not exist

I tried composer vendor-expose but no joy — any ideas for a quick fix?

If the underlying file for an image is missing, clicking the file in the assets admin section results in an internal server error:

Call to a member function getWidth() on null

Which comes from here:

I think we just need an if ($previewImage) check in there somewhere, I’m not sure what happens if you omit the previewUrl/previewWidth/previewHeight` schema items 😅

[Warning] Missing argument 1 for Image::CropWidth(), called ../public/framework/view/ViewableData.php on line 446 and defined

Attempting to use some code that worked properly in 3.x I end up with the following error in 4.0

[Emergency] Uncaught Error: Call to a member function getHeight() on array
GET /invest?flush

Line 98 in /var/www/html/vendor/jonom/focuspoint/src/FieldType/DBFocusPoint.php
Source

89      * @return int
90      */
91     public function getHeight(): int
92     {
93         $height = $this->getField('Height');
94         if ($height) {
95             return intval($height);
96         }
97         if ($this->record) {
98             return intval($this->record->getHeight());
99         }
100         return 0;
101     }
102 
103     /**
104      * Set the focus point Y coordinate
Trace

JonoM\FocusPoint\FieldType\DBFocusPoint->getHeight() 
ViewableData.php:133
SilverStripe\View\ViewableData->__get(Height) 
DBComposite.php:70
SilverStripe\ORM\FieldType\DBComposite->__get(Height) 
FocusPointExtension.php:81
JonoM\FocusPoint\Extensions\FocusPointExtension->FocusCropHeight(468) 
call_user_func_array(Array, Array) 
Extensible.php:144
SilverStripe\View\ViewableData->SilverStripe\Core\{closure}(<img src="/assets/Uploads/igor-kyryliuk-ihYFBetJY98-unsplash__Resampled.png" alt="igor kyryliuk ihYFBetJY98 unsplash" /> , Array) 
CustomMethods.php:61
SilverStripe\View\ViewableData->__call(FocusCropHeight, Array) 
LazyFocusFitImageExtension.php:141

Hi,

I want to ask, is there any chance that this module can be used in Silverstripe 4 ? I use this module a lot and would like to continue using it in the next release of Silverstripe.

If a user uploads an image, picks a focus point and the image is generated, they can't then change the focus point without deleting and re-uploading the image (or a dev running the flush images task).

Quite a minor issue, given that users can simply delete and re-upload the image (which is probably the first thing they’d try if they saw it hadn’t regenerated), but would be nice to fix.

Can be ‘fixed’ by passing extra arguments when we generate the image - changing this line to this:

$this->owner->getFormattedImage('CroppedFocusedImage', $width, $height, $this->owner->FocusX, $this->owner->FocusY)

However, this results in horrible file names (CroppedFocusedImage192142-0.6613418530351438-0.17666666666666675-image.jpg for example). Perhaps you could hash the X & Y, or use the last few digits for each (as these are unlikely to be identical if the focus point is moved)?

$Image.FocusFillMax(1024,512).AbsoluteLink
$Image.AbsoluteLink
$Image.Link

Tried these settings, only the ones without FocusFillMax have links. The first one does not have a link at all.

I've been sporadically getting the following error when using this module with the latest version of 3.1.

Catchable fatal error: Argument 1 passed to 
FocusPointImage::generateCroppedFocusedImage() 
must be an instance of GD, instance of GDBackend 
given in /home/newportg/public_html/focuspoint/code/FocusPointImage.php 
on line 90

At one point it was happening every time I tried to flush, but then it started always happening on different pages, so I had to remove the module.

Any ideas?

How about extending GDBackend class and overwrite croppedresize there to automatically add the great functionality?

The picker needs to be more obvious when disabled on ss4. Perhaps a different pointer icon.

I've just installed this and it shows in the CMS and works with images in the Files area. However when using it with the Gallery module: https://github.com/frankmullenger/silverstripe-gallery
The focus point does not save. You can move it but after clicking save it defaults back to the centre. I realise this is not an issue with this module directly but an incompatibility issue. Any advice on this would be great unless I should ask Frank Mullenger?

Many thanks

Eg. when I click a PDF or other non-image File in asset-admin, I get an error message as follows:

ERROR [Emergency]: Uncaught TypeError: Argument 1 passed to JonoM\FocusPoint\FocusPointField::__construct() must be an instance of SilverStripe\Assets\Image, instance of SilverStripe\Assets\File given
IN GET /admin/assets/schema/fileEditForm/4
Line 45 in /shared/httpd/site/htdocs/vendor/jonom/focuspoint/src/FocusPointField.php

Makes upgrading from old projects easier

Setting 'FocusY' => '1' at https://github.com/jonom/silverstripe-focuspoint/blob/1.x/code/FocusPointImage.php#L13 doesn't seem to work. Even onBeforeWrite isn't catching it after adjusting these points and re-saving.
This could also be overwritten in YAML (much better choice I understand) but I'm just experimenting currently and not getting this to work.

Found a problem where the values saved to DB was without the . for delimeters.

I had used setlocale(LC_ALL, 'da_DK'); in my_config.php to change all language settings to Danish. This causes the values to be incorrectly saved to DB without the decimal delimeter.

BTW: thank you for a very nice module - i use it for all my projects :o)

If an Image gets deleted but references still exists, FocusFill & FocusFillMax 'll error like: "Uncaught InvalidArgumentException: Width is required". A check like if ($this->owner->exists()) in FocusFill would prevent this. Would it be aceptable to just wrap the return in such a condition?

Hey!

Just a quick question. I'm currently fighting in my projects to reduce the number of tables generated by silverstripe (simply to make dumps faster, etc)

Attaching the extension on the image create three new table Image, Image_Live and Image_Versions

Although it makes more sense to attach it to the image, do you think it could make sense to actually attach the extension to all files? Or at least have the fields generated in the File table to avoid these 3 tables from being generated?

Hey @jonom,

I wonder if I missed anything on how to get this up and running. I installed the focuspoint module v3.1.1 via composer on my project (CMS + FW: 4.4.4). Afterwards, I've cleared the silverstripe-cache/ folder and ran dev/build?flush=all. Then I added:

    use JonoM\FocusPoint\Forms\FocusPointField;

    // [...]

    /**
     * @var array
     */
    private static $has_one = [
        'FocusImage' => '\SilverStripe\Assets\Image',
    ];

    // [...]

    public function getCMSFields()
    {
        $fields = parent::getCMSFields();

        // [...]

        $fields->addFieldToTab(
            'Root.Main',
            new FocusPointField('FocusImage', 'Focus', $this->FocusImage())
        );

        // [...]
        return $fields;
    }

to a Data Object and ran dev/build again. The image field got generated as expected. The CMS gives me this error afterwards:

When I try it without the image as a parameter new FocusPointField('FocusImage', 'Focus') I get a regular looking and working upload field. The database table includes the new fields (while the content is NULL):

Am I forgetting or missing something? Do I use the module correctly? Thanks for your help in advance!

Cheers,
Peter

I tried to combine focuspoint with https://github.com/heyday/silverstripe-responsive-images but it doesn't play well:

[User Error] Uncaught InvalidArgumentException: Filename FocusFillWzkwMCw1NjJd/DSC-2010.jpg that should be used to cache a resized image is invalid

Backtrace:

Image->cacheFilename(FocusFill,900,562)
call_user_func_array(Array,Array)
Image.php:724
Image->getFormattedImage(FocusFill,900,562)
call_user_func_array(Array,Array)
ResponsiveImageExtension.php:114
Heyday\ResponsiveImages\ResponsiveImageExtension->createResponsiveSet(Array,Array,responsivegalleryset)
ResponsiveImageExtension.php:67
Heyday\ResponsiveImages\ResponsiveImageExtension->__call(responsivegalleryset,Array)
Heyday\ResponsiveImages\ResponsiveImageExtension->responsivegalleryset()
call_user_func_array(Array,Array)
Object.php:748
Object->__call(ResponsiveGallerySet,Array)
ViewableData.php:446
Image->ResponsiveGallerySet()
ViewableData.php:446
ViewableData->obj(ResponsiveGallerySet,,,1)
ViewableData.php:519
ViewableData->XML_val(ResponsiveGallerySet,,1)

my config for responsive images module:

---

After: 'silverstripe-responsive-images/*'

Heyday\ResponsiveImages\ResponsiveImageExtension:
sets:
ResponsiveGallerySet:
method: FocusFill
arguments:
'(min-width:1000px)': [900,562]
'(min-width:800px)': [600,375]
'(min-width:400px)': [400,250]
default_arguments: [900,562]

So i try to use FocusFill as a default method for generating that image set. Fill works fine.

From what i see in the code it's because we don't have a generateFocusFill method available.

CroppedFocusedImage also throws an error the first time, parameter missing...

I have a pretty new site - and for some reason I am not seeing the FocusPoint field appearing in the admin.

https://www.dropbox.com/s/ifwg9g6zrho3b87/Screenshot%202017-02-10%2013.20.17.png?dl=0

I have tried various SS versions, updating the module - so I'm not sure -- I may be missing sometihng.

When using the ImagickBackend instead of the GDBackend for image manipulation, the module crashes because ImagickBackend lacks the crop method.

Not entirely sure if this should be fixed in SilverStripe core instead (eg. extending the Image_Backend interface with some additional methods, such as crop and also implement them in the ImagickBackend)?

With Image versioned now in SS4, newly uploaded (still in draft) image should have its FocusPointX and Y automatically set to 0.

As this module is very popular please use tags for versioning the module and if possible semantic versioning. This way updating your great module becomes much easier than just using dev-master branch.

I've inherited a project that uses this addon and after upgrading to SilverStripe 3.6.0 i've run into a bunch of interesting warnings.

[User Warning] DataObject::__construct passed The value 'random/image.jpg'. It's supposed to be passed an array, taken straight from the database. Perhaps you should use DataList::create()->First(); instead?

The error appears to be trigger by this change silverstripe/silverstripe-framework@1d36f35.

From my understanding this might make the FPImage_cached class in code/FPImage useful now?

To test this theory, if I uncomment the FPImage_cached class and swap out the class in yml as below my warnings disappear.

Injector:
  Image_Cached:
    class: FPImage_cached

Does all this sound sensible @jonom?