Enhanced loadModel() method signature with independent U and V flipping options.

[deveshidwivedi]

Resolves #5021

Changes:

The existing loadModel() method signatures are maintained, but a new, more flexible method is introduced as loadModel(path, [options]. This new method allows users to specify optional parameters in an options object, providing greater control over the loading process without being constrained by parameter order. The addition of flipU() and flipV() options enables users to apply U and V flipping to models after they've been loaded.

Screenshots of the change:

before:

after:

before:

after:

PR Checklist

• npm run lint passes
• Inline documentation is included / updated
• Unit tests are included / updated

[deveshidwivedi]

I have made the mentioned changes, please take a look. @davepagurek Thank you!

[davepagurek]

Nice work on this so far! I've added a few more comments about filling out the documentation, and whether or not we want to also add flipU and flipV support to the options object for convenience.

[davepagurek]

Btw looks like tests are failing due to this linting error:

Error: unner/work/p5.js/p5.js/src/webgl/loading.js:106:3: Parsing error: 'return' outside of function [Error]

[deveshidwivedi]

I tried adding the examples, how could these errors be solved?

[davepagurek]

Interesting that it's a timeout error. I wonder if there's another error but that isn't getting logged in the tests. If you launch a local docs server with grunt yui:dev and go to the docs for these methods on p5.Geometry, do the examples run successfully?

[deveshidwivedi]

I tried borrowing model path from the loadModel examples and the tests do not fail anymore. I'm getting the same results for both models:
FlipU():

FlipV():

On logging results for flipV, this is what I'm getting:

[davepagurek]

That's expected if you fill everything with a solid color. Since we're only flipping the texture coordinates, we will only see a difference if you load an image and use it as a texture. Maybe we can do that in the examples?

[deveshidwivedi]

Right!

On trying to load an image, the example doesn't seem to work.

* let img
 * let myModel;
 *
 *   function preload() {
 *   img = loadImage('assets/laDefense.jpg');
 *   myModel = loadModel('assets/octahedron.obj');
 * }
 *
 * function setup() {
 *   createCanvas(400, 400, WEBGL);
 *   background(200);
 * }
 * function draw() {
 *  background(0);
 *   translate(-150, 0, 0);
 *   texture(img);
 *   noStroke();
 *   plane(100);
 *
 *  translate(150, 0, 0);
 *  flipU(myModel);
 *  texture(img);
 *  noStroke();
 *  plane(100);
 *  model(myModel); 
 *
 * }
 * </code>
 * </div>

Could you suggest how I can make it better?

[davepagurek]

Just took a look again! Some notes:

• It looks like you're calling flipU(myModel); instead of myModel.flipU(), since it's a method on the object.
• It also looks like the octahedron model does not have texture coordinates in it, so flipping them has no effect. Maybe rather than loading a model, it would be better to manually create a simple model with texture coordinates? e.g. in preload, doing:

  myModel = buildGeometry(() => plane(50));

• Right now you're flipping the U values in draw. This should be done in setup, since once a model is drawn, updating its attributes (at least currently) will have no effect, since the resources are cached. Maybe we can make two models, that start off the same, but then we call .flipU() on one of them. Then you can draw them side-by-side in draw().
• If you run grunt yui:dev, it will launch a local version of the p5 reference so you can test your examples. This will help catch a few other minor issues, e.g. the example canvas looks pretty big compared to the text, so maybe it can be made smaller, and also the two side-by-side models being offcenter.

[deveshidwivedi]

Thank you very much, @davepagurek. Do you think any of these are okay to go ahead with, how can I make it work?

let img;
let model1;
let model2;

function preload() {
  img = loadImage('assets/laDefense.jpg');
}

function setup() {
  createCanvas(200, 200, WEBGL);
  background(200);

  model1 = buildGeometry(() => plane(50));

  model2 = buildGeometry(() => plane(50));
  model2.flipU();
}

function draw() {
  background(0);

  // original
  push();
  translate(-25, -25, 0);
  texture(img);
  noStroke();
  plane(50);
  model(model1);
  pop();

  //flipped
  push();
  translate(25, 25, 0);
  texture(img);
  noStroke();
  plane(50);
  model(model2);
  pop();
}

let img;
let model1;
let model2;

function preload() {
  img = loadImage('assets/laDefense.jpg');
}

function setup() {
  createCanvas(200, 200, WEBGL);
  background(200);

  model1 = createShape(50, 50);
  model2 = createShape(50, 50);
  model2.flipU();
}

function draw() {
  background(0);

  //original
  push();
  translate(-25, -25, 0);
  texture(img);
  noStroke();
  plane(50);
  model(model1);
  pop();

  // flipped
  push();
  translate(25, 25, 0);
  texture(img);
  noStroke();
  plane(50);
  model(model2);
  pop();
}

function createMyShape(w, h) {
  let myShape = createGraphics(w, h);
  myShape.beginShape();
  myShape.vertex(-w / 2, -h / 2, 0, 0);
  myShape.vertex(w / 2, -h / 2, 1, 0);
  myShape.vertex(w / 2, h / 2, 1, 1);
  myShape.vertex(-w / 2, h / 2, 0, 1);
  myShape.endShape(CLOSE);
  return myShape;
}

[davepagurek]

I think that first version will work! The second one could also work with some modifications (and would maybe actually be preferable since you can see the UV values being set!)

The secnd example is currently returning a p5.Graphics but trying to draw it with model(), which only works for p5.Geometry. If you want to turn your shape into geometry, it could look like this:

function createMyShape(w, h) {
  return buildGeometry(() => {
    textureMode(NORMAL);
    beginShape();
    vertex(-w / 2, -h / 2, 0, 0);
    vertex(w / 2, -h / 2, 1, 0);
    vertex(w / 2, h / 2, 1, 1);
    vertex(-w / 2, h / 2, 0, 1);
    endShape(CLOSE);
  })
}

[deveshidwivedi]

Hi @davepagurek please take a look, the examples work now.

[davepagurek]

Awesome, the examples look great!

I think the last thing to do is make sure that the new options in loadModel get displayed in the p5 reference properly. I left a comment with a suggestion there. After that we should be good to go!

[davepagurek]

Thanks for updating the docs, I think this is good to merge now!

[davepagurek]

@all-contributors please add @deveshidwivedi for code

[allcontributors]

@davepagurek

I've put up a pull request to add @deveshidwivedi! 🎉