Intervention / image

PHP Image Processing
https://image.intervention.io
MIT License
13.84k stars 1.5k forks source link

There's been a breaking change somewhere between 3.1 and 3.3.1 #1281

Closed simonhamp closed 7 months ago

simonhamp commented 7 months ago

Describe the bug My test suite is failing when running against 3.3.1, but passes when running against 3.1.

You can see the test suite results here

It seems to be occurring because I'm manually instantiating the driver-specific modifier.

I'm doing that so that I can use a custom version of the modifier which explicitly exposes the Imagick TextModifier's boxSize method's visibility as public.

I'm doing this so that I can re-use that mechanism of calculating the size of the text, which I'm using as an approximation to help decide where lines should be broken when the text is constrained to a specific width.

(Perhaps there is already a better way to do this?)

Code Example The files I've linked above may be the best code example.

Expected behavior Given that the tests pass when running v3.1 of intervention/image, I'd have expected them to still pass on v3.3.1 as it's not clear that there are breaking changes

Environment (please complete the following information):

olivervogel commented 7 months ago

This could well be, since this version jump it is no longer possible to instantiate the "driver specific" modifiers directly, you always have to go through the "generic" class.

This is also consistent with your error message.

If I understand you correctly, you are adding the ability to output text in a specific width with text wrapping. This is a long requested feature. See #143 Can you report on your success of the implementation? Perhaps you would like to integrate the wrapping feature directly into this library.

If I understand you correctly, one of the following things would provide a rather quick help.

simonhamp commented 7 months ago

Thanks for your rapid reply @olivervogel and confirming the issue 🙏

I don't think anything provides quick help unfortunately... it looks like I'm going to need to refactor my code one way or another to instantiate the modifier your way.

Short-term, I've locked to v3.1 as that's the version that works.

But ultimately I think the right solution here will be making boxSize public so that I don't have to create my very basic custom TextModifier. Then I could completely rely on your instantiation logic. I'm happy to PR this change.

I also agree that text-wrapping would be good as a core feature - that would also allow me to move that out of my package too. My approach is quite opinionated tho and is even still a bit buggy.

It would be great if we could collab on that piece at some point.

olivervogel commented 7 months ago

I hope I could help you with the MR. Feel free to contact me if you have any further questions or if you are interested in implementing your text wrapping solution directly into Intervention Image.