TYPO3 Core

Please note that tracking down that problem would have been much easier without that bug: http://bugs.typo3.org/view.php?id=1189

Hi,
thanks for your patch :)
it did not become clear to me (maybe i am a bit stupid right now ;) ) what I can throw out just for the "real" solution.
Is it correct that I could throw out the commented lines at the bottom and the $this->maskNegate variable completely (and all places where it is used)?
Or are there more changes needed?
Greets, Sebastian

THANK YOU!

I could not get this to work without your patch... I recommend including it into the main source, as it fixes all the problems.

Tested with ImageMagick-6.2.3-Q16 on windows XP

Who is in charge of that part of the code? Who did the graphicmagick implementation. It might be a good idea, to take what I did so far and code a solution that integrates graphicmagick, im4 - im6 support and may be opened for other toolkits as well.

However, the first step should be to get rid of the work-arounds that try to address the problem in the graphic functions directly instead of on the layer above.

My patch gets rid of the work-arounds which are spread all over the place and provides a concentrated solution in just a few lines.

I would really like to see this patch or a derivative work to be implemented and I am certainly willing to provide a more advanced patch myself.

So dear Typo3 team, please get in contact with me, give me some feedback on my patch, let me know in which direction you are going with graphicmagick support, so I can incorporate this in my patch.

elias

The GraphicsMagick implementation was made by me. However I have no time to look at this now.

Just a small question: Can you try if the original 3.8.0 would work if you set TYPO3_CONF_VARS[GFX][im_version5] to "im6"?

I ask because I was aware that IM changed the parameters order (just like GraphicsMagick did from its beginning), but I didn't document the "im6" property anywhere because I was not sure if it solves all problems. However, it seems that this even works on some IM5 setups...

Did you read my question? Please give a feedback, otherwise I'll close this bug.

I will have time to answer your question in September. I am too busy right now, but I will be on vacation then. But I think, that I remember, I had some other issue with this, hmm.

However, in my opinion simply closing this bug is not appropriate, since even if it works, it leaves the code in a totally messed up state, addressing this problem at several places in the code and not providing a straight forward solution that every user can get right, without the need to read the code. The installer interface would stay messed up as is too.

So if you want to use im6 as the standard operation to address the im5+ problem, the workaround fragments in the lower level functions (or higher depending on your point of view. i mean the ones that make use of combineExec), should be taken out.

My patch does that!

I think to remember, that it still provides the possibility to turn off certain effects, ... which was needed for some early bad performing IM5 versions.

What it does not address is to clean up the installer interface, getting rid of all the parts, that do not make any sense anymore and explain to the user, what is actually going on.

I already proposed to deliver such an extended patch. Since I already invested so much time to investigate that problem, I would like to make sure, this problem is gone for good.

I like your im6 approach and the fact that you placed it in imageMagickCommand instead of in combineExec. I would keep that as is.

However, I like my way of swapping more, since it is hardcoded and therefore easier to read! I see no need for the way you did it.

So, what do you say, do you want me to go ahead and solve this issue? If so, do you want me to base my patch on 3.8.0 or some CVS version?? I prefer 3.8.0, since I have ftp/http access to that version.

I would paste the patches here in about 2-3 weeks and you can upload them into CVS?!

elias

Hi Elias,

I cannot make any promises to you. I understand your concern but please keep in mind that the most important thing is to keep the compatibility for people who upgrade their installation.

So even if your change would result in a clean way, it has to be assured that this will not break any existing installations.

Please keep that in mind while working on a patch.

Regards, michael

PS: It's fine to make the patch based on 3.8.0

I did some testing.
Actually the IM_V5 flag should be an IM_V6 flag, as it should be 0 (zero) for both gm 1.1.6 and im 5.4.7 (and also for im 4.2.9).

