Cycles: Pass extra array size argument to builtin image pixels functions
authorSergey Sharybin <sergey.vfx@gmail.com>
Tue, 29 Nov 2016 10:03:11 +0000 (11:03 +0100)
committerSergey Sharybin <sergey.vfx@gmail.com>
Tue, 29 Nov 2016 10:03:11 +0000 (11:03 +0100)
This is a way to avoid possible memory corruption when render threads works
in parallel with UI thread.

Not guarantees complete safe, but makes things easier to check anyway.

intern/cycles/blender/blender_session.cpp
intern/cycles/blender/blender_session.h
intern/cycles/render/image.cpp
intern/cycles/render/image.h

index 171153dd440ddf76041a5ec19e1d1c4f0f18adf9..e16cea0ebaf4643d9538a701a1713b69d4655825 100644 (file)
@@ -126,8 +126,8 @@ void BlenderSession::create_session()
 
        /* setup callbacks for builtin image support */
        scene->image_manager->builtin_image_info_cb = function_bind(&BlenderSession::builtin_image_info, this, _1, _2, _3, _4, _5, _6, _7);
-       scene->image_manager->builtin_image_pixels_cb = function_bind(&BlenderSession::builtin_image_pixels, this, _1, _2, _3);
-       scene->image_manager->builtin_image_float_pixels_cb = function_bind(&BlenderSession::builtin_image_float_pixels, this, _1, _2, _3);
+       scene->image_manager->builtin_image_pixels_cb = function_bind(&BlenderSession::builtin_image_pixels, this, _1, _2, _3, _4);
+       scene->image_manager->builtin_image_float_pixels_cb = function_bind(&BlenderSession::builtin_image_float_pixels, this, _1, _2, _3, _4);
 
        /* create session */
        session = new Session(session_params);
@@ -1080,7 +1080,13 @@ int BlenderSession::builtin_image_frame(const string &builtin_name)
        return atoi(builtin_name.substr(last + 1, builtin_name.size() - last - 1).c_str());
 }
 
-void BlenderSession::builtin_image_info(const string &builtin_name, void *builtin_data, bool &is_float, int &width, int &height, int &depth, int &channels)
+void BlenderSession::builtin_image_info(const string &builtin_name,
+                                        void *builtin_data,
+                                        bool &is_float,
+                                        int &width,
+                                        int &height,
+                                        int &depth,
+                                        int &channels)
 {
        /* empty image */
        is_float = false;
@@ -1158,60 +1164,67 @@ void BlenderSession::builtin_image_info(const string &builtin_name, void *builti
        }
 }
 
