[asimage] fix bug in rainbow palette selection

[linev]

Predefined rainbow palette has only 10 points, but 12 were copied - overwriting memory outside allocated region.

Failure can be observed with tutorials/visualisation/image/galaxy_image.C trying to select "Rainbow" palette in combobox.

@ferdymercury
Code in the TASPaletteEditor::NewPalette() method must be reviewed.
I see a lot of arithmetic like 0xffff * (pt - 1) / (numPt - 3); which leads to overflow of last calculated point.

[github-actions]

Test Results

    23 files      23 suites   3d 18h 59m 7s ⏱️
 3 889 tests  3 887 ✅ 0 💤 2 ❌
80 373 runs  80 371 ✅ 0 💤 2 ❌

For more details on these failures, see this check.

Results for commit 3427f9c.

♻️ This comment has been updated with latest results.

[ferdymercury]

all palettes seem to start at [1], see for example

      case 1:  // gray
         for (pt = 1; pt < Int_t(numPt - 1); pt++) {
         
         

Would it make more sense to instead copy one element less but still start at 1, for consistency?

Or to make the others below also start at 0?

[linev]

@ferdymercury

Thanks for remark!
Complete arithmetic behind palette initialization here is wired.
And in fact first and last point in palette are duplication of second and N-1 points.
I will update PR

[silverweed]

instead of manually coding this number, which is error-prone, can we either use std::size or std::array and then .size()?
Or, at the very least, a defined constant kNRainbowElems = 10

[linev]

It is immediate bugfix of ROOT crash which I want backport to 6.40 as well.

I see many other problems here and code need to be reorganized.
See my comments to the PR description.
This can be done as separate PR then.

[silverweed]

It's not a big change at all to introduce...

[ferdymercury]

I see a lot of arithmetic like 0xffff * (pt - 1) / (numPt - 3); which leads to overflow of last calculated point.

For kGray palette, the loop end condition is pt < numPt - 1, so last calculated point is pt = numPt - 2, which leads to

0xffff * (numPt - 2 - 1) / (numPt - 3) = 0xffff

so the TPalette for gray has 13 elements, first and last elements are reserved, second is 0 and second-last is 65535, and the rest is linearly increasing inbetween

Where did you see the overflow issue?

[linev]

For kGray palette, the loop end condition is pt < numPt - 1, so last calculated point is pt = numPt - 2, which leads to
0xffff * (numPt - 2 - 1) / (numPt - 3) = 0xffff

There are too many -1 -2 -3 which I miss-interpret. Maybe one can do it more simpler.

[ferdymercury]

on line 680 change to:

numPt = (id == 0 ? kNRainbowElems : 11) + 2;