[Validator] Add min/max amount of pixels to Image constraint #22288

akeeman · 2017-04-05T08:50:11Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets
License	MIT
Doc PR	symfony/symfony-docs#7756

Useful for asserting image sizes/areas in pixels.

Moved from 3.3.0 to 3.4.0

ro0NL · 2017-04-06T07:54:31Z

src/Symfony/Component/Validator/Constraints/Image.php

@@ -25,6 +25,8 @@ class Image extends File
    const TOO_NARROW_ERROR = '9afbd561-4f90-4a27-be62-1780fc43604a';
    const TOO_HIGH_ERROR = '7efae81c-4877-47ba-aa65-d01ccb0d4645';
    const TOO_LOW_ERROR = 'aef0cb6a-c07f-4894-bc08-1781420d7b4c';
+    const TOO_FEW_ERROR = '1b06b97d-ae48-474e-978f-038a74854c43';


TOO_FEW_PX_ERROR?

That probably is a bit better, indeed.

ro0NL · 2017-04-06T07:58:58Z

src/Symfony/Component/Validator/Constraints/Image.php

@@ -72,6 +78,8 @@ class Image extends File
    public $minWidthMessage = 'The image width is too small ({{ width }}px). Minimum width expected is {{ min_width }}px.';
    public $maxHeightMessage = 'The image height is too big ({{ height }}px). Allowed maximum height is {{ max_height }}px.';
    public $minHeightMessage = 'The image height is too small ({{ height }}px). Minimum height expected is {{ min_height }}px.';
+    public $minPixelsMessage = 'The image has to few pixels ({{ pixels }} pixels). Minimum amount expected is {{ min_pixels }} pixels.';


This will show an arbitrary number of pixels ({{ pixels }}), perhaps {{ width }} × {{ height }} is more user friendly.

I think the error message should directly show the problem, so at least show the direct comparison between the set limit and the problematic value. If it helps, maybe there should be some kind of calculation shown. This will make the message more bloated, however. What can be done is that al least the height and width parameters will be set, so custom messages can use these.
If someone has an idea about this, please let us know :)

What can be done is that al least the height and width parameters will be set, so custom messages can use these.

Makes sense 👍 no strong opinion about the default message (just proposed an alternative). Personally i'd favor 300 × 250 vs. 75000 pixels

apfelbox · 2017-04-06T09:19:54Z

Just a curious question: what is the use case?
Couldn't the same constraint be set by using minWidth and minHeight explicitly?

ro0NL · 2017-04-06T09:31:20Z

This would constraint an image based on square units, whatever the width/heights may be. One could prefer such a constraint over e.g. filesize based.

apfelbox · 2017-04-06T11:55:31Z

Ok, I am just thinking of a valid usage of just this one constraint.

eg. minPixels = 10000 would accept an 1px x 10000px size image - probably not what is wanted. So this can only work reasonably in combination with the existing minWidth and minHeight constraints.

I would think that minWidth + minHeight + possibly minRatio/maxRatio would fit in every use case I can come up with much better than just the raw pixel count.
That's why I am asking for a specific use case.

…ssage, fixed bad english

ro0NL · 2017-04-06T12:44:52Z

Fair question! I havent one :) I can only assume one may want to restrict based on pixels (vs. dimensions or filesize). So i support the feature for the sake of completion solely.

akeeman · 2017-04-06T13:17:03Z

Fair question indeed. I hope this answer makes it more clear. I'm using this for two things: the first is an image wall where images may have a maximum area, but I don't care about their shape. The second is that processing images takes memory that scales with the area, so again I don't care about the shape, but I do about the area.

So why can't we restrict based on the currently available constraints? It's all about area. Consider Table 1, with an examples of a tiny image with a max area of 6 pixels squared. Based on this we can create a constraint set given by Table 2. Now consider an image of 3x3 pixels with ratio 1.00 and area 9 px². It falls well within the currently available constraint range, but is not valid due to it's oversized area. Beging able to validate the area helps to overcome this shortage.

In real world applications, of course, an image width of for example 1 pixel is not very useful most of the time. Therefore a combination of minWidth and minHeight or ratio constraints is a good idea to do.

width (px)	height (px)	w/h ratio (no unit)	area (px²)
1	6	0.17	6
2	3	0.33	6
3	2	0.67	6
6	1	6.00	6

Table 1: example values for an image of at most 6 px².

constraint type	min	max
width	1	6
height	1	6
ratio	0.17	6

Table 2: currently available constraint set.

apfelbox · 2017-04-06T13:24:04Z

@akeeman thanks for your explanation!

Validating at the approximate cost of processing the image is a perfect use case. 👍

fabpot

Can you change the base to 3.4 instead of master and rebase on current 3.4? Thanks.

fabpot · 2017-07-06T07:47:40Z

