p5.js icon indicating copy to clipboard operation
p5.js copied to clipboard

new getCamera function for V1 & V2

Open quarks opened this issue 3 months ago • 10 comments

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

quarks avatar Sep 19 '25 11:09 quarks

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 getCamera function to mirror the existing setCamera method, as you've suggested. There's some precedent for this, e.g. setBlue on p5.Color.
  • Create a new joint getter/setter, e.g. activeCamera(), where calling it with no params returns the active camera, and activeCamera(cam) does what setCamera currently does. We'd have to keep but deprecate setCamera as 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 createCamera method 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.

davepagurek avatar Sep 19 '25 11:09 davepagurek

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.

limzykenneth avatar Sep 19 '25 13:09 limzykenneth

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.

quarks avatar Sep 19 '25 13:09 quarks

Good catch! Just put up a PR to correct that.

davepagurek avatar Sep 19 '25 16:09 davepagurek

Is anyone currently working on this? If not, I'd like to take it on. This would be my first contribution to open source.

ombalgude avatar Sep 23 '25 05:09 ombalgude

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.

davepagurek avatar Sep 23 '25 09:09 davepagurek

is this being assigned to anyone ? if not, then i am also willing to contribute to this

Ptmishra69 avatar Nov 15 '25 18:11 Ptmishra69

@ombalgude has a PR in progress currently. @ombalgude , are you still working on this?

davepagurek avatar Nov 15 '25 23:11 davepagurek

@davepagurek & @Ptmishra69 I'm working on it

ombalgude avatar Nov 16 '25 05:11 ombalgude

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!

ombalgude avatar Nov 28 '25 06:11 ombalgude