0

I have a scaling algorithm which appears to scale the image to the right size, but produces artefacts(slight image corruption) in the right half of the image. As I am inexperienced using pointers, I suspect I may have blundered with my pointer arithmetic!

To run the project on OSX: 1. Download from :https://www.dropbox.com/s/myme1z1mkxjwyjf/artifact.zip?dl=0

  1. Open the xcodeproj file found in proj.ios

  2. All code that is of relevance, is in HelloWorldScene.cpp

In function test(), you can comment out / uncomment the method we wish to test.:

void HelloWorld::test(){
    testCopy(); //In this case the image appears as expected. (a simple copy)
//    testScale(); //In this case there are strange artifacts on the right tip of the arrow.
}

Test copy, is my attempt at just copying the contents of the buffer without doing anything bad like memory corruption, leaking etc... The image appears on the screen looking ok!

void HelloWorld::testCopy(){
std::string infile = _imageName;

Image* img = new Image();
img->initWithImageFile(infile);
auto odata = img->getData();

Image* copy = new Image();

int components = 4;
auto finalDataLen = img->getDataLen();
auto finalData = static_cast<unsigned char*>(malloc(finalDataLen));

for (int i = 0; i<img->getWidth(); i++) {
    for (int j = 0; j<img->getHeight(); j++) {
        unsigned char *pixel = odata + (i + j * img->getWidth()) * components;

        unsigned char *fpixel = finalData + (i + j * img->getWidth()) * components;
        fpixel[0] = pixel[0];
        fpixel[1] = pixel[1];
        fpixel[2] = pixel[2];
        fpixel[3] = pixel[3];
    }
}
copy->initWithRawData(finalData, finalDataLen, img->getWidth(), img->getHeight(), 8);
Texture2D* tk = new Texture2D();
tk->initWithImage(copy);

Sprite* foo = Sprite::createWithTexture(tk);
foo->setPosition(Director::getInstance()->getVisibleSize().width/2,Director::getInstance()->getVisibleSize().height/2);
foo->setScale(0.8);
this->addChild(foo);

delete img;
delete copy;
return;

}

Now comment out testCopy(); and uncomment testScale(); In this case the image appears but with some corruption the right side of the image!

void HelloWorld::testScale(){
    std::string infile = _imageName;

    Image* img = new Image();
    img->initWithImageFile(infile);
    Image* scl = new Image();
    scaleImage(img, scl, 0.8);

    Texture2D* tk = new Texture2D(); //Texture is needed as long as the sprite exists, so we aren't deleting it.
    tk->initWithImage(scl);
    Sprite* foo = Sprite::createWithTexture(tk);
    foo->setPosition(Director::getInstance()->getVisibleSize().width/2,Director::getInstance()->getVisibleSize().height/2);
    this->addChild(foo);

    delete img;
    delete scl;

    return;
}

void HelloWorld::scaleImage(Image* original,Image* scaledImage,const float& scale){
    int width = scale*original->getWidth();
    int height = scale*original->getHeight();

    int x=4;

    unsigned char* data = original->getData();

    auto dataLen = width * height * x * sizeof(unsigned char);
    auto data2 = static_cast<unsigned char*>(malloc(dataLen));

    //sprshrink seems to be the problem method.
    sprshrink(data2, width, height, data, original->getWidth(), original->getHeight());
    scaledImage->initWithRawData(data2, dataLen, width, height, 8);
}

