added a loadFromDataBuffer method to Image #177

Closed
victusfate opened this Issue May 30, 2012 · 14 comments

3 participants

@victusfate

Heyo TJ, in my fork I added another load method because for the life of me I couldn't figure out how to create an Image from a raw buffer with the existing source (widthheight4 bytes of rgb data goodness).

Here's the code, if you have a chance please let me know if it's not kosher

cairo_status_t
Image::loadFromDataBuffer(unsigned char *buf, int width, int height) {
  clearData();
  int stride = cairo_format_stride_for_width (CAIRO_FORMAT_ARGB32, width); // 4*width + ?
  _surface = cairo_image_surface_create_for_data(buf,CAIRO_FORMAT_ARGB32,width,height,stride);
  loaded();
  return cairo_surface_status(_surface);
}

victusfate@817a750

@tj
tj commented May 30, 2012

we have img.src = Buffer I believe, maybe not exposed as a nice c++ API I can't remember ATM

@victusfate
@tj
tj commented May 31, 2012

weird we dont have createImageData() implemented, that's probably what you're looking for if you just want to pass pixel data

@victusfate

groovy, the little section of code I pasted in above seems to do the trick for my local tests.
Would createImageData() do something different? Maybe your stream id (png, jpeg) could identify a raw uint8_t buffer on a img.src = check.

also I should clarify:

fs.readFile(__dirname + '/BobRoss.jpg', function(err, image_data){
  if (err) throw err;
  img = new Image;
  img.src = image_data; // is fast


  img.src = Canvas.toBuffer(); //is slowwwwwwww 

toBuffer is the one I'm working around. Having a good time reading through the code, although I bumped into a few issues with cairo, their docs were helpful.

@tj
tj commented May 31, 2012

well of course img.src = canvas.toBuffer() is slower :p not sure how that affects assigning the src from binary though, but we should add createImageData which would let you pass a PixelArray

@victusfate

that buffer route was the default path for loading canvases into Images, and a pain point in our process. If you've got a faster way to convert a painted on canvas into an Image I'm all ears!

I ran into a bug with the load method I added above (worked for my local test images, but was crashing when called form our server, which meant "forking" my console cout spam to syslog). I need to debug it more, I'm cairo surface learning impaired.

I'm kinda old school with images, rows, cols, a databuffer and I'm happy. Cubes too (rgba are just wh4). The cairo data handling takes a little getting used to.

Maybe I'm not doing the right thing memory wise with the clearData() and loaded() calls?

Btw, thanks for the responses, I know you're busy with a bunch of repos.

@victusfate

had to add some additional logging to my fork (for long term monitoring)
turns out there's a memory error created by my new method loadFromDataBuffer call

do I need to create or set a closure? is javascript trying to free an old buffer?
I see you refer to one in your other loadBuffer variations (jpeg, png)

  read_closure_t closure;
  closure.len = 0;
  closure.buf = buf;

ps, looking forward to learning more about gyp bindings from your example. all the examples I could find were too simplistic.

@victusfate

was just acking through the code looking for all allocations and I spotted this line in Canvas::resurface

V8::AdjustAmountOfExternalAllocatedMemory(4 * (width * height - old_width * old_height));

Do I need to inform V8 about memory allocated in the loadfromdatabuffer call?

update
ok it looks like Image::clearData decreases the amount to V8, I'll just increment it by the new data buffer size then.

second update
Image::loaded which I call after resurfacing already tells V8 about the new data buffer size, so I don't need to.

@victusfate

I'm confused (or failing to read the docs). Why does it take so long to go from a Canvas to an Image, if they're both just cairo surfaces internally?
I'm going down this road to begin with because

var image = new Image;
image.src = Canvas.toBuffer();

is hundred(s) of milliseconds.

I think going from a Canvas to an Image should be nearly instant (memcpy/single for loop) since they have have the same internal representation.

I could add a Canvas.toImage() method that will construct and return a new image, and it shouldn't take much time.

var image = Canvas.toImage();
@tj
tj commented Jun 6, 2012

it uses cairo_surface_write_to_png_stream and a memcpy to the buffer, we could probably add faster support for img.src = Canvas, right now it's just the data uri string and buffer

@victusfate

muchas gracias for the quick reply. trying to figure out the memory error I borked up, using an eclipses gdb wrapper.

I'm only seeing the error on the os x side, doesn't appear on ubuntu.

node(77583,0x7fff7585a960) malloc: *** error for object 0x7fad435184e0: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug

Seen anything like this before? It happens when v8 is cleaning things up at the end of any of my test scripts.

side note, I see I'm missing

HandleScope scope; 

in a number of functions. I see it's required where ever handles are handled

Local<Object> obj = args[argNum]->ToObject();
@victusfate

I may have concealed the issue or corrected it. I added HandleScopes to all the functions that deal with V8 wrappers and the code completes without error. It's frustrating not knowing for sure, giving myself some homework to read into how HandleScope signals javascript about object cleanup.
http://izs.me/v8-docs/classv8_1_1HandleScope.html#_details

@victusfate

for reference here's where I call loadImageData and while it runs locally under a variety of conditions, in our production environment when carefully tucked away within a frame server it appears to be causing segfaults (spent the day yesterday trying to gdb node_g our server code but the frame servers are kicked off by gearman... TL;DR didn't get a root cause stack trace)

https://github.com/victusfate/node-canvas/blob/master/src/Canvas.cc#L497

Handle<Value>
 Canvas::LoadImage(const Arguments &args) {
   HandleScope scope;
   LogStream mout(LOG_DEBUG,"node-canvas.paint.ccode.Canvas.LoadImage");    
   mout << "Canvas::LoadImage top " << LogStream::endl;

   Canvas *canvas = ObjectWrap::Unwrap<Canvas>(args.This());
   if (args.Length() < 1) {
     mout << "Canvas::LoadImage Error requires one argument of Image type " << LogStream::endl;
     return ThrowException(Exception::TypeError(String::New("Canvas::LoadImage requires one argument of Image type")));
   }

   Local<Object> obj = args[0]->ToObject();
   Image *img = ObjectWrap::Unwrap<Image>(obj);
   canvas->loadImageData(img);
   return Undefined();
}  

void Canvas::loadImageData(Image *img) {
  LogStream mout(LOG_DEBUG,"node-canvas.paint.ccode.Canvas.loadImageData");    
  if (this->isPDF()) {
    mout << "Canvas::loadImageData pdf canvas type " << LogStream::endl;
    cairo_surface_finish(this->surface());
    closure_t *closure = (closure_t *) this->closure();

    int w = cairo_image_surface_get_width(this->surface());
    int h = cairo_image_surface_get_height(this->surface());

    img->loadFromDataBuffer(closure->data,w,h);
    mout << "Canvas::loadImageData pdf type, finished loading image" << LogStream::endl;
  }
  else {
    mout << "Canvas::loadImageData data canvas type " << LogStream::endl;
    cairo_surface_flush(this->surface());
    int w = cairo_image_surface_get_width(this->surface());
    int h = cairo_image_surface_get_height(this->surface());

    img->loadFromDataBuffer(cairo_image_surface_get_data(this->surface()),w,h);
    mout << "Canvas::loadImageData image type, finished loading image" << LogStream::endl;
  }   
}

and here's what the current method in Image looks like (I removed some commented out logging info)
https://github.com/victusfate/node-canvas/blob/master/src/Image.cc#L240

/*
 * load from data buffer width*height*4 bytes
 */
cairo_status_t
Image::loadFromDataBuffer(uint8_t *buf, int width, int height) {
  this->clearData();
  int stride = cairo_format_stride_for_width (CAIRO_FORMAT_ARGB32, width); // 4*width + ?
  this->_surface = cairo_image_surface_create_for_data(buf,CAIRO_FORMAT_ARGB32,width,height,stride);
  this->data_mode = DATA_IMAGE;
  this->loaded();
  cairo_status_t status = cairo_surface_status(_surface);
  if (status) return status;
  return CAIRO_STATUS_SUCCESS;
}
@LinusU

Please place a comment if this is still relevant and I'll reopen, I believe that we have made quite a few changes to the relevant parts of the code though...

@LinusU LinusU closed this Feb 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment