p5.dom CSS transforms #389, translate() rotate()

[futuremarc]

Adds translate() and rotate() features to the dom add-on.

[lmccart]

it would probably be good to check here whether there are 2 or 3 arguments, and use regular translate rather than translate3d if only x and y are provided.

style wise, when there is a variable number of arguments, we generally leave the params empty and use arguments.length to test how many there and arguments[0] etc to grab them.

[lmccart]

@futuremarc this looks great at quick glance! @shiffman do you have time to review this and merge if all looks good?

[shiffman]

Yes, apologies, I am under crushing Learning Processing 2nd Edition deadline this Friday 4pm EST. Will come up for air and take a closer look after.

[futuremarc]

Sorry, this pull request was done with my master branch resulting in additional changes ... next time i'll make a branch of my fork for each PR.

[futuremarc]

since i didn't label all the issues in the commits, this pull request now addresses:

#389 : adds translate() and rotate()
#391 : fixes size() position() width and height
#561 : adds a callback to createImg() and updates width and height of elt

[shiffman]

Awesome! I'll be reviewing this over the weekend / Monday and will let @lmccart know if I have questions. Looks like there are some merge conflicts so you might want to check re: pulling any updates processing/p5.js. Apologies if my delays are the root of this.

[shiffman]

Ok, I've played with this a bit. This is what I've tested so far:

• size() works great!
• translate() works great!
• I'm not able to get rotate to work, I'm trying:

function setup() {
  noCanvas();
  var txt = createDiv("all the world's a stage");
  txt.style("background-color","#00FF00");
  txt.position(100, 100);
  txt.rotate(45);
  // txt.rotate(PI/4) etc.
}

• I'm not getting the createImg() callback to work, but maybe my syntax is wrong. My test code:

var img;

function setup() {
  createCanvas(250, 250);
  img = createImg('https://upload.wikimedia.org/wikipedia/commons/thumb/5/5c/Double-alaskan-rainbow.jpg/320px-Double-alaskan-rainbow.jpg',
    imgLoaded);
}

function draw() {

}


function imgLoaded() {
  console.log('loaded');
  img.hide();
  image(img, 0, 0, width, height);
}

By the way, if we don't already we should probably support. .

creatImg(url, callback);

function callback() {
  this.hide();
  image(this, 0,0);
}

Where the loaded image's context is bound to the callback (if I'm using the right terminology). @futuremarc let me know if I'm missing stuff, we can jump on hangout or switch to e-mail thread for debugging if preferable.

[futuremarc]

rotate() should be fixed now. createImg() takes a callback as its 3rd arg and it now supports being able to reference the element's this from within the callback.

[futuremarc]

Do you think we really need the alt argument in createImg()? I'm not sure many people will miss it in place of createImg(src,callback)

[shiffman]

I tested everything and it's working great. Two questions @lmccart.

1. Should p5.Element.rotate() follow angleMode() or is the DOM context different enough that it should remain degrees always?
2. I think the alt argument should probably be optional for createImg(). Do these ways of calling the function make sense?

createImg(url);
createImg(url, alt);
createImg(url, callback);
createImg(url, alt, callback);

I think the p5 precedent is callback always last?

[lmccart]

1. I think maybe it should ignore angleMode. My reason for this is that p5.js defaults to radians, but CSS operates in degrees. So it seems strange to abstract away the default for the sake of maintaining angleMode. I think it's reasonable to say that there are some states and function that only pertain to drawing (push/pop, textAlign and other text methods, colorMode, etc).
2. Yeah makes sense to make it optional, and the callback is always last. It would not be hard to support all the varieties above by checking the type of args 2 and 3.

[lmccart]

can you explain what's happening here with this change?

[futuremarc]

!important was breaking size() for Chrome and Firefox

[kadamwhite]

I'm a little uncomfortable using !important internally within our generated elements. There are cases when I'd imagine a user of p5 would want to be able to control some of this via styling; this change makes that more difficult.

[lmccart]

this styling is to handle the scaling for retina (canvases are created at a multiple size based on device pixel ratio, then scaled down with style attributes), but i'm not sure the !important is actually needed. @futuremarc could you test without and see if it still works without?

[futuremarc]

Yes, sure

[kadamwhite]

It's a minor thing, but in general it's best to avoid defining variables (i.e., using var) inside a conditional block: these two functions are exactly the same in operation

function one() {
  var a;
  if ( this.elt instanceof HTMLCanvasElement ) {
    a = true;
  }
}
// and
function one() {
  if ( this.elt instanceof HTMLCanvasElement ) {
    var a = true;
  }
}

and the former is usually considered preferable because it avoids the risk of unintuitive behavior due to variable hoisting.

Additionally, it would be best to avoid uninformative variable names like j and k: when p5 gets minified all variable names will be cleaned up anyway, so within the p5 code itself we try to use descriptive variable names like autoWidth or 2dDrawingContext.

[futuremarc]

These actually weren't my changes (merge was messed up), but good to know re: variable hoisting

[lmccart]

ok awesome, looks like this one is good to go now. thanks for all the work with the revisions.

[shiffman]

yay, great work @futuremarc!