new getCamera function for V1 & V2
Increasing access
It would provide compatability between versions 1 & 2 but more importantly provide users and library creators a standard method to access, modify and set cameras.
Most appropriate sub-area of p5.js?
- [ ] Accessibility
- [ ] Color
- [ ] Core/Environment/Rendering
- [ ] Data
- [ ] DOM
- [ ] Events
- [ ] Image
- [ ] IO
- [ ] Math
- [ ] Typography
- [ ] Utilities
- [x] WebGL
- [ ] Build process
- [ ] Unit testing
- [ ] Internationalization
- [ ] Friendly errors
- [ ] Other (specify if possible)
Feature request details
I suggest a new function for both V1 and V2 that returns a reference to the current camera object, something like
// V2
RendererGL.prototype.getCamera = function(cam) {
return this.states.setValue('curCamera', cam);
}
// V1
RendererGL.prototype.getCamera = function(cam) {
return this._curCamera;
}
This would provide compatibility between versions.
Additional comment:
I see that in version 2.0.5 the createCamera method still sets the newly created camera as the default. See issues
5941, 7077 and 7102
Thanks for opening this @quarks!
Normally we do getters/setters in a joint function, e.g. rectMode() returns the current rect mode, and rectMode(CENTER) sets the current rect mode. This is hard to do for cameras though because we already have a method called camera() that does not set the active camera, but changes the position/orientation of the camera. I can see two general API directions:
- Add a
getCamerafunction to mirror the existingsetCameramethod, as you've suggested. There's some precedent for this, e.g.setBlueonp5.Color. - Create a new joint getter/setter, e.g.
activeCamera(), where calling it with no params returns the active camera, andactiveCamera(cam)does whatsetCameracurrently does. We'd have to keep but deprecatesetCameraas an alias in order to not introduce a breaking change.
@limzykenneth @ksen0 do you have thoughts/preferences here? I slightly lean towards the joint getter/setter since those vastly outnumber explicit getters/setters in our codebase, but I don't have a strong opinion -- we do have explicit ones where there are edge cases and this may qualify as one.
I suggest a new function for both V1 and V2 that returns a reference to the current camera object
@ksen0 what are your thoughts on this? We don't normally add things to v1 any more to feature freeze it. Something like this could be useful for library authors trying to write for both versions, but also one can manually handle both with a method like this https://discourse.processing.org/t/p5js-v2-get-current-camera/47172
I see that in version 2.0.5 the
createCameramethod still sets the newly created camera as the default. See issues 5941, 7077 and 7102
This line doesn't set it as the active camera, it just gives the camera default camera position/orientation/etc 🙂 https://github.com/processing/p5.js/blob/4cd09f10784e9eea37456f4e225d5c98d64d8074/src/webgl/p5.Camera.js#L3983
Probably could use a rename for clarity.
I probably lean towards a joint getter setter where possible as that is more common. Some of the exception is for example getTargetFramerate() which returns a different thing from framerate() itself. The color getter function (red(), green(), blue(), hue()) etc is also quite an exception that I don't really like, I'd prefer color objects to just use object oriented property access instead (ie. . syntax), and their implementation is full of weird edge cases and defaults so quite complicated.
Please ignore the 'additional comment' in my previous post, revisiting the code shows I was wrong and it does not set the current camera. It does mean that the V2 reference needs amending.
Good catch! Just put up a PR to correct that.
Is anyone currently working on this? If not, I'd like to take it on. This would be my first contribution to open source.
Thanks @ombalgude , go ahead! This will be on the dev-2.0 branch as it will (at least primarily? to be determined) be in p5 2.x.
is this being assigned to anyone ? if not, then i am also willing to contribute to this
@ombalgude has a PR in progress currently. @ombalgude , are you still working on this?
@davepagurek & @Ptmishra69 I'm working on it
Hi! @davepagurek I've implemented this feature and opened a PR: #8304
The PR adds activeCamera() as a joint getter/setter function following the discussion preference. It returns the current camera when called with no arguments, and sets the active camera when called with a camera argument.
Please review the changes and let me know your feedback!