Closed maPer77 closed 3 years ago
The overlay code can be done in a cleaner way, I kept it that way to make it easier to see the changes ... Then I send another suggestion for this code.
I'm not sure I understand what this is intended to do, but wouldn't this achieve the same effect?
overlay('watermark.png', 'bottom right', 1, 0, 0)
No, if using offset 0 the image will be glued to the edge ... the goal is to have a space between the edge and the watermark image
overlay('watermark.png', 'bottom right', 1, 0, 0)
Apologies if I'm not understanding your intent here, but I don't think it will be obvious to other users either. What is the $borderOffset
argument supposed to do, and why is it a boolean instead of a pixel value?
the goal is to have a space between the edge and the watermark image
Can you not do this with the existing $xOffset
and $yOffset
arguments?
$ borderOffset is boolean because it only serves to indicate how the offset will be applied ... As offset pattern is applied to the top and left of "watermark.jpg" this causes "watermark" to be out of the image if I use "bottom right" as in the example I posted in the first message ...
Can you not do this with the existing $xOffset and $yOffset arguments?
yes, I can use it, but for that I have to use negative value, for example:
overlay('watermark.png', 'bottom right', 1, -20, -20)
With $ borderOffset true it looks like this:
overlay('watermark.png', 'bottom right', 1, 20, 20, true)
How the offset works today, I must check which position to set the offset value correctly ...
Then I apply the overlay:
$img->overlay($watermark, $position, 1, $x, $y);
with $ borderOffset true, I simply do this:
$img->overlay($watermark, $position, 1, 10, 10, true);
Did you understand what i meant ?
OK, thanks for explaining that — it wasn't obvious that this was purely for convenience and I was pretty sure it could already be done with the existing parameters. The change makes more sense now. 😄
I'm OK with adding it, but I'd suggest changing the name to $calcuateOffsetFromEdge
. The current name $borderOffset
isn't intuitive because the two preceding arguments are $xOffset
and $yOffset
which accept an integer. (That threw me off a bit too.)
The only other thing that needs to be done is to update the docs to describe the new argument. It's probably worth pointing out that $xOffset
and $yOffset
have no effect when center
is used as an anchor.
I don't know if the standard way of calculating the offset is useful for someone, speaking for me I don't see any use. Could we not eliminate the standard form of calculation and use only this new form of calculation?
Your approach is more convenient, but modifying the default would be a breaking change so it would need to wait for 4.0. Since the library is fairly mature, I don't have any plans for 4.0 on the roadmap, but if that time comes I'd definitely like to revisit it.
Calculates the offset for the image edges.
$borderOffset false
overlay('watermark.png', 'bottom right', 1, 20, 20, false)
$borderOffset true
overlay('watermark.png', 'bottom right', 1, 20, 20, true)