-bool BlenderSession::builtin_image_pixels(const string &builtin_name, void *builtin_data, unsigned char *pixels)
+bool BlenderSession::builtin_image_pixels(const string &builtin_name,
+                                          void *builtin_data,
+                                          unsigned char *pixels,
+                                          const size_t pixels_size)
 {
-       if(!builtin_data)
+       if(!builtin_data) {
                return false;
+       }
 
-       int frame = builtin_image_frame(builtin_name);
+       const int frame = builtin_image_frame(builtin_name);
 
        PointerRNA ptr;
        RNA_id_pointer_create((ID*)builtin_data, &ptr);
        BL::Image b_image(ptr);
 
-       int width = b_image.size()[0];
-       int height = b_image.size()[1];
-       int channels = b_image.channels();
+       const int width = b_image.size()[0];
+       const int height = b_image.size()[1];
+       const int channels = b_image.channels();
 
-       unsigned char *image_pixels;
-       image_pixels = image_get_pixels_for_frame(b_image, frame);
-       size_t num_pixels = ((size_t)width) * height;
+       unsigned char *image_pixels = image_get_pixels_for_frame(b_image, frame);
+       const size_t num_pixels = ((size_t)width) * height;
 
-       if(image_pixels) {
-               memcpy(pixels, image_pixels, num_pixels * channels * sizeof(unsigned char));
+       if(image_pixels && num_pixels * channels == pixels_size) {
+               memcpy(pixels, image_pixels, pixels_size * sizeof(unsigned char));
                MEM_freeN(image_pixels);
        }
        else {
                if(channels == 1) {
-                       memset(pixels, 0, num_pixels * sizeof(unsigned char));
+                       memset(pixels, 0, pixels_size * sizeof(unsigned char));
                }
                else {
+                       const size_t num_pixels_safe = pixels_size / channels;
                        unsigned char *cp = pixels;
-                       for(size_t i = 0; i < num_pixels; i++, cp += channels) {
+                       for(size_t i = 0; i < num_pixels_safe; i++, cp += channels) {
                                cp[0] = 255;
                                cp[1] = 0;
                                cp[2] = 255;
-                               if(channels == 4)
+                               if(channels == 4) {
                                        cp[3] = 255;
+                               }
                        }
                }
        }
-
-       /* premultiply, byte images are always straight for blender */
+       /* Premultiply, byte images are always straight for Blender. */
        unsigned char *cp = pixels;
        for(size_t i = 0; i < num_pixels; i++, cp += channels) {
                cp[0] = (cp[0] * cp[3]) >> 8;
                cp[1] = (cp[1] * cp[3]) >> 8;
                cp[2] = (cp[2] * cp[3]) >> 8;
        }
-
        return true;
 }
 
-bool BlenderSession::builtin_image_float_pixels(const string &builtin_name, void *builtin_data, float *pixels)
+bool BlenderSession::builtin_image_float_pixels(const string &builtin_name,
+                                                void *builtin_data,
+                                                float *pixels,
+                                                const size_t pixels_size)
 {
-       if(!builtin_data)
+       if(!builtin_data) {
                return false;
+       }
 
        PointerRNA ptr;
        RNA_id_pointer_create((ID*)builtin_data, &ptr);
@@ -1222,16 +1235,16 @@ bool BlenderSession::builtin_image_float_pixels(const string &builtin_name, void
                BL::Image b_image(b_id);
                int frame = builtin_image_frame(builtin_name);
 
-               int width = b_image.size()[0];
-               int height = b_image.size()[1];
-               int channels = b_image.channels();
+               const int width = b_image.size()[0];
+               const int height = b_image.size()[1];
+               const int channels = b_image.channels();
 
                float *image_pixels;
                image_pixels = image_get_float_pixels_for_frame(b_image, frame);
-               size_t num_pixels = ((size_t)width) * height;
+               const size_t num_pixels = ((size_t)width) * height;
 
-               if(image_pixels) {
-                       memcpy(pixels, image_pixels, num_pixels * channels * sizeof(float));
+               if(image_pixels && num_pixels * channels == pixels_size) {
+                       memcpy(pixels, image_pixels, pixels_size * sizeof(float));
                        MEM_freeN(image_pixels);
                }
                else {
@@ -1239,13 +1252,15 @@ bool BlenderSession::builtin_image_float_pixels(const string &builtin_name, void
                                memset(pixels, 0, num_pixels * sizeof(float));
                        }
                        else {
+                               const size_t num_pixels_safe = pixels_size / channels;
                                float *fp = pixels;
-                               for(int i = 0; i < num_pixels; i++, fp += channels) {
+                               for(int i = 0; i < num_pixels_safe; i++, fp += channels) {
                                        fp[0] = 1.0f;
                                        fp[1] = 0.0f;
                                        fp[2] = 1.0f;
-                                       if(channels == 4)
+                                       if(channels == 4) {
                                                fp[3] = 1.0f;
+                                       }
                                }
                        }
                }
@@ -1257,8 +1272,9 @@ bool BlenderSession::builtin_image_float_pixels(const string &builtin_name, void
                BL::Object b_ob(b_id);
                BL::SmokeDomainSettings b_domain = object_smoke_domain_find(b_ob);
 
-               if(!b_domain)
+               if(!b_domain) {
                        return false;
+               }
 
                int3 resolution = get_int3(b_domain.domain_resolution());
                int length, amplify = (b_domain.use_high_resolution())? b_domain.amplify() + 1: 1;
@@ -1270,10 +1286,10 @@ bool BlenderSession::builtin_image_float_pixels(const string &builtin_name, void
                        amplify = 1;
                }
 
-               int width = resolution.x * amplify;
-               int height = resolution.y * amplify;
-               int depth = resolution.z * amplify;
-               size_t num_pixels = ((size_t)width) * height * depth;
+               const int width = resolution.x * amplify;
+               const int height = resolution.y * amplify;
+               const int depth = resolution.z * amplify;
+               const size_t num_pixels = ((size_t)width) * height * depth;
 
                if(builtin_name == Attribute::standard_name(ATTR_STD_VOLUME_DENSITY)) {
                        SmokeDomainSettings_density_grid_get_length(&b_domain.ptr, &length);
index 66a6945cbc1cffc032e732e59fe545dc385d564b..82fe218b4cea67b993080edd400f97fdc937019d 100644 (file)
@@ -145,9 +145,21 @@ protected:
        void do_write_update_render_tile(RenderTile& rtile, bool do_update_only);
 
        int builtin_image_frame(const string &builtin_name);
-       void builtin_image_info(const string &builtin_name, void *builtin_data, bool &is_float, int &width, int &height, int &depth, int &channels);
-       bool builtin_image_pixels(const string &builtin_name, void *builtin_data, unsigned char *pixels);
-       bool builtin_image_float_pixels(const string &builtin_name, void *builtin_data, float *pixels);
+       void builtin_image_info(const string &builtin_name,
+                               void *builtin_data,
+                               bool &is_float,
+                               int &width,
+                               int &height,
+                               int &depth,
+                               int &channels);
+       bool builtin_image_pixels(const string &builtin_name,
+                                 void *builtin_data,
+                                 unsigned char *pixels,
+                                 const size_t pixels_size);
+       bool builtin_image_float_pixels(const string &builtin_name,
+                                       void *builtin_data,
+                                       float *pixels,
+                                       const size_t pixels_size);
 
        /* Update tile manager to reflect resumable render settings. */
        void update_resumable_tile_manager(int num_samples);
index 11193bf4974a7ef0fa2afa511447ab588f1257cc..ab830b19c5775c9821dac4b7b21b12c63078c704 100644 (file)
@@ -498,6 +498,7 @@ bool ImageManager::file_load_image(Image *img,
                pixels = (StorageType*)tex_img.resize(width, height, depth);
        }
        bool cmyk = false;
+       const size_t num_pixels = ((size_t)width) * height * depth;
        if(in) {
                StorageType *readpixels = pixels;
                vector<StorageType> tmppixels;
@@ -534,12 +535,14 @@ bool ImageManager::file_load_image(Image *img,
                if(FileFormat == TypeDesc::FLOAT) {
                        builtin_image_float_pixels_cb(img->filename,
                                                      img->builtin_data,
-                                                     (float*)&pixels[0]);
+                                                     (float*)&pixels[0],
+                                                     num_pixels * components);
                }
                else if(FileFormat == TypeDesc::UINT8) {
                        builtin_image_pixels_cb(img->filename,
                                                img->builtin_data,
-                                               (uchar*)&pixels[0]);
+                                               (uchar*)&pixels[0],
+                                               num_pixels * components);
                }
                else {
                        /* TODO(dingto): Support half for ImBuf. */
@@ -552,7 +555,6 @@ bool ImageManager::file_load_image(Image *img,
                        type == IMAGE_DATA_TYPE_HALF4 ||
                        type == IMAGE_DATA_TYPE_BYTE4);
        if(is_rgba) {
-               size_t num_pixels = ((size_t)width) * height * depth;
                if(cmyk) {
                        /* CMYK */
                        for(size_t i = num_pixels-1, pixel = 0; pixel < num_pixels; pixel++, i--) {
index 3da7338985cb7579a143cd97b5a10f89546f7add..47bbd92347ce12d230de6b110e5e85cc0cb5d836 100644 (file)
@@ -86,9 +86,25 @@ public:
 
        bool need_update;
 
-       function<void(const string &filename, void *data, bool &is_float, int &width, int &height, int &depth, int &channels)> builtin_image_info_cb;
-       function<bool(const string &filename, void *data, unsigned char *pixels)> builtin_image_pixels_cb;
-       function<bool(const string &filename, void *data, float *pixels)> builtin_image_float_pixels_cb;
+       /* NOTE: Here pixels_size is a size of storage, which equals to
+        *       width * height * depth.
+        *       Use this to avoid some nasty memory corruptions.
+        */
+       function<void(const string &filename,
+                     void *data,
+                     bool &is_float,
+                     int &width,
+                     int &height,
+                     int &depth,
+                     int &channels)> builtin_image_info_cb;
+       function<bool(const string &filename,
+                     void *data,
+                     unsigned char *pixels,
+                     const size_t pixels_size)> builtin_image_pixels_cb;
+       function<bool(const string &filename,
+                     void *data,
+                     float *pixels,
+                     const size_t pixels_size)> builtin_image_float_pixels_cb;
 
        struct Image {
                string filename;