I haven't tested on Windows.

Since alpha4 version, uploading an image seems successful, but no thumbnail nor image is generated, triggering a "This value should be of the correct primitive type" error when saving node.

Tested with png and gm.

CommentFileSizeAuthor
#19 2833315-19.patch24.74 KBmondrake
#15 2833315-15.patch27.58 KBmondrake
#15 interdiff_13-15.txt1.98 KBmondrake
#13 2833315-13.patch27.88 KBmondrake
#13 interdiff_11-13.txt10.66 KBmondrake
#11 2833315-11.patch24.6 KBmondrake
#11 interdiff_9-11.txt16.87 KBmondrake
#8 2833315-9.patch11.7 KBmondrake
#7 2833315-7.patch11.7 KBmondrake
#7 interdiff_6-7.txt1002 bytesmondrake
#6 2833315-6.patch11.71 KBmondrake
#3 imagemagick-broken_image_generation-2833315-3.patch831 bytessimgui8
#2 imagemagick-broken_image_generation-2833315-2.patch831 bytessimgui8

Comments

guillermo-simeon created an issue. See original summary.

simgui8’s picture

simgui8 commented
StatusFileSize
new imagemagick-broken_image_generation-2833315-2.patch831 bytes

Reverting 1st line in the parseFileViaIdentify() function to the alpha3 version fix the issue (at least with gm on linux).

simgui8’s picture

simgui8 commented
StatusFileSize
new imagemagick-broken_image_generation-2833315-3.patch831 bytes

Previous patch is inverted...

Here is the right patch.

mondrake’s picture

mondrake
Location 🇮🇹
commented

@guillermo-simeon thank you for your report.

1. Is this happening only with GraphicsMagick? What is your version? With GraphicsMagick 1.3.20 I cannot reproduce that.

2. Are you getting any error logged in the dblog?

3. Can you switch on the debugging option (http://cgit.drupalcode.org/imagemagick/tree/README.txt?id=deeba2c#n148) and report back commands and output?

mondrake’s picture

mondrake
Location 🇮🇹
commented
Title: Image generation is broken » Image generation is broken with GraphicsMagick
Priority: Normal » Critical
Issue tags: +Needs tests

So I checked and indeed it looks like the '%[]' syntax in the -format argument is not supported by GraphicsMagick.

The fix from @guillermo-simeon in #3 can temporarily work.

However:
1) this exposes the fact that we are missing tests for GraphicsMagick integration.
2) if we have both IM and GM installed on the server, then 'identify' may be run against IM even if the toolkit is set to use GM. This is because 'identify -format' is executed, whereas we should execute 'gm identify -format' when GM is selected.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Active » Needs review
StatusFileSize
new 2833315-6.patch11.71 KB

Broader patch with separate -format argument per graphics package and execution of 'gm identify -format' if GM is selected.

Add tests that may pass on DrupalCI (as the actual execution of binaries is skipped given that neither IM nor IM are installed on bots), but locally exposes some failures on GM that need to be addressed.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Issue tags: -Needs tests
StatusFileSize
new interdiff_6-7.txt1002 bytes
new 2833315-7.patch11.7 KB
mondrake’s picture

mondrake
Location 🇮🇹
commented
StatusFileSize
new 2833315-9.patch11.7 KB

The last submitted patch, 7: 2833315-7.patch, failed testing.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Assigned: Unassigned » mondrake
Status: Needs review » Needs work

More tests to do.

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs work » Needs review
StatusFileSize
new interdiff_9-11.txt16.87 KB
new 2833315-11.patch24.6 KB

Some progress, I am not yet happy though for interaction between checkPath/imagemagickExec etc.

While running on GM the same tests that pass with IM, we have 3 points of difference that I could not work out yet:

  1. The 'crop' operation on GIF images produces files that cannot be opened by GD imagecreatefromgif() :( ???
  2. Issues with colors after rotate on PNG files
  3. The CreateNew operation in IM uses -transparent-color for new GIF files, but that is not supported by GM. Tried to use -transparent but that seems not to work.

For these I just created test bypasses, will file separate follow-up issues.

simgui8’s picture

simgui8 commented

Thanks @mondrake

The patch from #11 works fine.

mondrake’s picture

mondrake
Location 🇮🇹
commented
StatusFileSize
new interdiff_11-13.txt10.66 KB
new 2833315-13.patch27.88 KB

Thanks for feedback @guillermo-simeon :)

The patch here takes the ::runOsShell method from the 8.x-2.x branch, which is now run from ::checkPath and resolves my unhappiness in #11.

Tests pass locally with both IM and GM. Adding tests for GM is a good addition to prevent future regressions.

If this is confirmed working please RTBC.

simgui8’s picture

simgui8 commented

The patch from #13 works for me.

Thanks @mondrake

mondrake’s picture

mondrake
Location 🇮🇹
commented
StatusFileSize
new interdiff_13-15.txt1.98 KB
new 2833315-15.patch27.58 KB

Thank you. One more minor cleanup of the toolkit code and tuning of the tests.

simgui8’s picture

simgui8 commented

Patch #15 works form me.

Nice work @mandrake

  • mondrake committed ca0d1d3 on 8.x-1.x 
    Issue #2833315 by mondrake, guillermo-simeon: Image generation is broken...
mondrake’s picture

mondrake
Location 🇮🇹
commented
Version: 8.x-1.0-alpha4 » 8.x-2.x-dev
Status: Needs review » Patch (to be ported)

OK, committed #15 to 8.x-1.x

Needs porting to 8.x-2.x

mondrake’s picture

mondrake
Location 🇮🇹
commented
StatusFileSize
new 2833315-19.patch24.74 KB
mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Patch (to be ported) » Needs review

  • mondrake committed b05d3e7 on 8.x-2.x 
    Issue #2833315 by mondrake, guillermo-simeon: Image generation is broken...
mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs review » Fixed

Committed #19 to 8.x-2.x

mondrake’s picture

mondrake
Location 🇮🇹
commented

Released 8.x-1.0-alpha5.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.