Addresses issue #6587

[Garima3110]

Addresses issue: Use Cubemaps for Light Feature Performance Improvement #6587

Changes to this solution are still in progress. I would love to have suggestions to this . Thank you..!

PR Checklist

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

[davepagurek]

@diyaayay looks like the PR is up here! I'm also going to give it a review soon when I get the chance, but it would be great to get your take on it too!

[Garima3110]

Thanks a lot @davepagurek for these suggestions ! I'll just look into these and get back to you soon.

[Garima3110]

Once that's working, it would also be great to put that up in a p5 editor sketch so people looking at the PR can also test it out!

Yeah sure @davepagurek ..!Thankyou for the suggestions , I'll just make these changes and get back to you soon :-)

[Garima3110]

Once that's working, it would also be great to put that up in a p5 editor sketch so people looking at the PR can also test it out!

Hey @davepagurek ! I tried running this test sketch after building p5.js

let cubemap;
function preload() {
  // Load cubemap texture images
  let imgPaths = [
    '1/px.png',
    '1/nx.png',
    '1/py.png',
    '1/ny.png',
    '1/pz.png',
    '1/nz.png'
  ];

  cubemap = [];
  for (let i = 0; i < 6; i++) {
    cubemap[i] = loadImage(imgPaths[i]);
  }
}

function setup() {
  createCanvas(100, 100, WEBGL);
}
function draw() {
  background(220);
  imageMode(CENTER);
  push();
  translate(0, 0, -200);
  scale(2);
  image(cubemap[2], 0, 0, width, height);
  pop();
  ambientLight(50);
  const cubemapTexture=new CubemapTexture(WEBGL,cubemap,{});
  imageLight(cubemapTexture);
  specularMaterial(20);
  noStroke();
  scale(2);
  rotateX(frameCount * 0.005);
  rotateY(frameCount * 0.005);
  box(25);
}

Sketch: https://editor.p5js.org/Garima3110/sketches/TSRG5grku And got this on the console window:

[davepagurek]

I think you'll need to upload lib/p5.js to your sketch, and then edit index.html in your sketch to reference your uploaded file rather than the CDN's file.

Also, I think the idea is for the cubemap texture to only be used internally, so you'd still preload just one equirectangular image, and pass that into imageLight(). But then when it creates the diffused texture, it would be stored internally as a cubemap.

[Garima3110]

This is the sketch now, please have a look , and let me know if there are some other suggestions. https://editor.p5js.org/Garima3110/sketches/CeG_DQ1n5

hii @davepagurek :-) I am not able to figure out why exactly are the tests failing even after trying out this sketch could you please provide some insights on this. It would be very helpful for me ..Thanks a lot..!

[Garima3110]

This is the sketch now, please have a look , and let me know if there are some other suggestions. https://editor.p5js.org/Garima3110/sketches/CeG_DQ1n5

hii @davepagurek :-) I am not able to figure out why exactly are the tests failing even after trying out this sketch could you please provide some insights on this. It would be very helpful for me ..Thanks a lot..!

I apologise directly pinging you again @davepagurek ! But could you please help me out in this?!

[Garima3110]

Hey @davepagurek ! (I apologise if this approach did sound silly.) I tried reordering some code in RendererGL file and did a few other changes to get rid of the error of this being undefined and this problem got solved too. But while doing this , the filterShaderVert and filterShaderFrags not being used but declared , give the eslint errors. Should I just disable ESLint for those specific lines , or do you have other suggestions in this regard ?!

[Garima3110]

Also, this is what I'm getting right now : https://editor.p5js.org/Garima3110/sketches/CeG_DQ1n5

[Garima3110]

Thankyou @davepagurek for your suggestions, but that fragment shader error unfortunately didn't get resolved. i'll just again have a look at what's going wrong here ..

[Garima3110]

@davepagurek I fixed some errors somehow, now I'm stuck with this one. Could you give some suggestion on this one too?! Thankyou. Sketch with the latest build: https://editor.p5js.org/Garima3110/sketches/CeG_DQ1n5

[davepagurek]

When I run the code I'm getting a slightly different error in the console:

Looks like it's coming from this:

var captureProjection = matrixInstance.mat4.perspective(matrixInstance.mat4.get(), Math.PI / 2, 1, 0.1, 10);

It looks like that bit of the sketch isn't in your code in this PR, is the PR or the sketch possibly using out of date code?

[Garima3110]

It looks like that bit of the sketch isn't in your code in this PR, is the PR or the sketch possibly using out of date code?

@davepagurek Thanks a lot for pointing this out..!

I apologise deeply for the confusion here, and for the delay too.. here are the changes:

Sketch with the latest build: https://editor.p5js.org/Garima3110/sketches/CeG_DQ1n5

[davepagurek]

Looks like it's hitting an error here because gl is undefined:

I think that's because it's expecting a renderer to be passed in, but we're passing in a p5 instance instead when we do cubemapTexture=new CubemapTexture(this._pInst,faces, {});. Maybe try cubemapTexture=new CubemapTexture(this,faces, {}); instead (with just this as the first argument)?

[Garima3110]

Sorry for the extreme delay @davepagurek ! No error now, But the desired result is missing, Sketch with the latest build :https://editor.p5js.org/Garima3110/sketches/20tKjsTup I am looking for the cause of the same, if you have any suggestions please do let me know, Thankyou!

[Garima3110]

Sketch with the latest changes: https://editor.p5js.org/Garima3110/sketches/20tKjsTup Unfortunately, a lot of errors this time! but just wanted to show what I am upto..!

[perminder-17]

From my honest opinion I feel like you don't need to work anymore on the shaders part. vertex shaders looks good, Also fragment shaders looks correct to me. The only thing I want to recheck is, are we getting the correct uniforms in fragment shaders from the CPU?