Version: 7.x
Sandbox: http://drupal.org/sandbox/OxideInteractive/1817208
Git clone: git clone --recursive --branch 7.x-1.x OxideInteractive@git.drupal.org:sandbox/OxideInteractive/1817208.git pointcrop

The pointcrop module is for smart cropping of images using drupal image styles.

The module provides 2 things:

  • The ability to select a single point on an image field image to use as the focal point when cropping.
  • "Pointcrop scale and crop" and "Pointcrop crop" image effects, which use that focal point.

The reason this module was created was to be able to have images that display in vastly different sizes and in different orientations, but always keep the subject in the frame.

There are a number of other image cropping modules which were reviewed before creating this one.

  • Smart crop: Tries to focus automatically based on entropy. This doesn't work well in practice.
  • Imagefield crop, Imagefield focus, Manual crop, Image javascript crop: These modules are based around the idea of the user being able to select a rectangle/crop area for uploaded images, which is then used for cropping. This is a more complex solution that doesn't give as much flexibility in terms of the size and orientation of the final image.

There is also a jQuery plugin (https://github.com/oxideinteractive/pointcrop), that is able to use the focus points this module provides to do dynamic cropping on the front end.
This means image size & orientation can change dynamically and the images will still also focus on the focal point.

Integration with this plugin will be provided out of the box in future, most likely in the form of a views plugin.

A demo site will very likely be set up for the plugin.

Screenshots will also soon be added to the project page, demonstrating pointcropped images vs regular cropped images.

Review bonus
#1907552: PHP Execute Extended Tools (rooby)

Comments

ramsonkr’s picture

quick review:

There are many issues related to indentations, extra spaces etc

http://ventral.org/pareview/httpgitdrupalorgsandboxoxideinteractive18172...

Focus point description,The point consists of x and y values of between 0 and 1

provide an example value.

bripatand’s picture

Manual review:
Nice module. See to work as expected after some basic testing.

Beside the few errors reported before by pareview, the code is well written and make appropriate use of the Drupal hooks according to me.

You may want to use two fields instead of one field with two values separated by a comma.

OxideInteractive’s picture

Thanks for the reviews.

I have pushed the coding standards changes from #1 in now.

I have run it through drush dcs now and it got no errors.
- As an aside, there is a lot of mention of using the coder module so that you don't have these kinds of time wasting issues during this process. However the current recommended release of coder does not catch these things, so it is a bit misleading.

In regards to one DB field vs two, I will have a think about that.

I was thinking seeing as they are coordinates that come as a set, they are fine as a single value, however there could be a chance that someday someone finds it useful to have the numbers stored in the database as decimals instead of varchar, so that they can do math with the numbers more easily.

I guess if it is going to change now would be the best time for such a change.

sittard’s picture

Status: Needs review » Needs work

Nice module, well coded and documented. I tested it on a image library site that currently uses Imagefield focus and found that it produces good results. The functionality is really simple and I would imagine even a non technical content manager would have no problem quickly setting the focus point.

Some mainly non technical feedback:

  1. The module provides similar functionality to a number of existing modules as already identified. From personal experience the module is very close to Imagefield focus. I think this module would be a great addition to Imagefield focus and I would recommend contacting the module developer to see if you can join forces.
  2. Good set of instructions in the README.txt and project homepage.
  3. Please include a version number (7.x-dev) in the module .info file.
  4. It's not possible to remove the image without setting a focus point. I kept getting the following error message 'You must set a focus point for the image'. Ideally you should be able to remove the image without setting the focus point first!
  5. I found the following error message 'The specified point value is invalid. The x and y values must both be a number between 0 and 1.' was not that clear. It does not specify that you need to enter two numbers between 0 and 1 seperated by a comma.
  6. It would also help to instruct the user to actually click on the image to set the co-ordinates.
  7. I did wonder if hiding the co-ordinate input text box entirely would also aid user functionality (I noticed you were also having some debate about using two fields).
  8. I would suggest removing the additional jQuery plugin feature or providing clearer documentation on it's intention. After installing the libraries API and the additional jquery pointcrop library. I started looking for the additional functionality, took me a little while to figure out that this was only provided as a mechanism for conveniently installing the Jquery plugin (unless I missed something).

Overall a really nice clean module and I'd be keen to see this module get released. Good work!

rooby’s picture

I am posting as a committer on the pointcrop project.

Thanks for the review. I will action the code related items in the next couple of days, but for now:

1. That was considered, however it was decided against for a couple of reasons. One being that, while similar, they are both doing different things, and the other is that integration with the jQuery plugin is planned, at which point it really is a module of it's own, and there would be no need to have the imagefield_focus functionality.
2. There is already some instructions in the README (which is also linked to from the documentation link on the project page). Detailed instructions are not necessary on the project page itself, that is what the documentation is for.
3. It is wrong to include a version number in the .info file. Drupal.org handles that automatically.
4. I will look into fixing that.
5. I see your point. It might be that it changes to two separate fields, as per #2 & #3, in which case it would be easier to follow.
6. I will add something to make that clear.
7. The field is visible currently due to the fact that you can also set the point manually in the field if you want to be more precise. Maybe it could be an option of the field settings form to show or hide it. I might do that.
8. It is noted that there is no out of the box implementation of the plugin yet (it will be coming), however I will try to make it more clear.

Thanks again for the review and I will fix the actionable items soon.

willvincent’s picture

Please also consider: #1831570: Compatibility with media module?. The available cropping modules that work with media (as far as I can tell) are few & far between. Given that the implementation is fairly straightforward, and huge popularity of the media module, including support for it would be beneficial, both for users of the media module, and the success of this module.

anrikun’s picture

The reason this module was created was to be able to have images that display in vastly different sizes and in different orientations, but always keep the subject in the frame.

ImageField Focus does this already.

rooby’s picture

@anrikun:
Based on that quote, I see now that it does provide a very similar thing. If you ignore the crop rectangle.

In fact, the real #1 reason it was created was to be able to more easily achieve this - http://www.canberra.edu.au/alumni/photo-archive

Where images of any dimensions can be uploaded by anonymous users.
They are then shown on that page, with a complex, semi date based sort, in a way that they could appear anywhere, at almost any size (depending on where they appear and how many images are around them).
Navigation, cropping, placement, etc. is all on the front end and images must be pre-image-cached, before they are pointcropped in the front end.
When you filter by searching, the same images can appear in different places, at different sizes.

So there are some differences between the two modules:
1. The module is a drupal end for the PointCrop plugin. https://github.com/oxideinteractive/pointcrop
Basically to make it easy to get the data required by the plugin out from drupal.
It would be possible to get the same data out of imagefield focus, with a little bit more work.
The jQuery plugin was the #1 purpose, when this module was originally created.
I see that there is an imagefield focus plugin coming soon also, which will be another way these modules differ.
It is common to have different drupal modules that provide integration with different jQuery plugins for similar purposes.

2. This module has a simpler interface. Both for admins (field settings) and end users (focus selection).
For the purpose of crop focusing, you only need a single point, and it is preferred to keep the interface as simple as possible for users to avoid confusion. An option to hide the text field will also make this even more streamlined.

3. When the user is able to select a rectangle, they are given influence over the scaling & cropping.
If they can only possibly select a single point they cannot affect the result.
There are two sides to this:
One is that when selecting a rectangle, they can select a vastly different rectangle than where the image will appear elsewhere on the site. In this case the image might have to be compromised when it is made to fit in the target space.
For example, on a 1000x1000 image, a portion 1000x100 is selected in the middle. If that selection has to be placed into an area 200x1000, you cannot easily get that selection to fit into that area.
The other side is that when selecting a single point, in the case of the 1000x1000 original, the user has to select a single point, which can then easily be placed into the 1000x200 space. The tradeoff is that if the focus point is a bunch of people in a line all the way across that image, the user is forced to select a single point and people outside that point could be cropped out, however there will still be a reasonable focus point.
So in that way the functionality is actually different.

4. Storage is different. This module stores a ratio, not actual x,y coords.
This means that when the jQuery plugin comes to the image, even if it is not the same size it was when the focus point was selected, the ratio will still be valid.
If the original image is 1000x1000; and the focus point is 500,500; and then you have to pointcrop a 100x100 version of the image on the front end; the 500,500 coordinate has just become useless.
And in this case you don't want to be outputting the original 1000x1000, because in this case it is guaranteed that no image will be over 100x100, so pushing out a hundred 1000x1000 images when you don't need to is unnecessary slowness.
Another bonus to storing as a ratio, is that on the node edit form, images can be scaled down to fit in the width of the form, without affecting the ratio.

So although these modules on the surface have a very similar purpose, there are a number of important differences.
It would be possible to merge the two modules, however due to these differences you would either have to change one of the modules functionality somewhat to fit nicely in with the other one, or else just be smooshing two different modules together into one.

I'm all for not duplicating efforts, however if there is difference enough I don't see the harm. Especially if bugs are getting fixed.
Also, these are not modules that require a huge amount of man hours to keep maintained, so I can't see there being a situation where both modules have hundreds of issues and everyone would be better off if everyone was working on a single issue queue.

For example, we have TAC, TAC Lite, content access, node access, etc.
These modules all provide access control, but in different ways.

These two modules provide focused cropping, but in different ways.

anrikun’s picture

I didn't mean that this was a duplicate effort. I just wanted to point out that there is already a module that what made for the same purpose, even if the way the 2 modules handle the task may be different.
Everybody is free to make modules, and if you think that yours might help users, you are welcome. Users are then free to pick up the one that best suit their needs :-)

adnasa’s picture

just a general question because I need to learn this also

The following line

/**
  * Include the image effects.
 */
require_once dirname(__FILE__) . '/pointcrop.effects.inc';

Shouldn't be enough to add the pointcrop.effects.inc in .info file along with the files[] array
so that it is automatically included?

willvincent’s picture

Shouldn't be enough to add the pointcrop.effects.inc in .info file along with the files[] array
so that it is automatically included?

My understanding is that the autoloader is for classes, the contents of the pointcrop.effects.inc file is procedural code.
I could be wrong, but that's been my understanding on how registering files with the files[] array in the .info file works.

anrikun’s picture

@adnasa: willvincent is right, the files[] array in the .info file is for classes only.

adnasa’s picture

#11: @willvincent, thanks for pointing that out! this is good info :-)

rooby’s picture

@anrikun:
No worries.
I guess it is good to have the differences listed any way as people will wonder.

klausi’s picture

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

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

If you reopen this please keep in mind that we are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

rooby’s picture

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

This is definitely still being worked on.

Will try to do some reviews too.

PA robot’s picture

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

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

I'm a robot and this is an automated message from Project Applications Scraper.

rooby’s picture

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

Still going on this...

PA robot’s picture

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

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

I'm a robot and this is an automated message from Project Applications Scraper.

