Don't include editors #2

Closed
markoheijnen opened this Issue Mar 19, 2013 · 5 comments

Comments

Projects
None yet
2 participants
@markoheijnen

Awesome stuff what you did with the code I build. When I saw the face detection I was planning the same. Didn't looked yet really good to the code but you shouldn't include image editor classes.

WP_Image_Editor_GD_Detect_Face needs it's own file and that should be included inside the wp_image_editors filter. See the code for the Gmagick plugin: http://wordpress.org/extend/plugins/gmagick

@roborourke

This comment has been minimized.

Show comment Hide comment
@roborourke

roborourke Mar 19, 2013

Contributor

It was a fairly short file so I didn't bother splitting it out yet, just wanted to get it working! Also
I am already adding the class by using the wp_image_editors filter - see the main plugin class at the top of the file :) Awesome work on that, I was really happy to discover it!

Unfortunately it's a bit too slow to use in production yet as I haven't found a good way to give the user feedback that it's detecting faces with the new media library code. There's a javascript event that fires when the upload starts and after the resizing is done but not when file is uploaded and before resizing begins as far as I could tell.

Ideally I should write a plugin that uses the actual openCV extension.

If you have any other tips for improvement I'd love to hear them!

Contributor

roborourke commented Mar 19, 2013

It was a fairly short file so I didn't bother splitting it out yet, just wanted to get it working! Also
I am already adding the class by using the wp_image_editors filter - see the main plugin class at the top of the file :) Awesome work on that, I was really happy to discover it!

Unfortunately it's a bit too slow to use in production yet as I haven't found a good way to give the user feedback that it's detecting faces with the new media library code. There's a javascript event that fires when the upload starts and after the resizing is done but not when file is uploaded and before resizing begins as far as I could tell.

Ideally I should write a plugin that uses the actual openCV extension.

If you have any other tips for improvement I'd love to hear them!

@markoheijnen

This comment has been minimized.

Show comment Hide comment
@markoheijnen

markoheijnen Mar 19, 2013

There is always the change of possible breakages.

Currently playing with an image editor in Backbone and was thinking about integrating something like this.
I think a feature like this should be done on user input. If I always upload nature images it doesn't make that much sense.

There is always the change of possible breakages.

Currently playing with an image editor in Backbone and was thinking about integrating something like this.
I think a feature like this should be done on user input. If I always upload nature images it doesn't make that much sense.

@roborourke

This comment has been minimized.

Show comment Hide comment
@roborourke

roborourke Mar 19, 2013

Contributor

I absolutely agree. Will see if I can add in a checkbox on the upload dialog that sets an option.

What's the best place to increase the max execution time for image resizing?

Contributor

roborourke commented Mar 19, 2013

I absolutely agree. Will see if I can add in a checkbox on the upload dialog that sets an option.

What's the best place to increase the max execution time for image resizing?

@markoheijnen

This comment has been minimized.

Show comment Hide comment
@markoheijnen

markoheijnen Mar 19, 2013

WordPress doesn't have a feature for that so set_time_limit(30) would do and in this case it should be inside WP_Image_Editor_GD_Detect_Face ::face_crop()

WordPress doesn't have a feature for that so set_time_limit(30) would do and in this case it should be inside WP_Image_Editor_GD_Detect_Face ::face_crop()

@roborourke

This comment has been minimized.

Show comment Hide comment
@roborourke

roborourke Mar 19, 2013

Contributor

Fixed this now - sorry I didn't see what you meant at first by not including the main editor files. I have a habit of not putting includes/requires inside functions but in this case obviously it makes sense!

Thanks for the advice and the example :)

Contributor

roborourke commented Mar 19, 2013

Fixed this now - sorry I didn't see what you meant at first by not including the main editor files. I have a habit of not putting includes/requires inside functions but in this case obviously it makes sense!

Thanks for the advice and the example :)

@roborourke roborourke closed this Mar 19, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment