WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 4 weeks ago

#18947 assigned enhancement

get_intermediate_image_sizes() should also contain width/height/crop values as sub array

Reported by: F J Kaiser Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.2.1
Component: Media Keywords: needs-patch
Focuses: Cc:

Description

Currently the function get_intermediate_image_sizes() only displays a combined list of built-in/default & (via add_image_size()) registered image size names. In lot's of cases it would be pretty handy to also have the height, width & crop values attached as sub array as you can see it in $GLOBALS['_wp_additional_image_sizes'].

I currently do not have a working dev version of wp installed, so I put it here as plain code:

Change for get_intermediate_image_sizes():

function get_intermediate_image_sizes() {
	global $_wp_additional_image_sizes;
	$image_sizes = array('thumbnail', 'medium', 'large'); // Standard sizes
	foreach ( $image_sizes as $size ) {
		$image_sizes[ $size ]['width']	= intval( get_option( "{$size}_size_w") );
		$image_sizes[ $size ]['height'] = intval( get_option( "{$size}_size_h") );
		// Crop false per default if not set
		$image_sizes[ $size ]['crop']	= get_option( "{$size}_crop" ) ? get_option( "{$size}_crop" ) : false;
	}
	if ( isset( $_wp_additional_image_sizes ) && count( $_wp_additional_image_sizes ) )
		$image_sizes = array_merge( $image_sizes, $_wp_additional_image_sizes );

	return apply_filters( 'intermediate_image_sizes', $image_sizes );
}

The only two other affected lines in core are pretty a simple fix: Use the array_keys() only:

wp_generate_attachment_metadata()
and
wp_delete_attachment():

foreach ( array_keys( get_intermediate_image_sizes() ) as $s ) {

I'm not really shure how the crop works, so this could maybe add additional DB calls if the option wasn't set - currently I only see thumbnail_crop added to the autoloaded options.

The links are to the 3.2 branch of the repo.

Attachments (9)

18947.patch (2.2 KB) - added by F J Kaiser 4 years ago.
18947_2.patch (5.3 KB) - added by F J Kaiser 4 years ago.
Adds a boolean argument to the function that switches between a flat numerical indexed array or a an associative array containing the complete image size data. Added to provide backwards compability to not break existsing plugins using the function.
18947_3.patch (1.2 KB) - added by F J Kaiser 4 years ago.
Removed code (belonging to some other ticket) that was accidently still in the patch.
18947_4.patch (1.3 KB) - added by F J Kaiser 3 years ago.
Updated to current nightly build trunk
18947_5.patch (1.3 KB) - added by F J Kaiser 21 months ago.
The new 3.8 compatible patch from DrupalCamp Vienna 2013
18947_5_1.patch (1.8 KB) - added by F J Kaiser 15 months ago.
Complete Patch, altering the function, fixing the missing argument from 18947_5 patch.
18947_5_2.patch (1.4 KB) - added by F J Kaiser 15 months ago.
For the sake of discussion, here's a second patch which moves the additional functionality outside the originally patched function and builds upon it.
18947_6.patch (1.5 KB) - added by johnnyb 4 weeks ago.
Adds a get_image_size() (singular) function. Very similar to @krogsgard's code in comment 16.
18947_6_2.patch (1.4 KB) - added by johnnyb 4 weeks ago.
Messed up 18947_6.patch, (didn't allow for crop to be an array). This fixes it.

Download all attachments as: .zip

Change History (31)

comment:1 @F J Kaiser4 years ago

  • Cc 24-7@… added

@F J Kaiser4 years ago

comment:2 @F J Kaiser4 years ago

  • Keywords has-patch added
  • Version changed from 3.2.1 to 3.3

comment:3 follow-up: @SergeyBiryukov4 years ago

  • Version changed from 3.3 to 3.2.1

Version field indicates when the enhancement was initially suggested, let's keep it at 3.2.1.

Wouldn't this break a lot of plugins currently using it? Probably should be implemented as another function.

@F J Kaiser4 years ago

Adds a boolean argument to the function that switches between a flat numerical indexed array or a an associative array containing the complete image size data. Added to provide backwards compability to not break existsing plugins using the function.

@F J Kaiser4 years ago

Removed code (belonging to some other ticket) that was accidently still in the patch.

comment:4 in reply to: ↑ 3 @F J Kaiser4 years ago

Replying to SergeyBiryukov:

Wouldn't this break a lot of plugins currently using it? Probably should be implemented as another function.

Current patch avoids breaking.

comment:5 @F J Kaiser4 years ago

  • Owner set to F J Kaiser
  • Status changed from new to assigned

comment:6 @F J Kaiser4 years ago

  • Keywords dev-feedback added

comment:7 @F J Kaiser4 years ago

  • Owner F J Kaiser deleted

@F J Kaiser3 years ago

Updated to current nightly build trunk

@F J Kaiser21 months ago

The new 3.8 compatible patch from DrupalCamp Vienna 2013

comment:8 @ericlewis15 months ago

Are there good use cases for this other than more declarative code?

I don't see a huge benefit here. Media functions are already obtuse, and this isn't a great developer experience improvement.

Last edited 15 months ago by ericlewis (previous) (diff)

comment:9 @kitchin15 months ago

Patch.5 has a problem: $keys_only is undefined because it's missing from the function arguments.

A plus for this patch is that otherwise plugin developers must use the $_wp_additional_image_sizes global variable, which has the scare underscore.

comment:10 @ericlewis15 months ago

  • Keywords needs-patch added; has-patch dev-feedback removed

If we were going ahead with this, I would suggest creating a new top-level function, something like get_image_sizes() so we're not tacking on unintuitive parameters to existing functions, and parity with add_image_size(). Would someone like to put a patch together?

Last edited 15 months ago by ericlewis (previous) (diff)

@F J Kaiser15 months ago

Complete Patch, altering the function, fixing the missing argument from 18947_5 patch.

@F J Kaiser15 months ago

For the sake of discussion, here's a second patch which moves the additional functionality outside the originally patched function and builds upon it.

comment:11 @F J Kaiser15 months ago

  • Keywords has-patch 2nd-opinion added; needs-patch removed

Replying to kitchin:

Patch.5 has a problem: $keys_only is undefined because it's missing from the function arguments.

Fixed.

Replying to ericlewis:

If we were going ahead with this, I would suggest creating a new top-level function, something like get_image_sizes() so we're not tacking on unintuitive parameters to existing functions, and parity with add_image_size(). Would someone like to put a patch together?

It's an *optional* argument to keep it backwards compatible. Not adding yet another function to the 2.5k+ public function imo is a benefit (for the global namespace). If anyone got a strong opinion on if this should be a new function or not: There are two patches attached, satisfying both ideas. Whatever argument "wins", we are prepared. The only +1 argument I can possibly find is IDE autocompletion. So if I would search for "image" functions and scan through the list (and in case the name is good enough) then I'd maybe quickly find that function instead of writing my own procedure.

comment:12 @SergeyBiryukov15 months ago

  • Milestone changed from Awaiting Review to 4.0

18947_5_2.patch looks good to me.

comment:13 follow-ups: @ericlewis15 months ago

Custom image sizes width/height info is not stored in the database, there should be a fallback to go to the global $_wp_addtional_image_sizes array to fetch this data.

Also, suggested previously get_image_sizes(). _data() sounds superfluous and lacks parity with add_image_size().

comment:14 in reply to: ↑ 13 @F J Kaiser15 months ago

Replying to ericlewis:

Custom image sizes width/height info is not stored in the database, there should be a fallback to go to the global $_wp_addtional_image_sizes array to fetch this data.

Actually the options have defaults... well, partly. I took a clean (single- as well as multi-site) install, added a blank dev/test theme (nothing aside from an empty style.css, index.php and functions.php) and wrote the following (mu-)plugin to confirm:

<?php
/** Plugin Name: (dev) Options and Globals dump on shotdown */

/**/
add_action( 'shutdown', function()
{
	$defaults = wp_load_alloptions();

	foreach ( get_intermediate_image_sizes() as $s )
	{
		var_dump( "Currently looking at size: {$s}" );

		# Remove "post-" from `post-thumbnail`
		# Use it with the Twenty* themes and calls 
		# to set_post_thumbnail_size(), else you will hit notices
		# $s = str_replace( "post-", '', $s );

		# Dump of attached callbacks
		$cbs = [
			"default_option_{$s}_size_w",
			"default_option_{$s}_size_h",
			"default_option_{$s}_crop",
		];
		foreach ( $cbs as $cb )
			isset( $GLOBALS['wp_filter'][ $cb ] )
				? var_dump( $GLOBALS['wp_filter'][ $cb ] )
				: var_dump( "No callback on the options defaults for: {$cb}" );

		# Dump of default option values
		foreach ( [
			'size_w',
			'size_h',
			'crop',
			] as $part )
		{
			# Let's take a look at the default options output:
			isset( $defaults["{$s}_{$part}"] )
				? var_dump( $defaults["{$s}_{$part}"] )
				: var_dump( "No default for: {$s}_{$part}" );

			# Let's take a look at the option output:
			var_dump( get_option( "{$s}_{$part}" ) );
		}
	}

	var_dump( $GLOBALS['_wp_additional_image_sizes'] );

}, PHP_INT_MAX -1 );
/**/

The output in a completely vanilla install will be the following:

string 'Currently looking at size: thumbnail' (length=36)
string 'No callback on the options defaults for: default_option_thumbnail_size_w' (length=72)
string 'No callback on the options defaults for: default_option_thumbnail_size_h' (length=72)
string 'No callback on the options defaults for: default_option_thumbnail_crop' (length=70)
string '150' (length=3)
string '150' (length=3)
string '150' (length=3)
string '150' (length=3)
string '1' (length=1)
string '1' (length=1)
string 'Currently looking at size: medium' (length=33)
string 'No callback on the options defaults for: default_option_medium_size_w' (length=69)
string 'No callback on the options defaults for: default_option_medium_size_h' (length=69)
string 'No callback on the options defaults for: default_option_medium_crop' (length=67)
string '300' (length=3)
string '300' (length=3)
string '300' (length=3)
string '300' (length=3)
string 'No default for: medium_crop' (length=27)
boolean false
string 'Currently looking at size: large' (length=32)
string 'No callback on the options defaults for: default_option_large_size_w' (length=68)
string 'No callback on the options defaults for: default_option_large_size_h' (length=68)
string 'No callback on the options defaults for: default_option_large_crop' (length=66)
string '1024' (length=4)
string '1024' (length=4)
string '1024' (length=4)
string '1024' (length=4)
string 'No default for: large_crop' (length=26)
boolean false
null

I've written it so it is quite verbose in its output. As you can see, there are no callbacks attached for defaults, but everything has a default value. Ok, everything aside from the medium and large crop sizes - this would maybe need a separate ticket - which I will take into account with the upcoming patch.

The last value is the return from what you suggested, $GLOBALS['_wp_additional_image_sizes'], which is NULL. The reason for that is quite simple: The global gets filled by add_image_size(). And if it was not used (which is the case for core image sizes) then the global simply stays empty.

Conclusion: Don't rely on globals.

Also, suggested previously get_image_sizes(). _data() sounds superfluous and lacks parity with add_image_size().

comment:15 in reply to: ↑ 13 @F J Kaiser15 months ago

Replying to ericlewis:

Custom image sizes width/height info is not stored in the database, there should be a fallback to go to the global $_wp_addtional_image_sizes array to fetch this data.

The following options are set during installation since 2.5:

	'thumbnail_size_w' => 150,
	'thumbnail_size_h' => 150,
	'thumbnail_crop' => 1,
	'medium_size_w' => 300,
	'medium_size_h' => 300,

and since 2.7 we got

	'large_size_w' => 1024,
	'large_size_h' => 1024,

Crop really isn't there as default. Imo and as this will be a new function, we should leave it being not set, so whoever writes a patch core later on can implement it independently and without thinking about anything relying on it. Else we would make false assumptions or define new settings just for the sake of an array key.

comment:16 follow-up: @krogsgard15 months ago

As shown in 18947_5_2.patch, get_image_sizes_data() won't return anything for custom image sizes, since that data isn't in the database and the global that stores that data isn't in the foreach loop, though the global is getting called from get_intermediate_image_sizes() anyway.

Perhaps another route is to create get_image_size_data() instead, which would also have more uniformity with add_image_size(). Then (unfortunately), a second function for get_image_sizes_data() that calls the first in its own foreach loop.

If you do something like the following, you can get single image size data without calling the global, as long as it's a standard size. It also avoids calling get_intermediate_image_sizes(). That function is a nightmare of a name considering it gets all image sizes, but says it only gets intermediate.

function get_image_size_data( $size = 'thumbnail' ) {
	$default_image_sizes = array( 'thumbnail', 'medium', 'large' ); // Standard sizes
	if ( in_array( $size, $default_image_sizes ) ) {
		$result['width'] = intval( get_option( "{$size}_size_w" ) );
		$result['height'] = intval( get_option( "{$size}_size_h" ) );
		// If not set: crop false per default.
		$result[ $size ]['crop']   = false;
		if ( get_option( "{$size}_crop" ) ) {
			$result[ $size ]['crop'] = get_option( "{$size}_crop" );
		}
	} else {
		global $_wp_additional_image_sizes;
		if ( in_array( $size, array_keys( $_wp_additional_image_sizes ) ) ) {
			$result = $_wp_additional_image_sizes[ $size ];
		}
	}

	return $result;
}

I think the ability to just get a single image size's data may suffice for many use cases (like a function to call to a CDN or something). When it's not, the second function to loop through all image sizes could call this one.

I'm not certain two functions is the best route, but I certainly don't think get_image_sizes_data() serves much purpose without the ability to get custom image size data in addition to defaults.

comment:17 @ircbot14 months ago

This ticket was mentioned in IRC in #wordpress-dev by helen. View the logs.

comment:18 @helen14 months ago

  • Keywords needs-patch added; has-patch 2nd-opinion removed
  • Milestone changed from 4.0 to Future Release

Sounds like we need a new patch to address custom image sizes. Seems like a reasonable enough thing to do, though.

comment:19 in reply to: ↑ 16 @F J Kaiser14 months ago

Replying to krogsgard:

As shown in 18947_5_2.patch, get_image_sizes_data() won't return anything for custom image sizes, since that data isn't in the database and the global that stores that data isn't in the foreach loop, though the global is getting called from get_intermediate_image_sizes() anyway.

That's the point. Why would we try to return something that isn't there. And the default assumption has to be the most minimal setup. As soon as this is taken into account (it already is) and not failing, we can build upon it.

If you do something like the following, you can get single image size data without calling the global, as long as it's a standard size.

function get_image_size_data( $size = 'thumbnail' ) {
	// (...) shortened internals
	if ( in_array( $size, $default_image_sizes ) ) {
		$result[ $size ]['crop'] = get_option( "{$size}_crop" );
	} else {
		$result = $_wp_additional_image_sizes[ $size ];
	}

	return $result;
}

This would for default sizes return an Array and a string for custom image sizes.

It also avoids calling get_intermediate_image_sizes(). That function is a nightmare of a name considering it gets all image sizes, but says it only gets intermediate.

The get_image_sizes_data() (18947_5_2.patch) patch removes that necessity by wrapping it up. I want to stay back from introducing doubled functionality just to remove a function name. You would have to move get_intermediate_image_sizes() to deprecated.php (and inform users to use the new function) just for the sake of renaming it.

Last edited 14 months ago by F J Kaiser (previous) (diff)

comment:20 @hypedtext12 months ago

Is this the right place to request that get_intermediate_image_sizes() or any new related function to accept imageid as an argument? So plugin devs can get a specific response related to available sizes (including custom) for specific images.

Here's my use case:

Good old Regenerate Thumbnails is failing on my host, so I'm trying to write a plugin that will check for custom image sizes to see if they're available for a given image as I can't be certain due to restrictions at WP Engine.

Hope that makes sense. Hope this is within the scope of your discussion...

comment:21 @markoheijnen12 months ago

hypertext: I would not do that. Since the meaning is different. You can request the meta for an image. You could check out https://github.com/markoheijnen/Improved-image-editor where I did implemented what you want.

comment:22 @johnnyb4 weeks ago

I came here after finding that something like get_image_size( 'sizename' ); or get_images_sizes() doesn't exist.

Use case:
I want to add a srcset attribute to a post thumbnail that provides 2x and 3x options for the image, (assuming it's a fixed pixel width). The wp_get_attachment_iamges_attributes filter gets the arguments $attr, $attachment, and $size, the last of which can be either a width/height array or a named size. In order to generate URLs for the 2x and 3x image sizes I need to know what size I'm dealing with, and if $size is something like 'homepage_hero_poster' that doesn't tell me much.

Naming:
We've already got add_image_size(), has_image_size(), and remove_image_size() as well as get_intermediate_image_sizes() (all in media.php). Adding get_image_size( $sizename ) would have name parity with the first 3 functions mentioned above, and would let someone easily implement a get_image_sizes() function themselves. Alternately we could implement both.

There's already decent-looking code for a get_image_sizes() function on the [Codex page for get_intermediate_image_sizes()](https://codex.wordpress.org/Function_Reference/get_intermediate_image_sizes).

Last edited 4 weeks ago by johnnyb (previous) (diff)

@johnnyb4 weeks ago

Adds a get_image_size() (singular) function. Very similar to @krogsgard's code in comment 16.

@johnnyb4 weeks ago

Messed up 18947_6.patch, (didn't allow for crop to be an array). This fixes it.

Note: See TracTickets for help on using tickets.