WordPress.org

Make WordPress Core

Opened 17 months ago

Closed 17 months ago

Last modified 17 months ago

#22512 closed enhancement (duplicate)

WP_Image_Editor - image_resize_dimensions called twice

Reported by: alexvorn2 Owned by:
Milestone: Priority: normal
Severity: minor Version: 3.5
Component: Media Keywords:
Focuses: Cc:

Description

Problem 1: - Prevent calling the image_resize_dimensions function twice.

When we need to resize an image we need first check if the resized image file name exists and after that - resize.

For checking if the resized image exist we need the new width and height that we can get from image_resize_dimensions(),

and after that we resize, using the resize function from the class, in the function we have image_resize_dimensions function too, so it happens that it is called twice...

Problem 2 - Cropping image position

For example we cropped an image with a 'top' position, (default crop is center position)
and after that if we will want a new position like center or bottom, or right/left we should delete the old cropped image before cropping the new one.

Problem 3 no documentation, no examples
this new class was integrated to fast, and the old functions were deprecated in the favor of this class, why? why not to deprecate in future versions?
No documentation and examples in codex, so the theme developers should found but itself how to integrates this new option of resizing images before the new version of WordPress to come out (release).

Change History (13)

comment:1 nacin17 months ago

  • Component changed from General to Media
  • Version set to trunk

comment:2 scribu17 months ago

after that if we will want a new position like center or bottom, or right/left we should delete the old cropped image before cropping the new one.

How is that different from before?

the old functions were deprecated in the favor of this class, why?

Because it's confusing to have two ways of doing things. Deprecated functions still work just like before; you just get a message if you have errors turned on.

comment:3 alexvorn217 months ago

Replying to scribu:

after that if we will want a new position like center or bottom, or right/left we should delete the old cropped image before cropping the new one.

How is that different from before?

Example we have an image with 800px height and 400px width, we will crop the image, and the default crop will be in the center 400x400px (image-400x400.png), but if we will want to crop from the top to the center with the same 400x400? we should delete the image before cropping a new one. It a nice idea to add position text in the file name: image-400x400-top.png (timthumb had these options to use image position for cropping, that's why it is so popular plugin)

comment:4 scribu17 months ago

It a nice idea to add position text in the file name: image-400x400-top.png

Sure, but that's not a regression from WP 3.4. See #19393

comment:5 scribu17 months ago

  • Summary changed from WP_Image_Editor - image_resize_dimensions called twice, cropping position to WP_Image_Editor - image_resize_dimensions called twice

comment:6 follow-up: markoheijnen17 months ago

I don't think problem 1 is an issue. Since the check inside the editor is needed. The one outside the class I can't remember but if it's there it has it's rights and it shouldn't be combined.

Problem 2 can be considered as an issue inside the editor. Not sure if the editor should care about it as in the code that is calling the editor should care about that. You can change the filename on save. multi_resize is something different in this case. As Scribu said it's not a regression but for me it would be a nice to have it in 3.6

Problem 3: Well documentation is good to have but to already start to complain about it is lame. If you miss them you can create the pages yourself. That why the codex is so awesome, everyone is allowed to write content on it. Eventually there will be blog post(s) about all the changes but that can be even done after the release. Deprecation doesn't mean that it's getting removed fast. Also writing good content costs a lot of time.

comment:7 in reply to: ↑ 6 alexvorn217 months ago

Replying to markoheijnen:

Problem 3: Well documentation is good to have but to already start to complain about it is lame. If you miss them you can create the pages yourself. That why the codex is so awesome, everyone is allowed to write content on it. Eventually there will be blog post(s) about all the changes but that can be even done after the release. Deprecation doesn't mean that it's getting removed fast. Also writing good content costs a lot of time.

I would like to create but I'm not an good expert in this.

comment:8 follow-up: DH-Shredder17 months ago

Agreed that more documentation is needed, and there will be further docs developed as part of this cycle. The beginning of this, of course, can be found within PHPDoc in trunk. I also recommend checking out some of core's uses of the editor, since the existing wp_crop_image() and deprecated image_resize() are good basic primers in that regard.

For clarification's purposes, are you arguing that there is a place in core code that image_resize_dimensions() is called twice, or is it only a complaint that your current code is requiring that the function be run once outside by yourself, then again within WP_Image_Editor? I don't see image_resize_dimensions() getting called in core except within the editors and image_resize_dimensions() itself (in a filter).

comment:9 in reply to: ↑ 8 ; follow-up: alexvorn217 months ago

Replying to DH-Shredder:
How do you check if the image you want to resize is already resized?
I use image_resize_dimensions to calculate and get the dimensions... I don't think there is other solution.
Or else the image will just resize every time you load the code.

My purpose of creating this ticked is to reduce the load of the script because the function is called twice, to improve just a little.

Last edited 17 months ago by alexvorn2 (previous) (diff)

comment:10 in reply to: ↑ 9 DH-Shredder17 months ago

  • Cc mike.schroder@… added
  • Severity changed from normal to minor
  • Type changed from defect (bug) to enhancement

Replying to alexvorn2:
What method were you using with the previous functions?

It'd depend on your implementation as to what's currently best. WordPress knows what files are in its media library, and if they're not there, your filesystem knows. If you really don't know what the final size of your image will be, then calling image_resize_dimensions is exactly what you want to do, and it isn't likely to cause performance problems.

As far as a framework to handle situations like this, I can see it happening at some point in the future, but likely not within WP_Image_Editor, since media library situations, and relationships to previously generated images are beyond its scope.

If what you're looking for are improvements in how we handle resized images, and their relation to the originals, I think this would fall within #21810.

comment:12 DH-Shredder17 months ago

  • Resolution set to duplicate
  • Status changed from new to closed

Duplicate of #21810.

Sure! Closing as duplicate for now; I'd suggest further comments on the other ticket =)

comment:13 ocean9017 months ago

  • Milestone Awaiting Review deleted
Note: See TracTickets for help on using tickets.