//Why does this method produce artifcats ?
void HelloWorld::sprshrink(unsigned char *dest, int dwidth, int dheight, unsigned char *src, int swidth, int sheight){
    int x, y;
    int i, ii;
    float red, green, blue, alpha;
    float xfrag, yfrag, xfrag2, yfrag2;
    float xt, yt, dx, dy;
    int xi, yi;


    dx = ((float)swidth)/dwidth;
    dy = ((float)sheight)/dheight;

    for(yt= 0, y=0;y<dheight;y++, yt += dy)
    {
        yfrag = (float) ceil(yt) - yt;
        if(yfrag == 0)
            yfrag = 1;
        yfrag2 = yt+dy - (float) floor(yt + dy);
        if(yfrag2 == 0 && dy != 1.0f)
            yfrag2 = 1;

        for(xt = 0, x=0;x<dwidth;x++, xt+= dx)
        {
            xi = (int) xt;
            yi = (int) yt;
            xfrag = (float) ceil(xt) - xt;
            if(xfrag == 0)
                xfrag = 1;
            xfrag2 = xt+dx - (float) floor(xt+dx);
            if(xfrag2 == 0 && dx != 1.0f)
                xfrag2 = 1;
            red = xfrag * yfrag * src[(yi*swidth+xi)*4];
            green =  xfrag * yfrag * src[(yi*swidth+xi)*4+1];
            blue =   xfrag * yfrag * src[(yi*swidth+xi)*4+2];
            alpha =  xfrag * yfrag * src[(yi*swidth+xi)*4+3];

            for(i=0; xi + i + 1 < xt+dx-1; i++)
            {
                red += yfrag * src[(yi*swidth+xi+i+1)*4];
                green += yfrag * src[(yi*swidth+xi+i+1)*4+1];
                blue += yfrag * src[(yi*swidth+xi+i+1)*4+2];
                alpha += yfrag * src[(yi*swidth+xi+i+1)*4+3];
            }

            red += xfrag2 * yfrag * src[(yi*swidth+xi+i+1)*4];
            green += xfrag2 * yfrag * src[(yi*swidth+xi+i+1)*4+1];
            blue += xfrag2 * yfrag * src[(yi*swidth+xi+i+1)*4+2];
            alpha += xfrag2 * yfrag * src[(yi*swidth+xi+i+1)*4+3];
            for(i=0; yi+i+1 < yt +dy-1 && yi + i+1 < sheight;i++)
            {
                red += xfrag * src[((yi+i+1)*swidth+xi)*4];
                green += xfrag * src[((yi+i+1)*swidth+xi)*4+1];
                blue += xfrag * src[((yi+i+1)*swidth+xi)*4+2];
                alpha += xfrag * src[((yi+i+1)*swidth+xi)*4+3];

                for (ii = 0; xi + ii + 1 < xt + dx - 1 && xi + ii + 1 < swidth; ii++)
                {
                    red += src[((yi+i+1)*swidth+xi+ii+1)*4];
                    green += src[((yi+i+1)*swidth+xi+ii+1)*4+1];
                    blue += src[((yi+i+1)*swidth+xi+ii+1)*4+2];
                    alpha += src[((yi+i+1)*swidth+xi+ii+1)*4+3];
                }

                red += xfrag2 * src[((yi+i+1)*swidth+xi+ii+1)*4];
                green += xfrag2 * src[((yi+i+1)*swidth+xi+ii+1)*4+1];
                blue += xfrag2 * src[((yi+i+1)*swidth+xi+ii+1)*4+2];
                alpha += xfrag2 * src[((yi+i+1)*swidth+xi+ii+1)*4+3];
            }

            if (yi + i + 1 < sheight)
            {
                red += xfrag * yfrag2 * src[((yi + i + 1)*swidth + xi) * 4];
                green += xfrag * yfrag2 * src[((yi + i + 1)*swidth + xi) * 4 + 1];
                blue += xfrag * yfrag2 * src[((yi + i + 1)*swidth + xi) * 4 + 2];
                alpha += xfrag * yfrag2 * src[((yi + i + 1)*swidth + xi) * 4 + 3];

                for (ii = 0; xi + ii + 1 < xt + dx - 1 && xi + ii + 1 < swidth; ii++)
                {
                    red += yfrag2 * src[((yi + i + 1)*swidth + xi + ii + 1) * 4];
                    green += yfrag2 * src[((yi + i + 1)*swidth + xi + ii + 1) * 4 + 1];
                    blue += yfrag2 * src[((yi + i + 1)*swidth + xi + ii + 1) * 4 + 2];
                    alpha += yfrag2 * src[((yi + i + 1)*swidth + xi + ii + 1) * 4 + 3];
                }
            }

            if (yi + i + 1 < sheight && x + xi + 1 < swidth)
            {
                red += xfrag2 * yfrag2 * src[((yi + i + 1)*swidth + xi + ii + 1) * 4];
                green += xfrag2 * yfrag2 * src[((yi + i + 1)*swidth + xi + ii + 1) * 4 + 1];
                blue += xfrag2 * yfrag2 * src[((yi + i + 1)*swidth + xi + ii + 1) * 4 + 2];
                alpha += xfrag2 * yfrag2 * src[((yi + i + 1)*swidth + xi + ii + 1) * 4 + 3];
            }
            red /= dx * dy;
            green /= dx * dy;
            blue /= dx * dy;
            alpha /= dx * dy;
            red = clamp(red, 0, 255);
            green = clamp(green, 0, 255);
            blue = clamp(blue, 0, 255);
            alpha = clamp(alpha, 0, 255);

            dest[(y*dwidth+x)*4] = (unsigned char) red;
            dest[(y*dwidth+x)*4+1] = (unsigned char) green;
            dest[(y*dwidth+x)*4+2] = (unsigned char) blue;
            dest[(y*dwidth+x)*4+3] = (unsigned char) alpha;
        }
    }
}

