DOI-USGS / DOI-USGS/ISIS3

ISIS Incorrectly Reads Special Pixels For UnsignedWord and UnsignedInteger

Open
#6,113 1 comment 0 reactions 0 assignees View on GitHub
bug
Dominant language
C++
Stars
245
Forks
181
Avg merge
1d 22h
Merged PRs (30d)
15

Description

**ISIS version(s) affected**: 3.7.0+

**Description**
Both UnsignedWord and UnsignedInteger bit types do not appropriately handle values above `VALID_MAXU2` and `VALID_MAXUI4` respectively. In other words, they do not appropriate identify His and Hrs pixels in the assocaited bit depth.

**How to reproduce**
The ISISTESTDATA contains the following cube $ISISTESTDATA/isis/src/base/unitTestData/isisTruth_Unsigned16Bit.cub which has special pixel values for UnsignedWord. Open the cube in qview and you will see two white stripes towards the bottom of the cube. These stripes should read as His and Hrs but read as ~`1.0003968515e+20`.

Image

While this is only UnsignedWord, the UnsignedInteger uses that same logic and produces the same issue.

**Possible Solution**
There are two potential solutions, update the pixel values within `SpecialPixel.h` to reflect the current program logic. Or, update the program logic to match the special pixels in `SpecialPixel.h`.

**Additional context**
When the code was originally written, the logic for translating special pixels was copied from `SignedWord` or `Real`where the special pixels were all negative. So any value greater than the minimum was a special pixel. In the unsigned (Word, and int) the special pixels for His and Hrs were larger than the maximum allowed value, but the logic for pixel translation was kept the same. so the check for His and Hrs is effectively dead:
```C++
if(raw >= VALID_MINU2) {
bufferVal = (double) raw * m_multiplier + m_base;
}
else if (raw > VALID_MAXU2) {
if(raw == HIGH_INSTR_SATU2)
bufferVal = HIGH_INSTR_SAT8;
else if(raw == HIGH_REPR_SATU2)
bufferVal = HIGH_REPR_SAT8;
else
bufferVal = LOW_REPR_SAT8;
}
```
Current UnsignedWord special pixels:
```C++
const unsigned short VALID_MINU2 = ((unsigned short) 3);
const unsigned short NULLU2 = ((unsigned short) 0);
const unsigned short LOW_REPR_SATU2 = ((unsigned short) 1);
const unsigned short LOW_INSTR_SATU2 = ((unsigned short) 2);
const unsigned short HIGH_INSTR_SATU2 = ((unsigned short) 65534);
const unsigned short HIGH_REPR_SATU2 = ((unsigned short) 65535);
const unsigned short VALID_MAXU2 = ((unsigned short) 65522);
```
This is the code for `UnsignedWord`, since `HIGH_INSTR_SATU2` and `HIGH_REPR_SATU2` are greater than `VALID_MINU2` the condition is never entered to check pixel values for His and Hrs.

Contributor guide

Open the contributing guide

Research direction

Start with SpecialPixel.h and the UnsignedWord conversion logic described in the issue, then reproduce the behavior using $ISISTESTDATA/isis/src/base/unitTestData/isisTruth_Unsigned16Bit.cub in qview. Verify that the UnsignedWord and UnsignedInteger paths identify the high instrument and high representation saturation pixels as His and Hrs rather than numeric values.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
computer-vision
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.