Problem/Motivation
On occasion people get the following error message:
imagecolorsforindex() Color index nnn out of range in GDToolkit
On first sight the code looks OK, but still it fails from time to time.
Proposed resolution
After a long time, a reproducable test was found by @mondrake in #61. Based on this test, a solution was written that prevents this error from occurring.
1 Thing that still bugs me (@fietserwin) a bit is the fact that if imagecolortransparent() can return a rubbish index outside the number of colors, I guess it also may return a rubbish index within the range of valid color indices, and we happily use that index.
Nevertheless, this seems to be the way to go.
Remaining tasks
None, perhaps the added test should have a comment that refers to this issue, but I don't know if that's standard procedure in Drupal. This comment can be added by the committer as well, as it doesn't really change the patch.
User interface changes
None.
API changes
None.
Original report by @Alex72RM
Hi,
I have this error in log, php section.
It comes with a .jpg image.
How to solve it?
Thanks,
Alessandro
Comment | File | Size | Author |
---|---|---|---|
#127 | 375062-125-TEST-ONLY.patch | 4.74 KB | mondrake |
#125 | 375062-125.patch | 7.99 KB | mondrake |
#125 | interdiff_112-125.txt | 5.52 KB | mondrake |
#112 | drupal-375062-112.patch | 3.89 KB | David_Rothstein |
Comments
Comment #1
Duplika CreditAttribution: Duplika commentedI have the same error with 6.x-1.6 version.
Comment #2
upupax CreditAttribution: upupax commentedconfirm 6.x-1.6 issue and subscribe...
Comment #3
Duplika CreditAttribution: Duplika commentedSame here on PHP 5.2.10.
Comment #4
splash112 CreditAttribution: splash112 commentedSubscribing:
warning: imagecolorsforindex() [function.imagecolorsforindex]: Color index 255 out of range in /home/tit.nl/public_html/drupal6/sites/all/modules/imageapi/imageapi_gd.module on line 251.
ImageAPI 6.x-1.6
Comment #5
selff CreditAttribution: selff commentedsubscribe
warning: imagecolorsforindex() [function.imagecolorsforindex]: Color index 7 out of range in /home/zzz/htdocs/sites/all/modules/imageapi/imageapi_gd.module on line 251.
ImageAPI 6.x-1.6
this warning come when i create new image advertisement (module ad) *.GIF
Comment #6
foxtrotcharlie CreditAttribution: foxtrotcharlie commentedI encountered the same error with 5.x-1.5:
imagecolorsforindex() [<a href='function.imagecolorsforindex'>function.imagecolorsforindex</a>]: Color index 255 out of range in C:\Apache2\htdocs\d5\sites\all\modules\imageapi\imageapi_gd.module on line 251.
I was using imagecache to display images from a view, but it turns out those images didn't exist in my local files directory. When I pulled them in, the error disappeared, so not sure if this is a bug or not.
Comment #7
marcushenningsen CreditAttribution: marcushenningsen commentedSame error, same version. Any news on this?
Marcus
Comment #8
AcuraImport CreditAttribution: AcuraImport commentedSubscribing
Comment #9
ramya.narayanan CreditAttribution: ramya.narayanan commentedsubsribing
Comment #10
usonian CreditAttribution: usonian commentedI'm also seeing this in v6.x-1.6, PHP 5.2.6.
Comment #11
sammyman CreditAttribution: sammyman commentedSame Here! Subscribing..
Comment #12
bstrange CreditAttribution: bstrange commentedSubscribing!
Same issue that OP experienced in Feb, 2009
Edit to add: In my case it only occurrs when I apply an imagecache preset to images imported into ddblock "News Item" content, and I am just curious if this is the case with any of the other posters here?
With no acknowledgement of this issue in over a year, I suspect we may need to work this one out ourselves.
Comment #13
bstrange CreditAttribution: bstrange commentedppblaauw, the developer / maintainer of the Dynamic Display Block module(http://drupal.org/project/ddblock), kindly looked over the Image API error (http://drupal.org/node/715502)for me and found that line 251 pertains to .gif images.
The workaround for me was to convert the .gif to .jpg (and I suspect .png would work as well). Of course this workaround would not be a good solution if you are using animated .gifs; however it's something, and seeing how this issue has persisted w/o acknowledgement for over a year in this queue, I figured the workaround was better than nothing!
Thanks ppblaauw! :)
Comment #14
klavs CreditAttribution: klavs commentedThe same problem has been handled here - it seems it has to with animated gifs: http://drupal.org/node/224163 - I "shut it up" by putting an @ in front of the function call on line 251.
Comment #15
verta CreditAttribution: verta commentedsubscribing - seeing this error and I don't think our GIF files are animated. It's actually
Color index 254 out of range in /home/content/...
for us.
Comment #16
Alexander Matveev CreditAttribution: Alexander Matveev commentedAnimated gif's.
Subscribing.
Comment #17
timwee CreditAttribution: timwee commentedSubscribing
Same error only on .gif
Comment #18
smk-ka CreditAttribution: smk-ka commentedI'm seeing the same behavior. A quite easy fix seems to be to check that the returned transparency color index is within the total number of colors in the image, and drop transparency otherwise.
Comment #19
scip CreditAttribution: scip commented#18 worked for me - thanks
Comment #20
Pete B CreditAttribution: Pete B commented#18 did the job for me too - thanks
Comment #21
skat CreditAttribution: skat commentedSorry, I have the same problem, How must i install the patch?
thanks a lot!
Comment #22
intyms CreditAttribution: intyms commentedi have the same problem with 6.x-1.8
I get this error when i try to upload a gif file.
The gif is not animated and also it isn't transparent.
the patch from #18 fixes this issue for me.
Comment #23
skat CreditAttribution: skat commentedthank you intyms
Comment #24
tribsel CreditAttribution: tribsel commentedthx, same problem. solved by #18
Comment #25
dspring0021 CreditAttribution: dspring0021 commentedPatch worked for me as well...D6.1.9 on ImageAPI 6.x-1.8...animated GIF.
Comment #26
drewish CreditAttribution: drewish commentedThanks, committed to DRUPAL-6--1.
Comment #28
drzraf CreditAttribution: drzraf commentedcan reproduce (non-animated gif).
that patch apparently never reached 7.x
may be moved to core.
Comment #29
theunraveler CreditAttribution: theunraveler commentedRe-rolled for Drupal core, 7.x.
Comment #30
theunraveler CreditAttribution: theunraveler commentedOops, forgot a brace.
Comment #31
andymantell CreditAttribution: andymantell commentedThe patch in #30 worked for me. I was getting these errors with an animated gif and after the patch it is fine.
Comment #32
FiNeX CreditAttribution: FiNeX commentedPatch #30 works fine, thanks. Would it be submitted on D7 ?
Comment #33
analystsynergy CreditAttribution: analystsynergy commentedI want to use Path #30 in my Drupal 7.14 website. How do I use it? Is there any link on Drupal.org site explaining how to implement patch?
Comment #34
drzraf CreditAttribution: drzraf commentedhttps://drupal.org/patch
Comment #35
analystsynergy CreditAttribution: analystsynergy commentedThank you very much!!
I have applied the patch successfully.
Comment #36
jday CreditAttribution: jday commented#30 patch worked for me, hope it gets committed before the next update
Comment #37
lolmaus CreditAttribution: lolmaus commentedA four-year old bug has hit me too.
Comment #38
TipiT CreditAttribution: TipiT commentedGot me too. :)
Comment #39
Anonymous (not verified) CreditAttribution: Anonymous commentedWarning: imagecolorsforindex() [function.imagecolorsforindex]: Color index 255 out of range in image_gd_create_tmp()
Is it ok to just ignore this warning ?
Comment #40
drzraf CreditAttribution: drzraf commentedComment #41
lolmaus CreditAttribution: lolmaus commentedComment #42
drzraf CreditAttribution: drzraf commentedThis patch cleanly applies to 7.x, it has been tested, reviewed (and even pushed for D6).
Can't this be committed now ?
Comment #43
David_Rothstein CreditAttribution: David_Rothstein commentedLooks like the same issue exists in Drupal 8; needs to be rerolled and committed there first.
Comment #44
David_Rothstein CreditAttribution: David_Rothstein commentedOops, that is not the tag I meant to add.
Comment #45
Beanjammin CreditAttribution: Beanjammin commentedThe patch in #30 is working great for me.
Isn't the issue mislabelled, it doesn't require a backport to D7 so much as it requires a forward port to D8?
Comment #46
marcingy CreditAttribution: marcingy commentedIssues fixed in the latest version of drupal and then backported so it labeled correctly.
Comment #47
juampynr CreditAttribution: juampynr commentedHere is a patch for Drupal 8. I see that the same approach has been suggested at http://stackoverflow.com/questions/3874533/what-could-cause-a-color-inde....
Comment #49
juampynr CreditAttribution: juampynr commentedThe above patch was failing because imagecolorstotal() returns 0 for truecolor images, thus making the condition to fail. Here is an updated version where this is checked using imageistruecolor().
Comment #51
juampynr CreditAttribution: juampynr commented#49: 375062-49-color-index-out-of-range.patch queued for re-testing.
Comment #52
hswong3i CreditAttribution: hswong3i commentedPatch reroll for both 7.x/8.x
Comment #54
mgifford52: drupal-8.x-color_index_out_of_range-375062-52.patch queued for re-testing.
Comment #56
Mirolim CreditAttribution: Mirolim commented30: 375062-imagegd-transparency-29.patch queued for re-testing.
Comment #60
quotesBro CreditAttribution: quotesBro commented#859304: Color index 255 out of range in image.gd.inc patch has more diffs.
Comment #61
mondrakeHere a test only patch to demonstrate the error, and a full patch with the fix (Drupal 8).
See this Stackoverflow post for an explanation why this happens.
Comment #63
fietserwin- The if condition can be simplified to:
$transparent >= 0 && $transparent < $palletsize
This because the pallet size will 0 for true color images, so it helps to understand this if the comment would be "closer" to the condition: therefore it is better to also remove the empty line.
- Rename variable to pallet_size or palletSize (not sure which is currently preferred).
- Rephrase comment to something like "The original has a transparent color, allocate it to the new resource as well."
Comment #64
cs_shadow CreditAttribution: cs_shadow commentedModified the patch in #61 according to the changes mentioned in #62.
Renamed $palletsize to $pallet_size as its the standard followed in the file for other variables.
Comment #67
mondrakeThanks @cs_shadow for #64.
we need to keep the condition like
($transparent >= 0 && ($pallet_size == 0 || $transparent < $pallet_size))
: if the image is truecolor, $pallet_size will be 0, but $transparent in GIF will be set to a value, so we get the test failure.so:
truecolor => $pallet_size == 0, $transparent == n ==> set $transparent_color
non-truecolor => $pallet_size == m, $transparent == n, with n<m ==> set $transparent_color
non-truecolor => $pallet_size == m, $transparent == n, with n>=m ==> DO NOT set $transparent_color
Comment #68
fietserwinHmmm, thus a truecolor image can have a colr set as transparent color. Not what I expected, so my fault.
Question: But how can imagecolortransparent return an index that does not exist in the resource?
Answer: http://stackoverflow.com/questions/3874533/what-could-cause-a-color-inde... (@Mondrake: you found the same?)
As the if condition is not easy to read/interpret, I would like to propose to split it into 2 if statements and comment it more elaborately (possibly referring to the given url). Something like this?
(BTW: $transparent will be -1 if no transparent color has been defined in the image (resource). As this is not documented, let's explicitly mention that as well.)
Comment #69
cs_shadow CreditAttribution: cs_shadow commentedIf we implement the solution suggested in #68, wouldn't the 'else if' part get affected? Because originally
else if
it was triggered when either of the two conditions were false and now it will be called only if($transparent >= 0)
evaluates to be false.Apart from that, do we really need to be this much descriptive with comments?
Comment #70
fietserwinThe elseif is part of the type "switch", so the if we are talking abut does not have an else?
How much description is enough? I had a very hard time getting:
- what this code does
- why the test fails (actually, I still don't get that)
- why the patch would work
- and if the patch is not just preventing symptoms (I still think it is, if imagecolortransparent() can return a rubbish index outside the number of colors, I guess it also may return a rubbish index within the range of valid color indices! and we happily use that index)
And all this, while I have been maintaining imagecache_actions and the D8 image system for a while now, so I'm not a beginner with GD.
As we may want to further refactor this code in the future, some good comments that explains what is really going on can be useful to not (re)introduce issues.
That said, yes it a lot of text,. Just see how far you want to go and what you want to use, I will accept it, even if it is less than I proposed.
Comment #71
cs_shadow CreditAttribution: cs_shadow commented- Sorry, didn't realized earlier that else was not corresponding to the concerned if statement.
- Retained most of the text you proposed, though rephrased it a little, let me know if this works.
Comment #73
fietserwinShould come before the if and with a space after //.
Comment #74
cs_shadow CreditAttribution: cs_shadow commented@fietserwin, I'll just fix that but do you have any idea about the reason for all those exceptions in the last submitted patch?
Comment #75
Pete B CreditAttribution: Pete B commented@cs_shadow: pallet_size is used before it is defined.
Comment #76
cs_shadow CreditAttribution: cs_shadow commentedMy bad... thanks @Pete B. This patch should fix the issue.
Comment #77
cs_shadow CreditAttribution: cs_shadow commentedComment #78
mondrakeMinor things
The new comment should be above
$transparent = imagecolortransparent($this->getResource());
and replace 'Grab transparent ...'. Also, put a full stop at the end of the comment.'truecolor' with no space in between
'Since the index of the transparent color is a property of the image rather than of the palette, it is ....'
Also, we probably have to use consistently a word in both comments and variable names: 'pallet' or 'palette'? I'd rather go for 'palette'. (Yes I know my fault to start with ;))
Comment #79
cs_shadow CreditAttribution: cs_shadow commented1. Moved the new comment above and removed the line
because it sounded redundant due to the new comment.
2. Fixed the space.
3. Replaced all occurences of 'pallet' with 'palette'.
Comment #80
fietserwinRTBC when it turns green.
Comment #81
cs_shadow CreditAttribution: cs_shadow commentedA green patch after a long time. RTBC?
Comment #82
fietserwinComment #83
catchExtremely minor but an extra space after the || that shouldn't be there.
Also minor, no comma after Since.
Comment #84
cs_shadow CreditAttribution: cs_shadow commentedThanks @catch for pointing that out. Fixed the two mistakes.
Attaching patch and interdiff.
Comment #85
cs_shadow CreditAttribution: cs_shadow commentedGo Testbot Go!
Comment #86
fietserwinGreen test and with the changes as per #83, it is RTBC again.
Comment #87
catchCommitted/pushed to 8.x, thanks!
Moving to 7.x for backport.
Comment #89
cs_shadow CreditAttribution: cs_shadow commentedPatch in #84 ported to D7.
Comment #90
ellen.davis CreditAttribution: ellen.davis commentedI was getting the error "Warning: imagecolorsforindex(): Color index 252 out of range in image_gd_create_tmp() (line 310 of /usr/local/apache-sites/htdocs/sites/sg/modules/system/image.gd.inc). " when uploading an image. THe image would upload, but some colors would be lost.
So I tried patch from #89. After I applied the patch, the image no longer uploaded at all. No error was displayed on the drupal website. I found this error in the apache log "PHP Fatal error: Using $this when not in object context in /usr/local/apache-sites/htdocs/sites/sg/modules/system/image.gd.inc on line 312"
Drupal 7.26
PHP 5.4.27
Comment #91
fietserwinimageFactory is a D8 thing: use image_load().
$this->getResource() is D8 syntax, use $image->resource
same.
Comment #92
cs_shadow CreditAttribution: cs_shadow commentedMy bad, forgot to change the D8 syntax. Fixed the issues as suggested in #91.
Comment #93
cs_shadow CreditAttribution: cs_shadow commentedComment #94
ellen.davis CreditAttribution: ellen.davis commentedTesting patch #92. Image does not upload. Getting this error in apache log.
PHP Fatal error: Call to undefined method stdClass::getResource() in /usr/local/apache-sites/htdocs/sites/sg/modules/system/image.gd.inc on line 312
Comment #95
cs_shadow CreditAttribution: cs_shadow commentedLets try this one.
Comment #96
fietserwinThanks for this quick backport.
Comment #97
ellen.davis CreditAttribution: ellen.davis commentedpatch #95 worked great. Thanks.
Comment #99
cs_shadow CreditAttribution: cs_shadow commentedComment #100
cs_shadow CreditAttribution: cs_shadow commented95: drupal-375062-95.patch queued for re-testing.
Comment #101
mgiffordOk, back to RTBC then. Bad bot!
Comment #103
cs_shadow CreditAttribution: cs_shadow commented95: drupal-375062-95.patch queued for re-testing.
Comment #105
David_Rothstein CreditAttribution: David_Rothstein commentedLikely testbot glitch - moving back to RTBC.
Comment #106
halfjapanese CreditAttribution: halfjapanese commented"Warning: imagecolorsforindex(): Color index 252 out of range in image_gd_create_tmp() (line 305 of C:\inetpub\wwwroot\stadtlabor\modules\system\image.gd.inc)."
I came across a similar error message when uploading a GIF, 1.57 MB, 3200 x 2463, Bit depth 8.
I have no idea how to install this patch. Novice here. Some advice would be great.
Comment #110
mondrake#95 still seems OK
Comment #112
David_Rothstein CreditAttribution: David_Rothstein commentedIt's not the fault of this issue, but none of these image GD tests appear to be running in Drupal 7 at all. (It's probably been that way for years.) If you look at the testbot results from the above patch, there is only a single pass for the image GD tests, which is likely this:
Let's try fixing that bug to see if these new tests pass/fail as appropriate.
Otherwise it looks ready to commit, although the wording of the code comment is uncomfortably close to the Stack Overflow answer mentioned above (http://stackoverflow.com/a/3898007). I'm not sure I'd call it plagiarism but it makes me a bit uneasy. We might be nice to at least link to the Stack Overflow answer anyway (in Drupal 8 too) since so much of the fix+understanding came from there?
Comment #113
David_Rothstein CreditAttribution: David_Rothstein commented@halfjapanese:
See https://www.drupal.org/patch/apply
Comment #114
David_Rothstein CreditAttribution: David_Rothstein commentedThe "tests only" patch passed too, so something is wrong. I checked Drupal 8 and the same thing happens there - the test fails to catch the bug (although at the time the patch was originally written in #61 it worked).
I think this needs a more direct test, because it's not really clear why re-loading each generated image (from the previously-run tests) should be expected to trigger this bug in the first place. I am working on a fix.
Comment #115
David_Rothstein CreditAttribution: David_Rothstein commentedLet's try these.
I also changed the code comment to link to the Stack Overflow answer.
Comment #117
mondrake@David_Rothstein re #114
The failure that we were able to capture in #61 was related to the Desaturate operation on the
image-test.gif
file. Apparently, desaturate is reducing the palette and the transparent color index is remaining out. This does not cause problems while the original image is kept in memory, but once flushed to the image file and reloaded, the error message was popping out.The fact that the test is no longer working is due, I think, to the changes introduced by #2244359: Lazy-load GD resource in GDToolkit, and missing to adjust that test. Now the GD resource is no longer loaded when the image is got from the factory. The toolkit waits for any call to
getResource()
to load the resource. The 'WOULD-FAIL-IF-EXISTING-TESTS-WERE-WORKING' fix would be to force that operation after getting the reloaded image, i.e.Comment #118
mondrakeComment #119
David_Rothstein CreditAttribution: David_Rothstein commentedNice find! I went ahead and added that line also (no harm in making it fail in two places, I suppose).
I do think the new image is the most robust way to test this, though, since it's a known quantity that is guaranteed to have this problem; it's not 100% clear why the desaturate option is triggering this (and may even be a bug of its own)?
Comment #121
mondrakeYeah, agree
Yes, we are fixing the effect in this issue, not the cause. As to the why, I'd say the preferred answer at http://stackoverflow.com/questions/8092750/transforming-transparent-gif-to-grayscale-while-saving-transparency quite describes the reason. If we want to address that I suggest to open a separate issue.
RTBC - credit for the followup goes to David_Rothstein
Comment #122
alexpottBack to porting the patch to D7 :)
Committed a1f0ff2 and pushed to 8.0.x. Thanks!
Comment #124
mgiffordChanging version..
Comment #125
mondrakeNew D7 patch, taking cues from latest D8 developments.
Interdiff against #112 which is the last D7 patch.
Comment #127
mondrakeTry again test only.
Comment #129
mondrakePatch in #125 is the one to review, patch in #127 is a test-only one expected to fail.
Comment #130
fietserwinI reviewed the patch and it looks good:
- patches #84 + #119 for D8 ported back correctly.
- test in image manipulations on image being truecolor ported back from D8 (core\modules\system\src\Tests\Image\ToolkitGdTest.php, line 271).
- solving the fact that GD is not tested at all in D7...
So I think we are there.
Comment #134
David_Rothstein CreditAttribution: David_Rothstein commentedComment #135
David_Rothstein CreditAttribution: David_Rothstein commentedCommitted to 7.x - thanks!
Comment #138
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedLinking to an issue that may have been caused by this patch, if anyone familiar with this topic has a chance to look at it.