I suspect my downscaling algorithm (sprshrink) works (because it is someone elses! :D), and suspect that I am blundering with my usage of pointers in testScale()! What do you think ? Am I allocating and using my pointers properly? What am I doing wrong?

Images:

Clear:

Clear picture

Artefacts when running testScale() instead of testCopy() (comment out testCopy). Artifcats after running the scaling method!

Rahul Iyer
  • 19,924
  • 21
  • 96
  • 190
  • 1
    What about a screenshot of the artifact so that we are talking about the same thing. *"I cannot use smart pointers because Texture2D requires the raw pointer!"* - I don't see how access to a raw pointer at some point inhibits use of a smart pointer. – IInspectable Oct 12 '16 at 15:44
  • @IInspectable As I mentioned earlier, I am inexperienced with using pointers, so perhaps I just don't understand how to do that. Righto! Pictures coming up.... but you really can't miss it if you run both methods one at a time... I will add pics..... – Rahul Iyer Oct 12 '16 at 15:49
  • 1
    You are expecting users to run Mac OS X, just to see the effect. That certainly limits the potential audience, as well as the usefulness of the question itself. – IInspectable Oct 12 '16 at 15:53
  • @IInspectable unfortunately this is the best I can do, as I am coding on a mac! Hopefully some day I will be a better programmer and ask the perfect question for the biggest possible audience.... :) – Rahul Iyer Oct 12 '16 at 15:57
  • 1
    Probably related: [Is floating point math broken?](http://stackoverflow.com/q/588004/1889329) – IInspectable Oct 12 '16 at 16:07
  • @IInspectable omg I so cwazy... but if that is the case it should show in memory ? Does that mean my blundering was not with my pointer allocation deallocation !? – Rahul Iyer Oct 12 '16 at 16:11
  • 1
    I agree with @IInspectable, the problem is with all the floating point going on in `sprshrink`. Every once in a while, roundoff errors might result in skipping over a destination pixel. It's better to do pixel address math as integers because it's more predictable. – Mark Ransom Oct 12 '16 at 16:19
  • @MarkRansom jajajaj I guess back to the drawing board!! But just curious ? Am I actually using pointers correct ? I'm totes faking it bros.... never had a lesson... – Rahul Iyer Oct 12 '16 at 16:22
  • 1
    I haven't actually looked at the entire wall of code, but I don't see any pointer problems in the bits that I did look at. – Mark Ransom Oct 12 '16 at 16:28
  • @MarkRansom Just in testScale, scaleImage and testCopy ? I mean its really fundamental, but I have no one I ask whether I'm doing it correctly. Can you tell me? They're really small functions and it would totes make my day! – Rahul Iyer Oct 12 '16 at 16:30
  • @IInspectable Sir - would you say my pointerquette in testCopy and testScale are kosher ? – Rahul Iyer Oct 12 '16 at 16:38
  • 1
    Again, nothing wrong with the bits I see, but I have two suggestions. First, become familiar with `std::unique_ptr` because it saves you from having to remember to `delete`. Second, don't even use pointers here! Instead of `Image* img = new Image();` you can simply do `Image img;`. When you need a pointer to pass to a function, just use `&img`. – Mark Ransom Oct 12 '16 at 16:38
  • @MarkRansom You are right sir. I hate pointers like the black plague! But Texture2D->initWithImage() takes an Image* as input. Doesn't that mean I can't use std::unique_ptr ? (I haven't used smart pointers before, so perhaps I just don't understand) – Rahul Iyer Oct 12 '16 at 16:41
  • 1
    [`std::unique_ptr`](http://en.cppreference.com/w/cpp/memory/unique_ptr) has a `get()` member function to return the pointer to the managed object. In case you need to transfer ownership, use `release()` instead. And smart pointers aren't just a convenience here, too. They are **mandatory** to make the code exception safe. – IInspectable Oct 12 '16 at 16:52
  • @IInspectable Thanks you sir! I shall now read up about std::unique_ptr and try to totes amaze balls into my app! I also did not know that about exceptions... But again just out of curiosity ? did i make any coding blunders (leak / corruption) in my test methods ? I have no experienced programmer to tell me if i did totes amaze balls or not... – Rahul Iyer Oct 12 '16 at 16:59
  • 1
    I haven't looked at the code. If you want to find out, run some static code analysis tools over it. Or run the application under a memory tracker (like [Valgrind](http://valgrind.org/)). Or simply make your code safer by using the [GSL](https://github.com/Microsoft/GSL), in particular the [`span`](https://github.com/Microsoft/GSL/blob/master/gsl/span) type. – IInspectable Oct 12 '16 at 17:06

0 Answers0