rooby’s picture

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

This still isn't dead.

willvincent’s picture

Status: Needs work » Reviewed & tested by the community

I really enjoy using this module, and while it is still only a sandbox project that hasn't been granted permission for promotion to a full project, I use it quite regularly on client sites. It's been my experience that its simple interface provides all the necessary power for focusing the subject of an image to be cropped, and as it's based on a single point is the most flexible of all the other options that aim to provide similar functionality currently on D.O.

For these reasons, and as per #3 indicating that all specified coding standard issues have been addressed, I'm going to mark this RTBC, and recommend it be accepted.

It works as advertised, is lightweight, and much more flexible than existing options.

rooby’s picture

Thanks Will for the review, it's good to see that people are using this.
I also use it on a couple of sites that have been live for a long time.

The more I think about the remaining tasks in this issue the more I think they don't belong here.

Point 4 from comment #4 is a bug that needs fixing.
The rest are pretty much just feature requests and potential feature requests, which is not what this process is about.

I also added a project I reviewed.
I know it's only 1 so far and the project has gone cold but it was a thorough review.

I actually really still want to see that module so I will try to push the maintainer to get it out.
I was so close to RTBCing it too.

rooby’s picture

I have added a bug report to the pointcrop issue queue for point 4 of #4.

I will add issues for the feature requests soon.

abhitesh.das’s picture

Title: Pointcrop » [D7] Pointcrop

Changed Issue title. FYI: http://drupal.org/node/1011698

stBorchert’s picture

Status: Reviewed & tested by the community » Needs work

Hi.
There are still several errors you need to fix before this can be flagged as "reviewed and tested":

Master Branch
It appears you are working in the "master" branch in git. You should really be working in a version specific branch. The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.
Major coding standards / best practice issues
An automated review of your project has found some issues with your code; As coding standards make sure projects are coded in a consistent style we please you to have a look at the report and try to fix them. Anyway, note that issues found are possibly false positives and fixing all issues is not a requirement for getting through the application process.
PAReview: Individual user account
It seems you are using a non-individual account.
All user accounts are for individuals. Accounts created for more than one user or those using anonymous mail services will be blocked when discovered (see Get a Drupal.org account).
Please change your account information and enter your realname.

Please keep in mind that we are currently quite busy with all the project applications and we can only review projects with a review bonus. Please help us reviewing and we'll take a look at your project right away.

PA robot’s picture

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

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

I'm a robot and this is an automated message from Project Applications Scraper.

PA robot’s picture

Issue summary: View changes

Added a link to a project that was reviewed.