Page 1 of 2

Avatar resize in core

Posted: Fri Sep 08, 2017 5:34 pm
by SalazarAG
It would be interesting if phpbb had the resize avatars feature by default. The admin sets the maximum dimensions (in px) in the ACP and when the user enters an avatar larger than the one defined in the ACP will be resized.

Re: Avatar resize in core

Posted: Fri Sep 08, 2017 7:07 pm
by koraldon
I know my users find it difficult indeed to use avatars, this will make it much easier to use.

Re: Avatar resize in core

Posted: Thu Sep 28, 2017 1:30 am
by jsebean
This semi-exists with just using gravatar.

Re: Avatar resize in core

Posted: Wed Oct 25, 2017 10:20 pm
by thecoalman
I would suggest the following.
  • Pluplaod for upload which can handle resizing and EXIF client side.
  • Setting for max size of avatar only effects avatar itself because...
  • Avatar is linked to profile, use thumbnail size image on profile page.
  • Link from thumbnail on profile page to full sized image.

Re: Avatar resize in core

Posted: Sun Nov 19, 2017 7:45 pm
by Tomba
Some code already exists for this. It only needs to be adapted and fitted into the core I think.
http://jrpickeral.com/?p=255

Various board system already support this, so a no-brainer to add this.

Re: Avatar resize in core

Posted: Sun Nov 19, 2017 9:38 pm
by david63
Tomba wrote:
Sun Nov 19, 2017 7:45 pm
Some code already exists for this
That link is for phpBB 3.0

Re: Avatar resize in core

Posted: Wed Nov 29, 2017 10:48 pm
by SalazarAG
I already know these tutorials, and Dion Design (for 3.2) really works. But the problem is that it is stressful to make this snippet every time you have to upgrade the forum to another version, which is why it would be cool if resizing avatars was a default feature. .

Re: Avatar resize in core

Posted: Fri Feb 02, 2018 1:55 pm
by Ger
I've tried to wrap my head around this for an extension, but the upload process is bugging me. Basically the only point in the entire process to hook into is core.avatar_driver_upload_move_file_before. I'm capable of resizing the file, but I cannot overwrite the protected width and height properties of the $file class. Therefore the $file->move_file() method decides that the image is still too large, aborts and throws an error.

Sorry guys, guess you'll have to wait for this to get into the core. :(

Re: Avatar resize in core

Posted: Fri Feb 02, 2018 4:01 pm
by mrgoldy
Ger wrote:
Fri Feb 02, 2018 1:55 pm
Just a thought, but if you're sure you've resized it to the correct/allowed dimensions, you could overwrite the 'allowed dimensions' from the upload class, to unlimited or original image size +1px or something like that?
Not sure if it's possible, but just an idea.

Re: Avatar resize in core

Posted: Fri Feb 02, 2018 8:46 pm
by thecoalman
Ger wrote:
Fri Feb 02, 2018 1:55 pm
I've tried to wrap my head around this for an extension, but the upload process is bugging me. Basically the only point in the entire process to hook into is core.avatar_driver_upload_move_file_before. I'm capable of resizing the file, but I cannot overwrite the protected width and height properties of the $file class. Therefore the $file->move_file() method decides that the image is still too large, aborts and throws an error.

Sorry guys, guess you'll have to wait for this to get into the core. :(
Correct me if I'm wrong here but could not the extension have it's own settings for what it will be resized to? Setting the dimensions in the core to a high amount and only used as a limit on the uploaded file size, yes?

I implemented this with a couple of lines editing core files, it's easy to do because I'm using imagemagick and that is basically how it works on my forum. In my case the resized dimensions are hard coded.

Re: Avatar resize in core

Posted: Mon Feb 05, 2018 10:34 am
by Ger
posey wrote:
Fri Feb 02, 2018 4:01 pm
Just a thought, but if you're sure you've resized it to the correct/allowed dimensions, you could overwrite the 'allowed dimensions' from the upload class, to unlimited or original image size +1px or something like that?
Not sure if it's possible, but just an idea.
That was the easiest option, but the dimensions are of course in the config and there is no sane event to hook into before the avatar processing is done. Actually, I see only core.common but that's the most ugly way to do this. @Thecoalmans approach seems more logical:
thecoalman wrote:
Fri Feb 02, 2018 8:46 pm
Correct me if I'm wrong here but could not the extension have it's own settings for what it will be resized to? Setting the dimensions in the core to a high amount and only used as a limit on the uploaded file size, yes?

I implemented this with a couple of lines editing core files, it's easy to do because I'm using imagemagick and that is basically how it works on my forum. In my case the resized dimensions are hard coded.
This can indeed work: setting the config values to idiot amounts to make sure every image is processed and then overwrite the whole thing using extension settings (either hard coded or ger_avatarresize_max_height and such).

