openframeworks / openFrameworks

openFrameworks is a community-developed cross platform toolkit for creative coding in C++.
http://openframeworks.cc
Other
9.97k stars 2.56k forks source link

ofPixels - artifacts / crashes with .resize() #6226

Open johanjohan opened 5 years ago

johanjohan commented 5 years ago

Since ofPixels.cpp has not been updated for over a year in the master branch, i therefore assume the following issue has not been fixed. i am using of 0.10.1 with win10 vs2017.

I am resizing images/pixels with the following function. That leads to issues:

calling the same function using image.resize() produces best results

question: i assume that i may use pixelsRes = _pixels; to create a full twin copy, right? if yes, then there is the described issue in the pixel resizeTo() function.

CODE:

#include "ofApp.h"

/*
    this function aims to either
        - crop the image to the largest possible dimensions respecting _aspect 
            then scales the longest side to _longestSideLengthPx
        - stretch it respecting _aspect
            then scales the longest side to _longestSideLengthPx
*/

static void resize(
    ofPixels     &_pixels,
    const float  &_longestSideLengthPx,
    const ofVec2f    &_aspect,
    const bool   &_bStretch, // otherwise crop
    const ofInterpolationMethod &_interp    // OF_INTERPOLATE_NEAREST_NEIGHBOR OF_INTERPOLATE_BILINEAR
) {
    ofScaleMode _scaleMode = _bStretch ? OF_SCALEMODE_FILL : OF_SCALEMODE_FIT;
    ofRectangle rectPixels(0, 0, _pixels.getWidth(), _pixels.getHeight());
    ofRectangle rectAspect(0, 0, _aspect.x, _aspect.y);
    rectAspect.scaleTo(rectPixels, _scaleMode);

    ofPixels pixelsRes;
    if (_scaleMode == OF_SCALEMODE_FIT) { // crop
        _pixels.cropTo(pixelsRes, rectAspect.x, rectAspect.y, rectAspect.width, rectAspect.height);
    }
    else { // stretch
        pixelsRes = _pixels;    

        // Q: is this a complete copy with new identical pixels?

        // looks like it does copy all params plus the pixels:
        //template<typename PixelType>
        //void ofPixels_<PixelType>::copyFrom(const ofPixels_<PixelType> & mom) {
        //  if (mom.isAllocated()) {
        //      allocate(mom.getWidth(), mom.getHeight(), mom.getPixelFormat());
        //      memcpy(pixels, mom.getData(), getTotalBytes());
        //  }
        //}
    }

    ofRectangle rectSize;
    if (rectAspect.width > rectAspect.height) {
        assert(rectAspect.width > 0);
        rectSize.set(0, 0, _longestSideLengthPx, _longestSideLengthPx / rectAspect.width * rectAspect.height);
    }
    else { 
        assert(rectAspect.height > 0);
        rectSize.set(0, 0, _longestSideLengthPx / rectAspect.height * rectAspect.width, _longestSideLengthPx);
    }

    ofLogNotice(__FUNCTION__) << "scaling _pixels: rectSize: " << rectSize;
    ofLogNotice(__FUNCTION__) << "_aspect   : " << _aspect;
    ofLogNotice(__FUNCTION__) << "_scaleMode: " << _scaleMode;
    ofLogNotice(__FUNCTION__) << "_interp   : " << _interp;
    ofLogNotice(__FUNCTION__) << "rectPixels: " << rectPixels;
    ofLogNotice(__FUNCTION__) << "rectAspect: " << rectAspect;

#if 1
    // ISSUE is right here

    // OF_INTERPOLATE_NEAREST_NEIGHBOR crashes on large sizes
    // OF_INTERPOLATE_BILINEAR : produces grey only pixels.well yes, "not implemented yet" in ofPixels
    // OF_INTERPOLATE_BICUBIC produces some  singular green pixel / blue artifacts
    // calling above function via image.resize() produces best results
    bool b = pixelsRes.resize(rectSize.width, rectSize.height, _interp); // THIS SEEMS BUGGY  in 0.10.1
    assert(b);
    _pixels = pixelsRes;
#else
    // all ok - done with freeimage functions
    ofImage image(pixelsRes);
    image.resize(rectSize.width, rectSize.height); // freeimage
    _pixels.setFromPixels(
        image.getPixels().getData(),
        image.getWidth(),
        image.getHeight(),
        image.getImageType()
    );
#endif
}
//--------------------------------------------------------------
void ofApp::setup() {

    ofSetLogLevel(OF_LOG_NOTICE);

    ofImage image;
    int longestSide = 15000; // somehow works with small size like 1000, crashes on large sizes
    ofInterpolationMethod interp;

    for (int i = 1; i <= 3; i++)
    {
        ofLogNotice(__FUNCTION__) << i;
        switch (i)
        {
        case 1: interp = OF_INTERPOLATE_NEAREST_NEIGHBOR; break;
        case 2: interp = OF_INTERPOLATE_BILINEAR; break;
        case 3: interp = OF_INTERPOLATE_BICUBIC; break;
        default:
            std::exit(1);
            break;
        }

        bool b = image.load("face.jpg");
        assert(b);

        bool bStretch = true;
        image.load("face.jpg");
        resize(image.getPixels(), longestSide, ofVec2f(16, 9), bStretch, interp);
        // how does the image know that the pixels are scaled now?
        image.setFromPixels(image.getPixels()); // ???
        image.save("face_169_stretch_" + ofToString(i) + ".jpg");

        image.load("face.jpg");
        resize(image.getPixels(), longestSide, ofVec2f(9, 16), bStretch, interp);
        image.setFromPixels(image.getPixels()); // ???
        image.save("face_916_stretch_" + ofToString(i) + ".jpg");

        bStretch = false;
        image.load("face.jpg");
        resize(image.getPixels(), longestSide, ofVec2f(16, 9), bStretch, interp);
        image.setFromPixels(image.getPixels()); // ???
        image.save("face_169_crop_" + ofToString(i) + ".jpg");

        image.load("face.jpg");
        resize(image.getPixels(), longestSide, ofVec2f(9, 16), bStretch, interp);
        image.setFromPixels(image.getPixels()); // ???
        image.save("face_916_crop_" + ofToString(i) + ".jpg");
    }
}
NickHardeman commented 5 months ago

