Skip to content

Conversation

@aceslowman
Copy link
Contributor

Changes:

  • In the Passing Shader Uniforms example I've fixed an issue where the geometry was being displayed mostly off-screen. It should now be centered.
  • The Primitives example was missing an ellipsoid shape.
  • Multiple Lights example was missing a spotLight().

Additionally I added describe() in each of these example sketches.

For GSOC 2022
mentor: @kjhollen

@Qianqianye Qianqianye requested a review from kjhollen October 10, 2022 21:34
Copy link
Member

@kjhollen kjhollen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! I'm going to build this locally, just to make sure everything runs, then I'll merge.

Copy link
Member

@kjhollen kjhollen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one question from running code—see below.

void main() {

vec2 center = resolution * 1.0; // draw the shape at the center of the screen
vec2 center = resolution * 0.5; // draw the shape at the center of the screen
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aceslowman hmm, this doesn't appear in the center of the sketch for me. it's in the lower left quadrant. using just resolution (or the * 1.0) works. is that expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's strange! Just checked on my end, and it's displaying fine for me, in the middle of the screen. Also made sure that it was getting the updated uniforms.frag. Any clue why this might be different between our machines?

p5Capture2

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, just caught some issue with line-endings, it wasn't loading for me until I changed the updated example files from CRLF to LF, otherwise I was getting 404s and the files weren't being properly assembled. I thought that the .gitattributes PR resolved this, not sure what is going on.

@Qianqianye
Copy link
Contributor

Thanks @aceslowman and @kjhollen for working on this. Please let us know if you need any help to get this PR merged.

@kjhollen
Copy link
Member

kjhollen commented Feb 1, 2023

@aceslowman I think I figured out what's happening with the center item in the shader uniforms example. I must have been looking at the example on my laptop, which has a 2x retina display. I was able to get the object to center by changing the code to:

theShader.setUniform('resolution', [width * displayDensity(), height * displayDensity()]);

If you make this change, I think we'll be ready to merge from there.

@aceslowman
Copy link
Contributor Author

aceslowman commented Feb 4, 2023

@kjhollen Just committed the fix you described and it seems to work well on my end. Quick question, I merged 'main' before making this commit, should I have held off on that?

@kjhollen
Copy link
Member

kjhollen commented Feb 6, 2023

@aceslowman should be fine, I'll build this on my end before I merge it just to be sure!

Copy link
Contributor

@davepagurek davepagurek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The density fix looks good!

@davepagurek davepagurek merged commit 87bfc58 into processing:main Oct 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants