Open jsenn opened 2 years ago
After digging into this a bit more, I think the actual problem isn't the lack of a static_cast, but actually just a bug in a part of the code unrelated to my example above. If you look at the warning stack, the root of the problem is in compute_tensor_entries, a function in numeric.hpp that's only used in the harris corner example and its associated test.
This code appears to be multiplying 2 16-bit shorts (producing a 32-bit integer), then assigning it to a 32-bit float grayscale pixel. Does this make sense? Shouldn't it be doing some sort of rescaling to make sure the value is between 0 and 1?
@jsenn IIRC the values from Harris corner detector are already small. Either way, the PR from GSoC of 2021 #624 should make it clear that the image used for computations are just treated as matrix of floating point values. There is no implied or required normalization. I'm in a bit of a roaming state atm, but should be able to get the PR in merge-ready state until the end of summer. For now, if you have the time, you could try the example from there and see how it looks like. Either way if silencing of warnings is needed, it might be better to make them under separate name, as some people could use warnings as a source of real problems.
Thanks for the quick response @simmplecoder. Is there anything in #624 that would solve the issue with compute_tensor_entries
? The issue I see is that the output matrices use boost::gil::float32_t
channel values, which are bounded to the interval [0, 1] (because float32_t
is defined as a scoped_channel_value
), but that function is assigning values from the partial derivative images which are given as shorts in the range [0, 65535]. Unless all the derivative values happen to be either 0 or 1, the output matrices will be "overflowed" in the sense that their imposed bounds have been violated. If the input values are greater than 2^12, then their product will be greater than 2^24, at which point there is precision loss from converting an int to a float (which is what the compiler warning is about).
It seems to me (correct me if I'm wrong) that either:
float32_t
image;compute_tensor_entries
should use a 32-bit integer as the output channel type to avoid precision loss; orfloat32_t
type).
Actual behavior
I get the following compiler warnings from pixel.hpp when using some gil functions:
This is in the grayscale pixel assign function, where it's assigning the int channel value directly to the float grayscale value without an explicit cast. Since not every int has a representation as a float, this is a loss of data, hence the warning. I guess the fix would be to add an explicit
static_cast<>
in there like the following?Expected behavior
No warning
C++ Minimal Working Example
Environment
Compiled in Visual Studio 2019 using MSVC with language standard set to latest (C++20).