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.

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

Add documentation for p5.Camera properties #5056

Closed
1 of 17 tasks
AndrewAung11 opened this issue Feb 24, 2021 · 15 comments · Fixed by #5071
Closed
1 of 17 tasks

Add documentation for p5.Camera properties #5056

AndrewAung11 opened this issue Feb 24, 2021 · 15 comments · Fixed by #5071

Comments

@AndrewAung11
Copy link

AndrewAung11 commented Feb 24, 2021

How would this new feature help increase access to p5.js?

Most appropriate sub-area of p5.js?

  • Accessibility (Web Accessibility)
  • Build tools and processes
  • Color
  • Core/Environment/Rendering
  • Data
  • DOM
  • Events
  • Friendly error system
  • Image
  • IO (Input/Output)
  • Localization
  • Math
  • Unit Testing
  • Typography
  • Utilities
  • WebGL
  • Other (specify if possible)

Feature enhancement details:

Want the code to give info about the camera position. Like camera.x, camera.y and camera.z which will really make camera object useful.

@Aloneduckling
Copy link
Contributor

Aloneduckling commented Feb 27, 2021

@stalgiag, I think we should have a function(getPosition) that will return the position of the Camera and the up vector values as an object. Similar to this.

var getVals = function(cam) {

(The code will of course be different)
Users can then access those values and use them as per their needs.

PS: I would like to work on this.

@stalgiag
Copy link
Contributor

Hi @AndrewAung11 you can get these with the centerX, centerY, centerZ properties of the camera object. For example:

let cam;

function setup() {
  createCanvas(400, 400, WEBGL);
  cam = createCamera();
  print(cam.centerX);
}

That might be necessary @Aloneduckling . It also might just be an issue of documentation. For example, the reference for p5.Camera doesn't mention the centerX, centerY, centerZ, eyeX, eyeY, eyeZ, upX, upY, upZ properties of camera object.

@AndrewAung11
Copy link
Author

AndrewAung11 commented Feb 28, 2021

Thanks @stalgiag I will try it.

@Aloneduckling
Copy link
Contributor

Aloneduckling commented Mar 1, 2021

@stalgiag, So should I go ahead and code the above function?
As for the documentation problem, I guess we should open an issue. I will do so after your confirmation.

Edit:
Here is the code for the function

//function will be called like this==>
let cam = createCam();
cam.getPosition();

p5.Camera.prototype.getPosition = function() {
        const cam = this;
  	return {
            ex: cam.eyeX,
            ey: cam.eyeY,
            ez: cam.eyeZ,
            cx: cam.centerX,
            cy: cam.centerY,
            cz: cam.centerZ,
            ux: cam.upX,
            uy: cam.upY,
            uz: cam.upZ
    };
};

@stalgiag
Copy link
Contributor

stalgiag commented Mar 1, 2021

Hi @Aloneduckling I am not sure that it makes sense to add this function. It would give an object with properties that are already directly accessible on the camera object. I think we should instead just make it more clear in the documentation that these properties exist on the camera object.

@Aloneduckling
Copy link
Contributor

@stalgiag, I agree that adding this function would be overdoing it. Updating the documentation makes more sense. I will update the docs and create a PR.

@Aloneduckling
Copy link
Contributor

@stalgiag, I have added these lines to the p5.Camera documentation.
image

is this ok?

@stalgiag
Copy link
Contributor

stalgiag commented Mar 2, 2021

@Aloneduckling that is very nicely written but it might be better to list the properties with their own entries similar to how it is done with the p5.Image reference. You can see how these are added here.

@Aloneduckling
Copy link
Contributor

@stalgiag Thanks. Also, the description for the properties is already given in the camera reference with detailed examples, so should I add an extra line with the link to it or should I write the entries separately.

@stalgiag
Copy link
Contributor

stalgiag commented Mar 3, 2021

I think it still makes sense to add the properties to the p5.Camera documentation separately. Those values are listed as parameters for the camera() function in the camera reference but this doesn't make it clear that when you create a p5.Camera with createCamera(), the resulting object will have those properties. camera() does not return a p5.Camera so there is a pretty big detachment between these two concepts for someone browsing the documentation.

@stalgiag stalgiag changed the title Need Enhancement for camera object. Add documentation for p5.Camera properties Mar 3, 2021
@Aloneduckling
Copy link
Contributor

Ok, I will add the properties separately.

@Aloneduckling
Copy link
Contributor

Aloneduckling commented Mar 4, 2021

@stalgiag, I have added some properties please have a look, if this is okay then I will finish with the rest.
image

image

image

PS:
Regarding the examples for centerX/Y/Z properties:

let cam;
function setup(){
  createCanvas(100, 100, WEBGL);
  background(0)
  cam = createCamera();
  cam.centerX = -50;
  setCamera(cam);
}
function draw(){
   box(50);
}

This code doesn't seem to be working when centerX/Y/ Z properties are used. So for these examples, I think we should print the values unless there is a way to make them work.

@Aloneduckling
Copy link
Contributor

Aloneduckling commented Mar 5, 2021

Update: For now I have added the following example which outputs a div with the contents of centerX/Y/Z properties and I did the same for upX/Y/Z properties.

image

@stalgiag
Copy link
Contributor

stalgiag commented Mar 5, 2021

Great job @Aloneduckling. The way that you are adding documentation looks good. The examples should be read-only. There are methods in place for setting these values that are safer. For example, the centerX, Y, Z docs can show the output for current camera center coordinates when the canvas is being manipulated by orbitControls() that way someone can see how they might read these values in a useful way in their sketch.

@Aloneduckling
Copy link
Contributor

@stalgiag thanks, I will now create a PR after adding the necessary changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants