ermig1979 / Simd

C++ image processing and machine learning library with using of SIMD: SSE, AVX, AVX-512, AMX for x86/x64, VMX(Altivec) and VSX(Power7) for PowerPC, NEON for ARM.
http://ermig1979.github.io/Simd
MIT License
2.01k stars 407 forks source link

Simd::Resize confusing results #253

Closed jay3d closed 5 months ago

jay3d commented 1 year ago

I have written a mipmap generation code based on Simd::Resize function. It's fast and nice when it works well... But it's randomly performing some kind of an image data-rotation on the resized/copied target as it is shown in the images.

Here's the function code:

[[nodiscard]] auto GenerateMipmaps()
{
    // Creating a Vector2 structure from the dimensions of the current image
    // ensuring that they're of the size_t type
    Math::Vector2< size_t > imageSize{ size_t( State.Image.Rectangle.Size.X ),
                                       size_t( State.Image.Rectangle.Size.Y ) };
    // Structure to hold details of the Mipmaps generated, including their size, offset, data size, and data
    struct MipInfo
    {
        Math::Vector2< size_t >          Size;
        size_t                           Offset{};
        size_t                           DataSize{};
        Containers::ArrayView< uint8_t > Data;
    };
    // Creating a vector to hold all the generated MipInfo structures
    std::vector< MipInfo > mipmaps;
    // We're calculating the total texture storage size upfront to allocate the necessary memory as a whole,
    // meanwhile we're storing each individual mip view's parameters in the mipmaps vector
    Containers::Array< uint8_t > texture{ NoInit,
                                          [ &imageSize, &mipmaps ]
                                          {
                                              size_t textureStorageSize{ 0 };
                                              auto mipSize{ imageSize };
                                              while ( mipSize.x() > 0 && mipSize.y() > 0 )
                                              {
                                                  // The size of the data in bytes for each mipmap level
                                                  const auto dataSize{ mipSize.product() * 4 };
                                                  // Pushing each mip's parameters into the mipmaps vector
                                                  mipmaps.push_back( { mipSize, textureStorageSize, dataSize } );
                                                  textureStorageSize += dataSize;
                                                  mipSize /= 2;
                                              }
                                              // Returning the total storage size
                                              return textureStorageSize;
                                          }() };
    // Now that we have recorded the mip parameters
    // we can create the mip views by slicing the allocated texture memory
    for ( auto&& mip : mipmaps )
    {
        mip.Data = texture.slice( mip.Offset, mip.Offset + mip.DataSize );
    }
    // Acquiring the top level mip, which is essentially the original image
    const auto& srcMip = mipmaps[ 0 ];
    using SimdView_t = Simd::View< Simd::Allocator >;
    // This step wraps the original image data, but currently, it might not be aligned
    // (or would Simd::View handle alignment? Requires verification)
    auto org =
        SimdView_t{ srcMip.Size.x(), srcMip.Size.y(), SimdView_t::Format::Rgba32, ( uint8_t* )State.Image.Data };
    auto src = SimdView_t{ srcMip.Size.x(), srcMip.Size.y(), SimdView_t::Format::Rgba32 };
    // Conducting an aligned copy with the intention of ensuring compatibility with Simd::Resize
    Simd::Copy( org, src );
    for ( auto& dstMip : mipmaps )
    {
        // Creating an empty view here, with the goal that Simd::Resize will use it to store the newly sized image
        auto dstTmp = SimdView_t{};
        Simd::Resize( src, dstTmp, { dstMip.Size.x(), dstMip.Size.y() }, SimdResizeMethodArea );
        // Conducting an un-aligned copy here based on the size of the destination mip rather than the size of the
        // view to eliminate any row padding that might occur due to alignment, effectively clipping the image to
        // valid contents
        Utility::copy( Containers::arrayView( dstTmp.data, dstMip.DataSize ), dstMip.Data );
    }
    // Returning the texture after performing all the operations
    return texture;
}

image image

Notice the slight shift of the image (the repeating image parts are coming from the image itself and that's the issue, there's no repetition parameters in the GPU texture)

ermig1979 commented 1 year ago

Hello! I can't tell the exact reason of this behaviour, but it seems to using of integers for determination of output rectangle coordinates.

jay3d commented 1 year ago

I have question: Is it proper to have a Simd::View around unaligned image data array? How would that behave? If you notice this line:

    // This step wraps the original image data, but currently, it might not be aligned
    // (or would Simd::View handle alignment? Requires verification)
    auto org =
        SimdView_t{ srcMip.Size.x(), srcMip.Size.y(), SimdView_t::Format::Rgba32, ( uint8_t* )State.Image.Data };
    auto src = SimdView_t{ srcMip.Size.x(), srcMip.Size.y(), SimdView_t::Format::Rgba32 };
    // Conducting an aligned copy with the intention of ensuring compatibility with Simd::Resize
    Simd::Copy( org, src );

org is a Simd::View around image data array, and I thought I made Simd copy that to another view copy, maybe it's fixing the alignement. Was that necessary?

ermig1979 commented 1 year ago

Oh, I'h understood the trouble. You use View constructor:

    /*!
        Creates a new View structure with specified width, height, pixel format, pointer to pixel data and memory alignment.

        \param [in] w - a width of created image view.
        \param [in] h - a height of created image view.
        \param [in] f - a pixel format of created image view.
        \param [in] d - a pointer to the external buffer with pixel data. If this pointer is NULL then will be created own buffer.
        \param [in] align - a required memory alignment. Its default value is determined by function Allocator::Alignment.
    */
    View(size_t w, size_t h, Format f, void * d = NULL, size_t align = Allocator::Alignment());

but I would recommend the following:

    /*!
        Creates a new View structure with specified width, height, row size, pixel format and pointer to pixel data.

        \param [in] w - a width of created image view.
        \param [in] h - a height of created image view.
        \param [in] s - a stride (row size) of created image view.
        \param [in] f - a pixel format of created image view.
        \param [in] d - a pointer to the external buffer with pixel data. If this pointer is NULL then will be created own buffer.
    */
    View(size_t w, size_t h, ptrdiff_t s, Format f, void * d);

You can set exact value of row size (stride) and avoid of mentioned above troubles.

jay3d commented 1 year ago
        using SimdView_t = Simd::View< Simd::Allocator >;

        // This step wraps the original image data
        auto src =
            SimdView_t{ srcMip.Size.x(),
                        srcMip.Size.y(),
                        ptrdiff_t( ( ( srcMip.Size.x() * pixelSize + Simd::Alignment() - 1 ) / Simd::Alignment() ) *
                                   Simd::Alignment() ),
                        SimdView_t::Format::Rgba32,
                        imageView };

        for ( auto& dstMip : mipmaps )
        {
            // Creating an empty view here, with the goal that Simd::Resize will use it to store the newly sized image
            auto dst =
                SimdView_t{ dstMip.Size.x(),
                            dstMip.Size.y(),
                            ptrdiff_t( ( ( dstMip.Size.x() * pixelSize + Simd::Alignment() - 1 ) / Simd::Alignment() ) *
                                       Simd::Alignment() ),
                            SimdView_t::Format::Rgba32,
                            nullptr };

            Simd::Resize( src, dst, SimdResizeMethodArea );

            // Conducting an un-aligned copy here based on the size of the destination mip rather than the size of the
            // view to eliminate any row padding that might occur due to alignment, effectively clipping the image to
            // valid contents
            Utility::copy( Containers::arrayView( dst.data, dstMip.DataSize ), dstMip.Data );
        }

The above seems to work, I'm posting here to verify correctness