Skip to content

Solves issue #6891#7165

Merged
davepagurek merged 3 commits into
processing:mainfrom
Garima3110:createModel
Aug 20, 2024
Merged

Solves issue #6891#7165
davepagurek merged 3 commits into
processing:mainfrom
Garima3110:createModel

Conversation

@Garima3110
Copy link
Copy Markdown
Member

Resolves #6891

Changes:
This is a continuation of the work initiated by @mathewpan2 in the PR #6936

I would love to have further suggestions to the work.
Thankyou!

PR Checklist

@Qianqianye Qianqianye requested a review from davepagurek August 16, 2024 23:07
Copy link
Copy Markdown
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.

Thanks for getting the tests passing on this! Just some comments on the documentation, and then this looks good!

Comment thread src/webgl/loading.js Outdated
/**
* Load a 3d model from an OBJ or STL string.
*
* <a href="#/p5/loadModel">createModel()</a> should be placed inside of <a href="#/p5/preload">preload()</a>.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we don't need this sentence, since loading from a string means we don't have to asynchronously load the source code and everything happens immediately.

Comment thread src/webgl/loading.js Outdated
* let octahedron;
*
* function preload() {
* octahedron = createModel(octahedron_model);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Similar to above, we can move this to setup to avoid confusion about how loading works.

Comment thread src/webgl/loading.js Outdated
*
* function setup() {
* createCanvas(100, 100, WEBGL);
* describe('Vertically rotating 3-d octahedron.');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Small update for consistency of language across our docs:

Suggested change
* describe('Vertically rotating 3-d octahedron.');
* describe('Vertically rotating 3D octahedron.');

@davepagurek
Copy link
Copy Markdown
Contributor

oh do we want to keep in the description in the docs of the options object, since we still allow you to pass in things like flipU?

@Garima3110
Copy link
Copy Markdown
Member Author

oh do we want to keep in the description in the docs of the options object, since we still allow you to pass in things like flipU?

yeah we should , that was a mistake from my side thanks for noticing that..!

Copy link
Copy Markdown
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.

Thanks, looks good!

@davepagurek davepagurek merged commit 0312e44 into processing:main Aug 20, 2024
@Garima3110 Garima3110 deleted the createModel branch August 21, 2024 05:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow loading of models from strings

2 participants