Yeah, check out here for loading into a texture. https://github.com/openframeworks/openFrameworks/blob/638a6cb9a09005d3bbc860f1cb312a435819eca1/libs/openFrameworks/gl/ofCubeMap.cpp#L332

I had to do some checks for OPENGL_ES since some versions or platforms didn't like GL_RGB32F and values over a certain limit would cause strange issues.

float* fPixData = fpix.getData();
unsigned int numFs = fw * fh * fpix.getNumChannels();
for( unsigned int i = 0; i < numFs; i++ ) {
    // clamp to the maximum value of float16, medium precision
    if(fPixData[i] > 65504.f) {
        fPixData[i] = 65504.f;
    }
}
NickHardeman commented 5 months ago

Using ofLoadImage( texture, "img.hdr" ) to load a texture should be supported as well. https://github.com/openframeworks/openFrameworks/blob/638a6cb9a09005d3bbc860f1cb312a435819eca1/libs/openFrameworks/graphics/ofImage.cpp#L361-L382

dimitre commented 5 months ago

@janimatic I've updated your tests and pushed code to ofTests repo. it seems 16bit resize is still not working Digital_LAD_HD720_16bit_copy

janimatic commented 5 months ago

@dimitre nice find, i'll take a look asap! Talking about the current hdr support in ofImage, I believe (I might be wrong) :

janimatic commented 5 months ago

@dimitre i just looked quickly at the new test... but resizeIOTest_ofImage<float>(thumbnailProxy, "Digital_LAD_HD720_16bit.png"); is using ofImage so the resize is done with FreeImage, not OF if i am not mistaken. We could try to do the resize though of pixels by loading 16 bits with stb though... (or even simpler, loading float pixels from 16 bits pixels in stb : didn't resizeIOTest_ofPixels<float>(thumbnailProxy, "Digital_LAD_HD720_16bit.png"); work ?)

dimitre commented 3 months ago

I think this issue can be tested again with latest master so we know if it can be closed already