src/Symfony/Component/Validator/Constraints/Image.php

@@ -25,6 +25,8 @@ class Image extends File
    const TOO_NARROW_ERROR = '9afbd561-4f90-4a27-be62-1780fc43604a';
    const TOO_HIGH_ERROR = '7efae81c-4877-47ba-aa65-d01ccb0d4645';
    const TOO_LOW_ERROR = 'aef0cb6a-c07f-4894-bc08-1781420d7b4c';
+    const TOO_FEW_PX_ERROR = '1b06b97d-ae48-474e-978f-038a74854c43';


Could you rename to TOO_FEW_PIXEL_ERROR? Same for the other constant?

akeeman · 2017-07-06T09:31:32Z

I've cleaned up my forks a while ago, deleting the branch associated with this PR. I've found no way of restoring or re-linking a branch, leaving me unable to edit this PR. Therefore I opened a new one, succeeding his one. Please check #23431.

@fabpot

…traint (akeeman) This PR was merged into the 3.4 branch. Discussion ---------- [Validator] Add min/max amount of pixels to Image constraint | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | | License | MIT | Doc PR | symfony/symfony-docs#7756 Useful for asserting image sizes/areas in pixels, for instance to estimate processing work load. **This PR continues #22288**. I cleaned up my associated branch a while ago and found no way of restoring or re-linking. @fabpot: "Could you rename to [TOO_FEW_PIXEL_ERROR](https://github.com/akeeman/symfony/blob/9ab5263d712881f8aa24920631685d14acd3a19b/src/Symfony/Component/Validator/Constraints/Image.php#L28)? Same for the other constant?" This is done. @fabpot "Can you change the base to 3.4 instead of master and rebase on current 3.4? Thanks." This is done too. Commits ------- 9ab5263 add minimum and maximum amount of pixels to Image validator

…raint (akeeman) This PR was submitted for the master branch but it was squashed and merged into the 3.4 branch instead (closes #7756). Discussion ---------- [Validator] Add min/max amount of pixels to Image constraint Documentation for symfony/symfony#22288 Commits ------- 32f90ee Add minPixels and maxPixels constraints

Arjan Keeman added 2 commits April 5, 2017 09:40

add minimum and maximum amount of pixels to Image validator

3053cfe

update Changelog file

6e1beaa

carsonbot added Status: Needs Review Validator Feature labels Apr 5, 2017

akeeman mentioned this pull request Apr 5, 2017

[Validator] Add min/max amount of pixels to Image constraint symfony/symfony-docs#7756

Closed

nicolas-grekas added this to the 3.4 milestone Apr 5, 2017

Amount of pixels scheduled for milestone 3.4.0

846e264

Moved from 3.3.0 to 3.4.0

ro0NL reviewed Apr 6, 2017

View reviewed changes

improved constant names, added height and with parameters to error me…

431d4bf

…ssage, fixed bad english

fix tests

44fae97

nicolas-grekas approved these changes Jul 6, 2017

View reviewed changes

fabpot approved these changes Jul 6, 2017

View reviewed changes

fabpot requested changes Jul 6, 2017

View reviewed changes

This was referenced Jul 6, 2017

[Validator] Add min/max amount of pixels to Image constraint #23429

Closed

[Validator] Add min/max amount of pixels to Image constraint #23431

Merged

akeeman closed this Jul 6, 2017

Uh oh!

[Validator] Add min/max amount of pixels to Image constraint #22288

[Validator] Add min/max amount of pixels to Image constraint #22288

Uh oh!

Conversation

akeeman commented Apr 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ro0NL Apr 6, 2017

Choose a reason for hiding this comment

Uh oh!

akeeman Apr 6, 2017

Choose a reason for hiding this comment

Uh oh!

ro0NL Apr 6, 2017

Choose a reason for hiding this comment

Uh oh!

akeeman Apr 6, 2017

Choose a reason for hiding this comment

Uh oh!

ro0NL Apr 6, 2017

Choose a reason for hiding this comment

Uh oh!

apfelbox commented Apr 6, 2017

Uh oh!

ro0NL commented Apr 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

apfelbox commented Apr 6, 2017

Uh oh!

ro0NL commented Apr 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

akeeman commented Apr 6, 2017

Uh oh!

apfelbox commented Apr 6, 2017

Uh oh!

fabpot left a comment

Choose a reason for hiding this comment

Uh oh!

fabpot Jul 6, 2017

Choose a reason for hiding this comment

Uh oh!

akeeman Jul 6, 2017

Choose a reason for hiding this comment

Uh oh!

akeeman commented Jul 6, 2017

Uh oh!

Uh oh!

akeeman commented Apr 5, 2017 •

edited

Loading

ro0NL commented Apr 6, 2017 •

edited

Loading

ro0NL commented Apr 6, 2017 •

edited

Loading