Btw. it seems the Text compositing is also dependant on gdlib v2, because on my system with gdlib v1 the text compositing goes wrong with both gm 1.1.6 and im 5.4.7, showing a grey instead of violett background. While on my test server with gdlib v2 it works with gm 1.1.6, im 4.2.9 and im 6.0.6 (im 6.0.6 only works with this patch in 3.8.0).

Where is the im_version im6 implemented? I didn't see references to it in 3.8.0... I'd like to check it out.

The problem obviously lies not in the t3lib_stdgraphics library. This one already handles the "maskNegate" parameter of the T3 Configuration properly.

The problem lies in the Install Tool. It just calls the "combine" IM function without prior checking if the mask needs to get negated (combine does just what it should - combine the images - no negateMask handling in this method).

The attached patch:
Install_Tool_maskNegate_2005-10-21.diff
fixes the issue in the install tool.

Berndhard,

Please have a closer look at the way the initial author of the IM5 workaround dealt with the problem. I just found out that he not only deals with the problem in several functions in the class.t3lib_stdgraphic.php but also in other classes (like class.tslib_content.php)!

WHY? To make the code totally unreadable? This problem can be dealt with in one or two lines of code at the right position in the code and that is what my patch tried to do.

Don't you agree that, since IM combine behavior changed it should be dealt with in IM combine function and not in every function that makes use of it? This is just ridiculous and I can't believe that nobody seems to see that or even care.

Don't you understand that the code will get more and more complicated if things are done like that?

Don't you guys understand that the more code you write to deal with that problem the higher the potential to introduce a new bug and to forget to deal with the problem at a certain point in the code?

Typo3 "developers" are very fast to shout at people who state that their code is messed up: "Stop complaining and provide patches!" But they are extremely slow in implementing those patches if somebody provides one. Rather they implement some dirty fix, like making the install tool not use the combine function directly (what the hell is wrong with that?) but use another function that uses the combine function.

Sorry that I am loosing my patience, but this is just STUPID!

I will provide another patch that will find every place where somebody tried to deal with the "negate" problem somewhere else but in the combine function and substitute that mad approach by 3 lines of code in just one place.

How about that?

I hope this time the patch will be implemented!

Elias: Constructive comments, please!

You are right that it is not the most beautiful solution, but since this works everywhere instead in the Install Tool, it is the most logical solution to make this change only in one place!

Unless you cannot prove that the Bernhards fix does not work, I'm in favour of using this one.

My patch "Install_Tool_maskNegate_2005-10-21.diff" changes the problem with the NEGATE mask just in the Install Tool. No changes in tslib_content (Pherhaps you looked at my other patch for the GIFBUILDER 256 colors problem).

You are rigth when you state that the "negation" should happen in a central place like the combineExec method. But that would break every existing extensions which handle the maskNegation on their own ... And T3 is well known for its downward compatibilty.

Pherhaps this behaviour can get changed for T3 4.0.0 and has to get noted in the Update-HowTo.

And the core-team can't just include every patch if somebody says: Yes it works. We have to test it on our own - I don't want to commit anything which I haven't proven myself to work on ALL IM versions like expected.

And please notice that Michael Stucki has already taken care of the order of the IM combine/composite parameters in T3 3.8.0 (Look in t3lib_stdgraphic.php ... you have a call to t3lib_div::imageMagickCommand which takes care of the order)

I will make further investigations and you can be sure that in the next release this problem is at least fixed. The clean-up will follow for the next major release.

Backward compatibility can easily be achieved!
All you have to do is to change the behaviour of the install tool to allow im_negate_mask to be turned off even when im_version_5 is selected and find a place where im_negate_mask gets turned off in any case.

Therefore the modules think they work with IM4 but since im_version_5 is set t3lib_div::imageMagickCommand would swap the arguments.

OK, I see the point.
But please, this should really get cleaned out in the next big release!!!! Cleaned out completely!!! It introduces more problems than it solves! I still do not understand, why this ever was approached like that instead of in one single function?!

elias

I committed Bernhards solution to CVS for 3.8.1 and later.