P5.js: p5.Graphics and copy()

[shiffman]

I'm testing an example where I'm using copy() with createGraphics(). copy() works as expected with p5.Image.

var img = createImage(100, 100);
copy(img, 0, 0, 100, 100, 0, 0, 100, 100);

but does not work with p5.Graphics.

var graphics = createImage(100, 100);
copy(graphics, 0, 0, 100, 100, 0, 0, 100, 100);

It fails on this line in copyHelper()

  var s = srcImage.canvas.width / srcImage.width;

I believe I just need to detect the argument type p5.Graphics object or p5.Image, something like:

p5.Renderer2D._copyHelper =
function (srcImage, sx, sy, sw, sh, dx, dy, dw, dh) {
  var canvas;
  if (srcImage instanceof p5.Image) {
    canvas = srcImage.canvas;
  } else if (srcImage instanceof p5.Graphics) {
    canvas = srcImage.elt;
  }
  var s = canvas.width / srcImage.width;
  this.drawingContext.drawImage(canvas,
    s * sx, s * sy, s * sw, s * sh, dx, dy, dw, dh);
};

This should work, but for whatever reason, this.drawingContext is pointing to the canvas made with createGraphics() when copy() is called and not the actual default p5 canvas! I'm a little stuck.

Am I making sense? Can anyone shed any light? I believe copy() should support p5.Graphics, yes?

[laurentoget]

I thought trying to figure this out would be a good way to make myself familiar with the code base (great project btw, nice to meet you all), but after 30 minutes in the code, i am still utterly confused. Feels to me like a good moment to put of moratorium on the use of instanceof until someone who understands the relationship between canvas, renderer and drawingcontext makes a drawing for the rest of the world.

[lmccart]

@shiffman is copy() supposed to work for p5.Graphics? looking at the processing reference, it looks like it only supports PImage? or does it also work for PGraphics? I suppose we can make it support p5.Graphics in any case, just wondering if there's some reason Processing made the decision not to that we should consider?
https://processing.org/reference/copy_.html

[lmccart]

@shiffman well, the above question aside, I think your fix was really close. I got the following code to work:

function setup() {
  createCanvas(400, 400);
  background(0);
  var img = createGraphics(100, 100);
  img.ellipse(50, 50, 50, 50);
  copy(img, 0, 0, 100, 100, 0, 0, 100, 100);
  updatePixels();
}

with this patch:

p5.Renderer2D._copyHelper =
function (srcImage, sx, sy, sw, sh, dx, dy, dw, dh) {
  var canvas;
  if (srcImage instanceof p5.Image) {
    canvas = srcImage.canvas;
  } else if (srcImage instanceof p5.Graphics) {
    srcImage.loadPixels();
    canvas = srcImage.elt;
  }
  var s = canvas.width / srcImage.width;
  this.drawingContext.drawImage(canvas,
    s * sx, s * sy, s * sw, s * sh, dx, dy, dw, dh);
};

the key is the srcImage.loadPixels() call I added in there. note that I also had to call updatePixels() at the end of setup to get it to show up. is this correct behavior or maybe this should be called in _copyHelper() as well?

[lmccart]

@laurentoget thank you for having a go at this. I hear your concern and we are in the process of writing up better developer documentation. it's just a matter of finding time to do all the things we want to with the volunteers and hours we have to work on this. :)

in general, issues marked "beginner" are good starting points for someone new to the codebase. (though I see there aren't too many with that label at the moment.)

[shiffman]

Ah, you are right. I just _assumed_ you could copy() a PGraphics object because, well, why wouldn't you? But in fact it is not supported. I discovered, however, that set() and get() work with PGraphics in Processing. . . but not p5.Graphics here. Except that actually I do get it to work back in 0.4.9, but not in anything post 0.4.10.

The thing I'm trying to do is grab the pixels of an image loaded with createImg() rather than loadImage(). So the working code I do have in 0.4.9 is as follows:

var img;

function setup() {
  createCanvas(400, 300);
  background(0);
  img = createImg('sunflower.jpg', created);
}

function created() {
  graphics = createGraphics(img.width, img.height);
  graphics.image(img, 0, 0, graphics.width, graphics.height);
  var pix = graphics.get(0, 0, 50, 50);
  image(pix, 0, 0);
}

So I think there are two questions / issues:

1. I think this is a bug? get() should work as above in 0.4.15.
2. Is there a better way to get access to the pixels of an image loaded with createImg(). Could get() or copy() be applied directly to a <img> p5.Element?

[laurentoget]

I do not understand this enough to figure why but I suspect adding either graphics.loadPixels or img.loadPixels will get this working.

[lmccart]

I just pushed a patch which should fix problem 1. I don't think there's a better way than what you have above, but I'd be open to adding something if we could think of something intuitive and not too hacky.

[shiffman]

I sometimes wonder how much magic we should make happen behind the scenes with loadImage() and urls. For example, we could detect a URL as argument to loadImage(), use createImg() behind the scenes, hide the element, and copy the pixels into a p5.Image object sending it into the callback.

But this is probably too much magic and leaves users open to other problems and confusion about how things work later? We can leave this open for a bit to think about it, but I think best to see how things go with student projects and where trouble arises (or doesn't) before adding in functionality like this.

[lmccart]

sounds good. what do you recommend in terms of supporting p5.Graphics in copy? I can add that patch in if it makes sense. we should then probably close this issue or at least update to reflect the current question. though if this is more of an open question/thinking point maybe it's better just to close it completely and keep it floating in our heads, and reopen at the end of the semester if it seems useful to implement something.

[shiffman]

While trying out a workaround related to #1022, I attempted to get access to the pixels of a p5.Graphics object, i.e.

function setup() {
  createCanvas(200, 200);
  background(0);
  var g = createGraphics(100, 100);
  g.background(255, 0, 0);
  g.loadPixels();
  image(g, 0, 0);
  console.log(g.pixels);
}

The console shows:

[]

I think this is probably the same question as this issue? Should we support copy() and loadPixels() and pixels for p5.Graphics objects? I'm now really leaning towards yes!

[lmccart]

yes, pixels() and loadPixels() should be supported, this is a bug. I can add the copy() support in there as well.

[Dolphin101546015]

Have same problem in final processing release.
With patch applying issue was fixed.
Make changes in release please!

[BenjaminHabert]

Is there still an issue here ? It is unclear what needs fixing, can someone rephrase it ?

I reproduced in the p5 web editor the previously shown snippets that used to fail and they all seem to work:

• copy() image and graphics to canvas
• copy() and image() image to graphics
• copy() and image() graphics to graphics (note: in the latter case the scaling is wrong, which seems related to #2077)
• image() an image from createImg() to graphics
• get() portion of a graphics
• loadPixels() and .pixels of a graphics

[shiffman]

I think this issue can be closed? It was a lovely discussion but it does appear it's all been resolved?

[limzykenneth]

Let's close this, if anything else seem wrong, just open a new issue for that specific problem 😃