Make WordPress Core

Opened 12 years ago

Last modified 4 years ago

#18947 assigned enhancement

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

Reported by: f-j-kaiser's profile F J Kaiser Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.2.1
Component: Media Keywords: has-patch dev-feedback
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 12 years ago.
18947_2.patch (5.3 KB) - added by F J Kaiser 12 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 12 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 11 years ago.
Updated to current nightly build trunk
18947_5.patch (1.3 KB) - added by F J Kaiser 10 years ago.
The new 3.8 compatible patch from DrupalCamp Vienna 2013
18947_5_1.patch (1.8 KB) - added by F J Kaiser 10 years 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 10 years 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 9 years 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 9 years 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 (34)

#1 @F J Kaiser
12 years ago

  • Cc 24-7@… added

@F J Kaiser
12 years ago

#2 @F J Kaiser
12 years ago

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

#3 follow-up: @SergeyBiryukov
12 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 Kaiser
12 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 Kaiser
12 years ago

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

#4 in reply to: ↑ 3 @F J Kaiser
12 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.

#5 @F J Kaiser
12 years ago

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

#6 @F J Kaiser
12 years ago

  • Keywords dev-feedback added

#7 @F J Kaiser
12 years ago

  • Owner F J Kaiser deleted

@F J Kaiser
11 years ago

Updated to current nightly build trunk

@F J Kaiser
10 years ago

The new 3.8 compatible patch from DrupalCamp Vienna 2013

#8 @ericlewis
10 years 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 10 years ago by ericlewis (previous) (diff)

#9 @kitchin
10 years 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.

#10 @ericlewis
10 years 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 a patch together?

Version 0, edited 10 years ago by ericlewis (next)

@F J Kaiser
10 years ago

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

@F J Kaiser
10 years 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.

#11 @F J Kaiser
10 years 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.

#12 @SergeyBiryukov
10 years ago

  • Milestone changed from Awaiting Review to 4.0

18947_5_2.patch looks good to me.

#13 follow-ups: @ericlewis
10 years 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().

#14 in reply to: ↑ 13 @F J Kaiser
10 years 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().

#15 in reply to: ↑ 13 @F J Kaiser
10 years 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.

#16 follow-up: @krogsgard
10 years 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.

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


10 years ago

#18 @helen
10 years 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.

#19 in reply to: ↑ 16 @F J Kaiser
10 years 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 10 years ago by F J Kaiser (previous) (diff)

#20 @hypedtext
10 years 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...

#21 @markoheijnen
10 years 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.

#22 @johnnyb
9 years 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 9 years ago by johnnyb (previous) (diff)

@johnnyb
9 years ago

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

@johnnyb
9 years ago

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

This ticket was mentioned in Slack in #feature-respimg by kaiser. View the logs.


8 years ago

#24 @SergeyBiryukov
7 years ago

#39583 was marked as a duplicate.

#25 @Howdy_McGee
4 years ago

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

This patch is 4 years old and needs tested. There seems to be interest in moving forward with this change.

Note: See TracTickets for help on using tickets.