ArJcptRenderer encapsulates potentially necessary fields

[michaelboyles]

ArJcptRenderer encapsulates World, Camera and FrameBuffer objects. These fields should be accessible in some way. For example, I needed all 3 of these within my activity to calculate a normal based on a user tapping the AR scene.

Either getters should be provided, or these fields should be made protected so that users can extend ArJcptRenderer and add their own getters if required.

I'm happy to implement this if you advise on the preferred approach.

[plattysoft]

Thanks for the suggestion.

The encapsulation was intentional to enforce simplicity. If you need to access those objects, you may want to extend the Renderer and do that calculation in there.

Anyway, you can get a reference to the World on configure World. That was the only case when I thought it was going to be needed.

In either case, public variables are generally a bad idea, getters are not too bad. However I think the FrameBuffer should not have a getter. For World I can see that it could be useful.

You really don't want to manipulate the camera on an AR application, everything will be messed up, that's why it is removed.

[michaelboyles]

I'm absolutely in favour of encapsulating as much as possible but this encapsulates too much.

The following snippet is an example of getting an object based on a user tap:

SimpleVector dir = Interact2D.reproject2D3DWS(camera, frameBuffer, /*x pos*/, /*y pos*/).normalize();
Object[] res = world.calcMinDistanceAndObject3D(camera.getPosition(), dir, /*tolerance*/);

It uses World, Camera, and FrameBuffer in ways that are totally valid and non-destructive. You could move this functionality into ArJcptRenderer:

public Object[] getObjectAt(int x, int y, int tolerance)
{
    SimpleVector dir = Interact2D.reproject2D3DWS(mCamera, mBuffer, x, y).normalize();
    return mWorld.calcMinDistanceAndObject3D(mCamera.getPosition(), dir, tolerance);
}

but I feel like this goes beyond the responsibility of a renderer. Also, who know what other uses people may have for those fields besides this one.

If you don't want to provide getters, I feel like the best approach is to change the fields to protected. If a user of ArJcptRenderer then wants to get access to the camera and frame buffer, they can extend the class and implement their own getters. If, by using these getters, they break the configuration somehow then it's their responsibility.

[plattysoft]

Good point, sometimes we encapsulate too much (which is hard to know until someone goes with particular use cases)

I think a getter for World makes sense, moving Camera and FrameBuffer to protected also makes perfect sense, please go for it.

I'm keen of getting the Camera hidden to avoid temptations about changing it (as opposed to what you'd normally do with a 3D Engine)

[plattysoft]

Also, I was considering doing some raycasting to get objects, to allow interacting with the augmentations, so that method may be a better fit and even something we can include as part of the library.

[plattysoft]

Thanks for the PR! I know it is a small thing, but I appreciate contributing, specially appreciate the discussion.