P5.js: Update documentation for image() to clarify what arguments mean

Created on 10 Sep 2017  路  14Comments  路  Source: processing/p5.js

Nature of issue?

  • [x] Found a bug
  • [ ] Existing feature enhancement
  • [ ] New feature request

Most appropriate sub-area of p5.js?

  • [ ] Color
  • [ ] Core
  • [ ] Data
  • [ ] Events
  • [x] Image
  • [ ] IO
  • [ ] Math
  • [ ] Typography
  • [ ] Utilities
  • [ ] WebGL
  • [ ] Other (specify if possible)

Which platform were you using when you encountered this?

  • [x] Mobile/Tablet (touch devices)
  • [x] Desktop/Laptop
  • [ ] Others (specify if possible)

Details about the bug:

  • p5.js version: v0.5.4
  • Web browser and version: tested on Crome & Safari
  • Operating System: tested on windows and MacOSX
  • Steps to reproduce this:
    Its a documentation issue, when using p5's Image function to draw an image the code listed on the reference is:
    image(img,dx,dy,dWidth,dHeight,sx,sy,sWidth,sHeight)
    (d for destination, s for source)
    When in reality it is:
    image(img,sx,sy,sWidth,sHeight,dx,dy,dWidth,dHeight)
    (d for destination, s for source in both examples).
beginner

Most helpful comment

@mathuramg and I are taking this so we can use it next Thurs in a 'How to contribute to p5' workshop at ITP.

All 14 comments

@mathuramg and I are taking this so we can use it next Thurs in a 'How to contribute to p5' workshop at ITP.

@kaganjd is that workshop open to the public by any chance?

@austince yeah, if you're in NYC, you should come by! details:

9/21, 6-7:30pm (ish)
721 Broadway, 4th floor, Conference Room
my email is jen.[email protected] if you have trouble getting into the building.

if you're not in NYC, we'll publish the notes here: http://mathuramg.com/p5-contr-workshop/

@kaganjd I am! I'll try to make it, thanks for the deets 馃

Please ignore that commit above-- was just a demo in the workshop.

But I did dig into this issue and I think the documentation for this function is confusing. I struggled to make sense of the "source" "destination" and "sub-rectangle" language.

What if we use "main canvas" "image canvas" "original image" and "drawn image" to describe how these dimensions relate to each other? See the comments in this sketch, where I draw different parts of the same photo using the different parameters: https://alpha.editor.p5js.org/kaganjd/sketches/SkLtyxzo-

Apart from wording, I think simply adding an image like the following would be great at showing what the arguments meant.
drawimage_arguments

I like the idea of updating the wording and including a diagram to make this clear

Is this workshop available online? I enjoy using p5.js and am also interested in ITP and would love to contribute to this if possible!

@limzykenneth that's super helpful. I'm traveling with limited internet access at the moment, but I'll submit a PR in a few days with wording changes + that graphic.

@bethyd our workshop notes are here: http://mathuramg.com/p5-contr-workshop/ feel free to email me if you have questions about getting set up!

Sorry all, I'm working on the image() webGL function at the moment, and I just wanted to check something regarding this. It's entirely possible I'm misunderstanding something, but I'd feel odd not raising my hand.

So there are actually two image() functions.

One in src/image/loading_displaying.js and another in the renderer itself (src/core/p5.Renderer2D.js).
If you take a look at the former, its signature is

p5.prototype.image =
  function(img, dx, dy, dWidth, dHeight, sx, sy, sWidth, sHeight) {

Which is indeed what's reflected in the documentation (it then goes and sets default values for anything not passed in, and passes those data through to the image() of the renderer).
This makes sense, because it allows the source dimension arguments to be optional.

As far as I can tell, the documentation's argument order is right, at least in terms of the way that images are loaded as described in this page of the reference.

Note that the arguments were switched on the 27th October 2016 here: 6ac5bc00e101c739e1c423939294a8db51cf3210

p5.js v0.5.4 was the 1st October 2016 release, so the mismatch originally reported is because they need to use older documentation or a newer p5 version.

I see no issues with the code as it currently stands, but documentation can always be improved.

Ah, brilliant @meiamsome, that clarifies it (for me, at least)!
Maybe we (or someone who's able) should change the name/description of the issue (or create a new one) in that case, if @kaganjd is still going to be working on clarifying the documentation?

@bomoko I only really thought it might have changed because you pointed out the differences between the two prototypes, so that was a bit lucky really...

On another note, perhaps having versioned documentation on the site would be useful? It might be a bit of work but would probably be quite useful. Perhaps I should open an issue for that separately.

Opened a PR for this, please let me know if you have suggestions @limzykenneth @bomoko @meiamsome !

Was this page helpful?
0 / 5 - 0 ratings

Related issues

ghost picture ghost  路  3Comments

b0nb0n picture b0nb0n  路  3Comments

Patchy12 picture Patchy12  路  3Comments

aferriss picture aferriss  路  3Comments

kartikay-bagla picture kartikay-bagla  路  3Comments