OpenChemistry / tomviz

Cross platform, open source application for the processing, visualization, and analysis of 3D tomography data
https://tomviz.org/
BSD 3-Clause "New" or "Revised" License
316 stars 84 forks source link

Weird colormap output for Linear green (Gr4L) #1367

Closed ercius closed 6 years ago

ercius commented 6 years ago

tomviz 1.2.1, windows 10.

I think that the Linear Green (Gr4L) color map is incorrect. It briefly goes brownish on the lower end of the histogram. You can also see this error (?) in the preset window. See screenshots below:

cquammen commented 6 years ago

I cannot replicate on my Mac in my development build. I don't have a good explanation for why you are seeing the blue and brown.

cryos commented 6 years ago

I don't see this on Linux, we should look at the 1.2.1 binaries when we are back in the office tomorrow.

ercius commented 6 years ago

I replicated it on another windows machine. So, its not specific to my computer.

cryos commented 6 years ago

@cjh1 is there any chance you could give this a try on your Windows 10 machine? I think just trying the 1.2.1 binary would be great, then we may want to take a look at a ParaView binary to see if the same issue presents there.

cjh1 commented 6 years ago

@cryos Sure, I will take a look.

cjh1 commented 6 years ago

As you can see I am seeing the same on my window 10 box:

screen

I will check ParaView as well ....

cryos commented 6 years ago

If you look in the Tomviz directory I think we actually bundle the ParaView binary in there too, so you can just take a quick look using that I think.

cjh1 commented 6 years ago

Looks like ParaView doesn't have this problem:

screen

cryos commented 6 years ago

That is unexpected, the ParaView widget in Peter's screenshot for picking the color map has the splodge in it too, so I had expected to see the issue in ParaView. Is this the Tomviz provided binary, or a ParaView upstream binary you are testing with? I assume there is no splodge in the color map picker in ParaView either. Very strange to see it in the color bar, the histogram, and the picker but not in PV.

cjh1 commented 6 years ago

That is with the ParaView executable shipped with tomviz.

cryos commented 6 years ago

That is so strange, thanks for the confirmation.

cryos commented 6 years ago

Documenting that we looked at this a little more on Chris' windows 10 machine using the 1.2.1 binary. It seemed to happen most when looking at the color map chooser before loading data, but there seemed to be an element of randomness to it. We only observe the issue with this color map, and once it appears it stays until the application is restarted.

Looking at the color map the actual color points appear to have reasonable values, but the interpolation between them is off. We will have to try and get a debug build to dig deeper into this, it appears to be an initialization issue or perhaps something writing to a memory location it should not.

It is unclear whether it would also randomly happen in ParaView, it is harder to get to the color map, you must load some data first. We should probably disable the button until data is loaded too, as there is nothing useful to be done. I will see if we can find some more time to look into this soon.

cquammen commented 6 years ago

Utkarsh seems to have found the likely culprit in ParaView. See the discussion in https://gitlab.kitware.com/paraview/paraview/issues/17933.

There is a bad color space in the JSON defining the Linear Green (Gr4L) color map (CIELAB instead of Lab) and uninitialized memory in the JSON processing code.

cjh1 commented 6 years ago

@utkarshayachit Good catch!

cryos commented 6 years ago

That is great, I had been meaning to circle back to this, but had no solid plan of attack. Thanks for letting us know.

ercius commented 6 years ago

Mystery solved!

On Wed, Feb 7, 2018, 8:23 AM Marcus D. Hanwell notifications@github.com wrote:

That is great, I had been meaning to circle back to this, but had no solid plan of attack. Thanks for letting us know.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/OpenChemistry/tomviz/issues/1367#issuecomment-363824095, or mute the thread https://github.com/notifications/unsubscribe-auth/AO7olN0PhbLtkZnylFiweleIPp237Vvtks5tSc4BgaJpZM4Q-SLd .

cryos commented 6 years ago

Resolved - PV bump will fix this.