Skip to content

Commit

Permalink
Fix normalization when the result overflows (#701)
Browse files Browse the repository at this point in the history
Using uint for max values now because uint Max32 can't be represented in IEEE754 single precision float. In calculations it will be promoted to float.

Changed normalization function for single channel images to return max of the data type assuming (x, 0, 0, 0) vectors. And changed the test values accordingly

---------

Co-authored-by: Wasim Abbas <abbas.wasim@gmail.com>
  • Loading branch information
wasimabbas-arm and abbaswasim authored May 17, 2023
1 parent fefd4a6 commit f81330b
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 18 deletions.
10 changes: 5 additions & 5 deletions tests/unittests/image_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ void test_by_channel( const uint32_t min_res[], const uint32_t max_res[],
}

// Hand tested vectors and their normalised results in GeoGebra
static constexpr uint32_t min_res[3]={54, 13849, 907633408}; // These are -0.577350 float in 8bit-UNORM, 16bit-UNORM and 32bit-UNORM values
static constexpr uint32_t min_res[3]={54, 13849, 907633408}; // These are -0.577350 float in 8bit-UNORM, 16bit-UNORM and 32bit-UNORM values
static constexpr uint32_t max_res[3]={201, 51686, 3387333888};

static constexpr uint32_t some_res8[3]={30, 192, 179};
Expand All @@ -120,10 +120,10 @@ static constexpr uint32_t some_res16_2[3]={9541, 9654, 32768};
static constexpr uint32_t some_res32_2[3]={628983424, 628983424, 2147483648};

static constexpr uint32_t min_res1[3]={0, 0, 0};
static constexpr uint32_t max_res1[3]={255, 65535, 0}; // Last is not 4294967295 because of overflow
static constexpr uint32_t some_res8_1[3]={0, 198, 128};
static constexpr uint32_t some_res16_1[3]={0, 32768, 32768};
static constexpr uint32_t some_res32_1[3]={0, 2147483648, 2147483648};
static constexpr uint32_t max_res1[3]={255, 65535, 4294967295};
static constexpr uint32_t some_res8_1[3]={255, 0, 0}; // Second and third values are unused because its single channel image result
static constexpr uint32_t some_res16_1[3]={65535, 0, 0}; // Same as above
static constexpr uint32_t some_res32_1[3]={4294967295, 0, 0}; // Same as above

TEST(NormaliseColorTest, multi_channel_test) {
test_by_channel<4>(min_res, max_res, some_res8, some_res16, some_res32);
Expand Down
26 changes: 13 additions & 13 deletions tools/toktx/image.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ struct vec3_base {
};

static constexpr float gc_m[5]={0.0f, 128.0f, 32768.0f, 0.0f, 2147483648.0f};
static constexpr float gc_s[5]={0.0f, 255.0f, 65535.0f, 0.0f, 4294967295.0f};
static constexpr uint32_t gc_s[5]={0, 255, 65535, 0, 4294967295};

template <typename componentType>
struct vec3 : public vec3_base {
Expand All @@ -199,15 +199,15 @@ struct vec3 : public vec3_base {
if (gc_m[i] == r && gc_m[i] == g && gc_m[i] == b) {
return;
} else {
r = r / gc_s[i] * 2.0f - 1.0f;
g = g / gc_s[i] * 2.0f - 1.0f;
b = b / gc_s[i] * 2.0f - 1.0f;
r = (float)(r / (double)gc_s[i]) * 2.0f - 1.0f;
g = (float)(g / (double)gc_s[i]) * 2.0f - 1.0f;
b = (float)(b / (double)gc_s[i]) * 2.0f - 1.0f;
clamp(-1.0f, 1.0f);
base_normalize();
r = (std::floor((r + 1.0f) * gc_s[i] * 0.5f + 0.5f));
g = (std::floor((g + 1.0f) * gc_s[i] * 0.5f + 0.5f));
b = (std::floor((b + 1.0f) * gc_s[i] * 0.5f + 0.5f));
clamp(0, gc_s[i]);
r = (std::floor((r + 1.0f) * (float)gc_s[i] * 0.5f + 0.5f));
g = (std::floor((g + 1.0f) * (float)gc_s[i] * 0.5f + 0.5f));
b = (std::floor((b + 1.0f) * (float)gc_s[i] * 0.5f + 0.5f));
clamp(0.0f, (float)gc_s[i]);
}
}
};
Expand Down Expand Up @@ -342,7 +342,7 @@ class color<componentType, 2> : public color_base<componentType, 2> {
}
void normalize() {
vec3<componentType> v((float)r, (float)g,
gc_s[sizeof(componentType)] * 0.5f);
(float)gc_s[sizeof(componentType)] * 0.5f);
v.normalize();
r = (componentType)v.r;
g = (componentType)v.g;
Expand Down Expand Up @@ -381,10 +381,10 @@ class color<componentType, 1> : public color_base<componentType, 1> {
return 1;
}
void normalize() {
vec3<componentType> v((float)r, gc_s[sizeof(componentType)] * 0.5f,
gc_s[sizeof(componentType)] * 0.5f);
v.normalize();
r = (componentType)v.r;
// Normalizing single channel image doesn't make much sense
// Here I assume single channel color is (X, 0, 0, 0)
if (r != 0)
r = (componentType)gc_s[sizeof(componentType)];
}
};

Expand Down

0 comments on commit f81330b

Please sign in to comment.