In any case, everything feels hacky. If the properties phpbb\files\filespec::width phpbb\files\filespec::height where public instead of protected it should be a very easy extension. Or create a setter function for them, like

Code: Select all

public function dimensions_set($width, $height) 
{
	if (is_int($width) && is_int($height))
	{
		if (!empty($width)) 
		{
			$this->width = $width;
		}
		if (!empty($height)) 
		{
			$this->height = $height;
		}
	}
	return $this;
}
That way any extension can do to uploaded images whatever they want to as long as they provide integer values for dimensions. All checks can be done as usual and extensions can work without hacky work-arounds or needing to add extra data and processing to the mix.

Re: Avatar resize in core

Posted: Mon Feb 05, 2018 2:59 pm
by thecoalman
Ger wrote:
Mon Feb 05, 2018 10:34 am
In any case, everything feels hacky.
I don't think it's hacky, the admin may want to set a limit for upload. Those images are converted server side as I'm sure you are aware. I don't know if it's improved but the GD library will consume a lot of RAM for conversion and can easily exceed the RAM limits in the past. If I recall correctly am image with a width of about 5000px was pushing the limits for a 256MB value set for the max RAM a script could consume. If I understand correctly GD converts to uncompressed before conversion hence the reason it will consume so much RAM. Hopefully future version of phpBB utilize plupload for this.

As far as hardcoding the resized value that works for me but I'm sure most people will want to set their own value.

Re: Avatar resize in core

Posted: Mon Feb 05, 2018 4:11 pm
by Ger
thecoalman wrote:
Mon Feb 05, 2018 2:59 pm
Ger wrote:
Mon Feb 05, 2018 10:34 am
In any case, everything feels hacky.
I don't think it's hacky, the admin may want to set a limit for upload. Those images are converted server side as I'm sure you are aware. I don't know if it's improved but the GD library will consume a lot of RAM for conversion and can easily exceed the RAM limits in the past. If I recall correctly am image with a width of about 5000px was pushing the limits for a 256MB value set for the max RAM a script could consume. If I understand correctly GD converts to uncompressed before conversion hence the reason it will consume so much RAM. Hopefully future version of phpBB utilize plupload for this.

As far as hardcoding the resized value that works for me but I'm sure most people will want to set their own value.
Perhaps I expressed myself wrong. English is not my native language so this happens sometimes :P

What I meant is that in an ideal world you want to have a setting to do what it's intended for, or at least be used at all. Having a setting to deny avatars larger than X pixels, it should deny anything else. In your solution, while technically solid, you set that amount to 5000px while that's certainly not what you want. You actually set it to let any validation pass. In that case, the validation is actually processed but it passes since no uploaded image has those dimensions.

Now I'm sure it works and I understand the benefit of imagemagick vs. the GD library. So your solution works. Even more: phpBB's create_thumbnail() function for attachments also uses imagemagick if available. But of course: you make edits to the core file so any extension doing that would be instantly denied.

If I would create an extension I simple want to use the dimension settings to resize any larger avatar larger to a maximum of those given dimensions. So the description "maximum width/height" would still apply, there's only no error presented to the user but the avatar is resized to those dimensions in the upload process.
I've found quite an easy solution, requiring only 1 change to the core: the event core.avatar_driver_upload_move_file_before should also pass the $file object as var. I've done so locally and with it, I'm perfectly capable to resize any given avatar within the given KB range and store it as desired.

Now I only have to figure out how I get this var added to that event... It's easy to create a PR but does anyone know how I'd have to post and refer to this in the tracker? I've done something like that about a year ago but that didn't go very smooth.

Re: Avatar resize in core

Posted: Mon Feb 05, 2018 4:26 pm
by thecoalman
Ideally you have plupload handle it, have you explored that option? The image would be resized client side to the desired dimensions before upload.

---edit----
There is a bug that may affect this. The EXIF dimensions are used if present instead of determining the actual image size. You would need to alter the pluplaod values to strip the EXIF data before upload.

Re: Avatar resize in core

Posted: Wed Feb 07, 2018 11:24 am
by Ger
thecoalman wrote:
Mon Feb 05, 2018 4:26 pm
Ideally you have plupload handle it, have you explored that option?
I've looked at it, but quite frankly I don't have a complete understanding of how plupload actually works or how I should implement it in the avatar upload functionality. I do have quite some experience with resizing on the PHP side of things so it's easier for me to take that approach.

I've created a ticket and a PR to pass the $file object to given event. If that's accepted I'll post my extension in the extension development forum.