Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

added createModel() to enable loading stl/obj files from a plaintext string within editor #6936

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

added createModel() to enable loading stl/obj files from a plaintext string within editor #6936

wants to merge 3 commits into from

Conversation

mathewpan2
Copy link

Resolves #6891

Changes

Added a method called createModel() within src/webgl/loading.js that allows users to call for loading a obj or stl file from a plain text string within the editor. It includes the same options as the loadModel() function, as well as using p5._validateParameters('loadModel', arguments); to check for arguments (I wasn't sure it warranted adding a separate check for 'createModel' since they share the same arguments).

Wanted to make sure this implementation was acceptable before adding unit tests.

Screenshots of the change

const octahedron_model = `
v 0.000000E+00 0.000000E+00 40.0000
v 22.5000 22.5000 0.000000E+00
v 22.5000 -22.5000 0.000000E+00
v -22.5000 -22.5000 0.000000E+00
v -22.5000 22.5000 0.000000E+00
v 0.000000E+00 0.000000E+00 -40.0000

f     1 2 3
f     1 3 4
f     1 4 5
f     1 5 2
f     6 5 4
f     6 4 3
f     6 3 2
f     6 2 1
f     6 1 5
`

//draw a spinning octahedron
let octahedron;

function preload() {
  octahedron = createModel(octahedron_model, {type: 'obj'});
}

function setup() {
  createCanvas(100, 100, WEBGL);
  describe('Vertically rotating 3-d octahedron.');
}

function draw() {
  background(200);
  rotateX(frameCount * 0.01);
  rotateY(frameCount * 0.01);
  model(octahedron);
}

Given the sampe code above, this successfully generates a rotating 3D model from the string:

picture

PR Checklist

@welcome Welcome
Copy link

welcome bot commented Apr 3, 2024

🎉 Thanks for opening this pull request! Please check out our contributing guidelines if you haven't already. And be sure to add yourself to the list of contributors on the readme page!

davepagurek
davepagurek reviewed Apr 22, 2024
View reviewed changes
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.

Thanks for taking this on, it's looking good so far!

src/webgl/loading.js Outdated
}
}
const model = new p5.Geometry();
model.gid = `octa.obj|${normalize}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this might lead to conflicting IDs if you make more than one. Can we maybe make a global counter that increments each time this is called so that every object gets a unique gid?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah definitely something I missed, took your suggestion and added the global counter. Set up the gid as model.gid = `${fileType}|${normalize}|${modelCounter++}`;, let me know if you think this is clear enough.

src/webgl/loading.js Outdated
model.normalize();
}

if (flipU) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like these steps get called regardless of the type of the model. Maybe we can make the if statement by inside the try so that we can call flipU(), flipV(), and the success callback in one spot instead of duplicating the code?

Copy link
Author

Choose a reason for hiding this comment

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

I was trying to follow the convention of loadModel(), where they also call those statements twice, but I agree that it's cleaner if it's only called once at the end.

src/webgl/loading.js Outdated
* rendered without color properties.
*
* Options can include:
* - `path`: Specifies the plain text string of either an stl or obj file to be loaded.
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than specifying a path, maybe we can make the fileType mandatory and place it after the source string for all the different overloads for this method?

Copy link
Author

Choose a reason for hiding this comment

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

I added the filetype to the argument and fixed some of the overloads, let me know if you like it more now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davepagurek davepagurek davepagurek left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow loading of models from strings
2 participants
@mathewpan2 @davepagurek