Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix dest ROI #104

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Fix dest ROI #104

wants to merge 5 commits into from

Conversation

hhubschle
Copy link

The oSizeROI needs to be the size of the dest image,

@kunzmi
Copy link
Owner

kunzmi commented Aug 9, 2021

Hi,

thanks for reporting and fixing!
The *Border* filter functions gave me already so many headaches ;-)
It took me now a while to understand what is all wrong, because when Nvidia introduced the *Border* filter functions in Cuda 6.0 I tested them all out with the Pre-Release version of CUDA. And I got them to work with the settings I put in the code... well not all, because some had bugs that I reported to Nvidia.

Apparently, Nvidia then changed the API from the pre-release to the release version of Cuda 6 so that one had to provide the pointer to the first ROI-pixel and not the pixel (0,0) (these days also stated in the docs). As it seems this was wrong in managedCuda for now 6 years and I was never aware of that change :(

But, these *Border* functions continue to give me pain, because as my tests now showed, the size provided must be the size of the entire image, not the size of the ROI, which is in contrast to the documentation. On the other hand it wouldn't make sense to provide the size of the ROI, as the information of the first ROI-pixel is of no help if one doesn't know the entire image size... (and the ROI size is already given by the destination image).

I'll have to investigate more on that to make sure to get it right this time ;)

Cheers,
Michael

@kunzmi
Copy link
Owner

kunzmi commented Aug 17, 2021

Ok, I did more testing and found my logical error: The *Border-filter functions actually have two ROIs that I messed up.
The first ROI is the same ROI as always and can be set with the method SetRoi() as usual. The second ROI is passed to NPP with oSrcOffset and oSrcSize. This second ROI is important as it tells NPP where the filter is allowed to read pixel values and where to start with the border treatment.
Imagining the following case: An image of size 512x512 and a border of 2 pixels with invalid data. In case we set a ROI to (2,2/508,508), NPP should never read these pixels for any filter. But if we set a ROI to (10,10/100,100), a filter of size 3x3 will never access the invalid pixels and can safely use pixels outside the ROI to do the filtering. This is where the second ROI is needed, to tell NPP where the no-go-area starts.

My solution so far is something like this, with an additional optional parameter for the second ROI:

public void FilterGaussBorder(NPPImage_8uC4 dest, MaskSize eMaskSize, NppiBorderType eBorderType, NppiRect filterArea = new NppiRect())
{
    if (filterArea.Size == new NppiSize())
    {
        filterArea.Size = _sizeRoi;
    }
    status = NPPNativeMethods.NPPi.FilterGaussBorder.nppiFilterGaussBorder_8u_C4R(_devPtrRoi, _pitch, filterArea.Size, filterArea.Location, dest.DevicePointerRoi, dest.Pitch, dest.SizeRoi, eMaskSize, eBorderType);
    Debug.WriteLine(String.Format("{0:G}, {1}: {2}", DateTime.Now, "nppiFilterGaussBorder_8u_C4R", status));
    NPPException.CheckNppStatus(status, this);
}

For reducing filtering to the actual ROI and disallow any read from outside pixels, oSrcOffset must be (0, 0) and the ROI set as usual.
Omitting the optional parameter, one can use the ROIs correctly and reduce the filter area to the ROI or one can set advanced settings with the second ROI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants