maximuslight
commented
6 months ago Thanks for the PR.
Biggest thanks to you, for this great library - it's great help, very easy to use and fun to work with to generate images!
Closed maximuslight closed 6 months ago
maximuslight
commented
6 months ago Thanks for the PR.
Biggest thanks to you, for this great library - it's great help, very easy to use and fun to work with to generate images!
olivervogel
commented
6 months ago Great. I have a few more thoughts.
With GD no outline function is natively supported, so in PR the text is written with a kind of "compound function" which writes the outline with a 3x3 matrix behind the actual text. This is perfectly ok, as the result works very well.
However, I see two things that need to be considered.
With GD and the method described above, the text is given an outline which is outside the actual text. See example:
With Imagick, however, the result (with exactly the same code) looks different and the outline is drawn in the middle of the text shape.
Unfortunately, this cannot be changed. I have already looked to see if Imagick has an option to draw the outline outside. There doesn't seem to be an option here.
I am thinking about not using the native Imagick function here, but also using the "compound method", which is already used in the GD driver. The results would be the same. What do you think?
As the GD driver draws the outline with a trick, higher values have the effect that no outline is drawn, but the 3x3 matrix becomes visible.
Example with font-size 120 & stroke-width 60.
This effect depends on the ratio of the font size and the stroke width. A quick test of mine showed that 1/5 of the font size produces an acceptable result. I'm thinking about adding a limit here and throwing an exception for stroke widths that exceed this value.
What do you think?
maximuslight
commented
6 months ago Agree, these are very valid concerns.
I am thinking about not using the native Imagick function here, but also using the "compound method", which is already used in the GD driver. The results would be the same. What do you think?
This is certainly a solution when goal is to achieve Imagick outline instead of inline. To be consistent with both drivers.
In regards limiting the stroke width, it seems that it is necessary.. however, unfortunately in my tests not 1/5 (20%) but lower value (~13%) is producing acceptable result, please see the test case below. What do you think?
I would make changes later today to limit max stroke width as well as Imagick similar compound function.
Test Case: GD driver
olivervogel
commented
6 months ago This is certainly a solution when goal is to achieve Imagick outline instead of inline. To be consistent with both drivers.
The more I think about it, the more I think this is the right way to go to let both drivers work with the same method for the moment. It would be great if you could implement this.
In regards limiting the stroke width, it seems that it is necessary.. however, unfortunately in my tests not 1/5 (20%) but lower value (~13%) is producing acceptable result, please see the test case below. What do you think?
I just did some more tests with different TTF fonts and realized that the character of the font also plays a role.
With very bold fonts (I used "Impact" in my first example), a limit of 20% is sufficient. The narrower the silhouette of the font, the smaller the limit must be to avoid the unwanted effect.
With Mooli Regular, for example, the stroke width may only be around 6-7% for a font size of 120.
It should be noted that very low outline values such as 1 - 3 usually do not cause any problems. Neither with small nor with large font sizes. The limit should therefore only be checked above a certain limit.
If you also consider that the "Mooli example" only works with 6% at 120, which corresponds to a stroke width of approx. 7, you can probably save yourself the calculation and simply allow only a certain range of values (I'm thinking 0 to 5). What do you think?
I would make changes later today to limit max stroke width as well as Imagick similar compound function.
Great. I'm looking forward to it.
maximuslight
commented
6 months ago I agree, probably percentage is not reliable due unknown font characters.
Made the above mentioned changes as promised.
Those are:
stroke(int $width, mixed $color = 'ffffff',int $limit = 5)
Please review and let me know your thoughts.
olivervogel
commented
6 months ago Thank you for your ideas. @maximuslight. I understand that you want to keep things flexible.
I think it is better to use the limit only internally in a fixed form and not make it changeable via the public API. I'm afraid that in its current form this could cause confusion and I don't like the idea of a parameter value whose range can be overridden by another parameter value.
I believe that very large stroke widths only work under very specific circumstances anyway (Bold Font & Large Font Size), so I think it's okay to only support a certain range for this library.
I even briefly considered supporting only a width of 1, as even a small value like 5 would have an unwanted effect (see example below). Not sure if this is a good idea though.
Mooli Regular, Font Size 40, Stroke Width 5
maximuslight
commented
6 months ago I removed settable limit so method remains simple:
stroke($width, $color)
Have improved the compound method, basically I looked up that one what others did in such scenario, including Goldengide. They run X,Y in all directions as many as strokeWidth needs, I placed hardcoded limit to 10 out of resource/memory considerations.
Please see test cases with Mooli Regular and Protest Strike fonts
Imagick Driver:
Gd Driver:
Please let me know your thoughts
maximuslight
commented
6 months ago Changes requested are made.
Ran docker-compose run --rm --build analysis no errors found
php ./vendor/bin/phpstan.phar analyze --memory-limit=512M --level=4 ./src no errors found
php ./vendor/bin/phpcs the only issues I see are about linebreaks everywhere not being
End of line character is invalid; expected "\n" but found "\r\n" I am not often contribute to github, my apologies, but this doesn't seem feasible to edit everywhere something so minor.
olivervogel
commented
6 months ago Changes requested are made.
Thank you very much.
Ran
docker-compose run --rm --build analysisno errors foundphp ./vendor/bin/phpstan.phar analyze --memory-limit=512M --level=4 ./srcno errors found
Thats strange. I get 4 issues here. In the GitHub workflow action they are also visible.
php ./vendor/bin/phpcsthe only issues I see are about linebreaks everywhere not being
This is even stranger, because your screenshot shows errors in files that you have not edited by you at all. The errors from your screenshot are also not reported in the GitHub action. Only the files that were edited in the PR are problematic here.
These should be fixed, I will then take care of the errors of the static analyzer.
FILE: /project/src/Drivers/Imagick/Modifiers/TextModifier.php
------------------------------------------------------------------------------
FOUND 23 ERRORS AFFECTING 14 LINES
------------------------------------------------------------------------------
34 | ERROR | [x] Whitespace found at end of line
37 | ERROR | [x] Whitespace found at end of line
38 | ERROR | [x] Expected 0 spaces after opening bracket; 1 found
38 | ERROR | [x] Expected 0 spaces before closing bracket; 1 found
38 | ERROR | [x] Expected 1 space after closing parenthesis; found 0
42 | ERROR | [x] Whitespace found at end of line
43 | ERROR | [x] Expected 0 spaces after opening bracket; 1 found
43 | ERROR | [x] Expected 0 spaces before closing bracket; 1 found
43 | ERROR | [x] Expected 1 space after closing parenthesis; found 0
44 | ERROR | [x] Space after opening parenthesis of function call prohibited
44 | ERROR | [x] Expected 0 spaces before closing parenthesis; 1 found
46 | ERROR | [x] Whitespace found at end of line
51 | ERROR | [x] Blank line found at start of control structure
52 | ERROR | [x] Whitespace found at end of line
53 | ERROR | [x] Expected 0 spaces after opening bracket; 1 found
53 | ERROR | [x] Expected 0 spaces before closing bracket; 1 found
53 | ERROR | [x] Expected 1 space after closing parenthesis; found newline
54 | ERROR | [x] Whitespace found at end of line
54 | ERROR | [x] Duplicate spaces at position 18.
56 | ERROR | [x] Blank line found at start of control structure
68 | ERROR | [x] Whitespace found at end of line
76 | ERROR | [x] Blank line found at end of control structure
76 | ERROR | [x] Whitespace found at end of line
------------------------------------------------------------------------------
PHPCBF CAN FIX THE 23 MARKED SNIFF VIOLATIONS AUTOMATICALLY
------------------------------------------------------------------------------
FILE: /project/src/Drivers/Imagick/FontProcessor.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
69 | ERROR | [x] Whitespace found at end of line
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: /project/src/Drivers/Gd/Modifiers/TextModifier.php
-----------------------------------------------------------------------------------------------
FOUND 28 ERRORS AND 1 WARNING AFFECTING 20 LINES
-----------------------------------------------------------------------------------------------
25 | WARNING | [ ] Function's nesting level (6) exceeds 5; consider refactoring the function
32 | ERROR | [x] Whitespace found at end of line
34 | ERROR | [x] Whitespace found at end of line
35 | ERROR | [x] Expected 0 spaces after opening bracket; 1 found
35 | ERROR | [x] Expected 0 spaces before closing bracket; 1 found
39 | ERROR | [x] Whitespace found at end of line
40 | ERROR | [x] Expected 0 spaces after opening bracket; 1 found
40 | ERROR | [x] Expected 0 spaces before closing bracket; 1 found
40 | ERROR | [x] Expected 1 space after closing parenthesis; found 0
41 | ERROR | [x] Space after opening parenthesis of function call prohibited
41 | ERROR | [x] Expected 0 spaces before closing parenthesis; 1 found
44 | ERROR | [x] Whitespace found at end of line
49 | ERROR | [x] Whitespace found at end of line
50 | ERROR | [x] Expected 0 spaces after opening bracket; 1 found
50 | ERROR | [x] Expected 0 spaces before closing bracket; 1 found
50 | ERROR | [x] Expected 1 space after closing parenthesis; found newline
53 | ERROR | [x] Blank line found at start of control structure
54 | ERROR | [x] Whitespace found at end of line
68 | ERROR | [x] Whitespace found at end of line
79 | ERROR | [x] Blank line found at end of control structure
79 | ERROR | [x] Whitespace found at end of line
82 | ERROR | [x] Blank line found at start of control structure
83 | ERROR | [x] Whitespace found at end of line
84 | ERROR | [x] Expected 0 spaces after opening bracket; 1 found
84 | ERROR | [x] Expected 0 spaces before closing bracket; 1 found
84 | ERROR | [x] Expected 1 space after closing parenthesis; found newline
87 | ERROR | [x] Blank line found at start of control structure
88 | ERROR | [x] Whitespace found at end of line
100 | ERROR | [x] Whitespace found at end of line
-----------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 28 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------
FILE: /project/src/Typography/Font.php
----------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 4 LINES
----------------------------------------------------------------------
142 | ERROR | [x] Expected 1 blank line after method, found 2.
147 | ERROR | [x] Whitespace found at end of line
159 | ERROR | [x] Whitespace found at end of line
169 | ERROR | [x] Whitespace found at end of line
----------------------------------------------------------------------
PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: /project/src/Typography/FontFactory.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
43 | ERROR | [x] Whitespace found at end of line
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: /project/src/Interfaces/FontInterface.php
----------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 3 LINES
----------------------------------------------------------------------
22 | ERROR | [x] Whitespace found at end of line
22 | ERROR | [x] Duplicate spaces at position 36.
23 | ERROR | [x] Whitespace found at end of line
54 | ERROR | [x] Whitespace found at end of line
----------------------------------------------------------------------
PHPCBF CAN FIX THE 4 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------- Thank you! Just to assist a little in understanding why there are different in messages:
This is likely due my inexperience with pull requests/contribution and perhaps differences in our set ups.
I have a live project (and dev clone of it, where all happens) on the Debian server that I use for battle-testing changes and when everything is good, committing to Github.
Then to comply and run tests, I was doing the following separately (from project above):
git clone https://github.com/maximuslight/image.git on my local machine to get all latest repo files.
then run docker-compose run --rm --build tests it says 2 errors because of "\r" result:
then run docker-compose run --rm --build analysis result:
then php ./vendor/bin/phpstan.phar analyze --memory-limit=512M --level=4 ./src (understand that this is same as above, but just to see it's result differs):
then php ./vendor/bin/phpcs result:
[same as screenshot in comment regarding '\n' and '\r\n' everywhere that it fills whole my cmd.]
olivervogel
commented
6 months ago I have fixed some bugs and the warnings from the pipeline. I also did some refactoring.
Your changes from this PR are squash merged in: https://github.com/Intervention/image/commit/a673763e14641e4689dfcc5a6496f177e6840353
Thanks again @maximuslight for your collaboration. Thanks also to @Goldengide , who first initiated this feature.
Added strokeWidth and strokeColor support to "develop" branch to avoid conflicts.
Tested on PHP 8.1 with both drivers.
Aware of Goldengide PR, this is similar.