Unimplemented but documented Camera functions

[mix3d]

The Camera Documentation states that the following functions are available:

applyTransform(transform) Apply a transform to the camera. The camera position, focal-point, and view-up are re-calculated using the transform’s matrix to multiply the old points by the new transform.

But this is not implemented at all.

Additionally,

getOrientation() Get the orientation of the camera (x, y, z orientation angles from the transformation matrix).

and

getOrientationWXYZ() Get the wxyz angle+axis representing the current orientation. The angle is in degrees and the axis is a unit vector.

are both implemented as () => {};

Several other functions are only half-documented, (explicitly stating a getFunction when there are both getters and setters) and some are even duplicated, (eg: roll() and roll(angle), yet does not state if the angle is in degrees or radians)

Camera functions are a pretty important part of setting up a viewer. Either state that certain functions are unimplemented or don't include them in documentation until finished.

What can I do to help?

[jourdain]

If you feel like it, and that would be of great help, you can update the api.md or improve the code in the camera by implementing the missing functions. For that you can look at the VTK/C++ counter part if need be.

[mix3d]

I'm halfway through a docs update. Even in the C++ counterpart, I can't see where Scissor is actually or referenced, outside of its getters/setters.

If they aren't used anywhere, should we drop them from the spec, to reduce API clutter?

[jourdain]

Yes, simpler is better

On Wed, Jul 3, 2019 at 7:20 PM Madison Dickson notifications@github.com wrote:

I'm halfway through a docs update. Even in the C++ counterpart, I can't see where Scissor is actually or referenced, outside of its getters/setters.

If they aren't used anywhere, should we drop them from the spec, to reduce API clutter?

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/Kitware/vtk-js/issues/1135?email_source=notifications&email_token=AACH45R7AWTYDAJTGSBC3P3P5TNV7A5CNFSM4H4IVTJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZFECHA#issuecomment-508182812, or mute the thread https://github.com/notifications/unsubscribe-auth/AACH45R54QO6JJXOCKVRNSLP5TNV7ANCNFSM4H4IVTJA .

[mix3d]

Running into more things, hopefully you can help:

Some functions that are implemented, but not called anywhere and look like they would be a Bad Idea^TM:

setViewMatrix(mat4)

• Manually sets a view matrix, without validation. Overrides subsequent calculations of getViewMatrix(), since the result of getViewMatrix() is shortcut if model.viewMatrix exists. Changes to the camera position, focalpoint, or viewUp do not update the viewMatrix. (this also breaks orthogonalizeViewUp()). setViewMatrix is not referenced anywhere.
• It seems to intend to do what computeViewParametersFromViewMatrix(vmat) actually does.
• Same concern with setProjectionMatrix(), which causes getProjectionMatrix(aspect, nearz, farz) to ignore it's input params, but IS referenced in the OpenGLRenderWindow's vrRender() function, so unsure what to do there. At least it properly sets to null. Perhaps that's the answer here, tell users in docs what it does and how to undo it.

Looking at places where camera is used, Core/Renderer, for example, I could not find how camera.render(renderer); works / where it is implemented.

Some API inconsistencies:

• getPhysicalToWorldMatrix() and getWorldToPhysicalMatrix follow the gl-matrix style of modifying the provided matrix param (a pattern which is rather un-javascript, imo), but getViewMatrix, getProjectionMatrix, and getCompositeProjectionMatrix (to name a few) return a new matrix object.

Lastly, the overall Camera implementation is a bit removed from the C++ version, whole sets of functions are new, some missing functions, and some difficult to reproduce without also implementing a local transform object. (vtkTransform, not yet implemented)

Example unreferenced and/or undocumented and new to vtk.js camera functions:

• getPhysicalToWorldMatrix, getWorldToPhysicalMatrix (Are these trying to be ComputeWorldToScreenMatrix, which is a locally stored/cached transformation matrix in C++?)
• computeViewParametersFromViewMatrix (probably doesn't need to be an exposed publicApi function)
• computeViewParametersFromPhysicalMatrix referenced by openGL/RenderWindow
• physicalOrientationToWorldDirection (used for VR I guess?)

Should these be documented, and if so, who could write their explanation and uses? I've tried to add as much as I could, but I can only do so much.

PR coming soon.

[jourdain]

The physical stuff are for VR. Regarding the rest, @martinken should be able to help.

[mix3d]

Trying to check that my docs changes are compiling correctly, but getting the following message, even against a clean fork:

Unhandled rejection WebpackOptionsValidationError: Invalid configuration object. Webpack has been initialised using a configuration object that does not match the API schema.

Any suggestions?

Update: works on a mac. Maybe a file url issue on Windows. Will look into it, but the docs compiled just fine.

[mix3d]

@martinken My first pass at updating the Camera docs has been merged, but my open Qs still stand. Did you have a chance to comment?

The API inconsistencies around matrices is still bothering me; sometimes a a length 16 array, sometimes an array of arrays, functions that modify the matrix param and functions that return a matrix instead...

[mix3d]

@jourdain Any thoughts on the need to maintain two different types of matrix formats internally?

vtkMath uses subarray matrices, while gl-matrix ( a dependency and often referenced, in ~40 files) uses flat arrays for its matrix representation.

Under what circumstances should we be uses vtkMath vs gl-matrix?

[jourdain]

vtkMath was taken "as-is" from C++ and may not have much sense in vtk.js. I would rather use the 1D array approach in vtkMath.

vtkMath is really a helper class that is widely used in VTK algo (C++), but with vtk.js we don't have any algo using it yet, so we are free to reshape vtkMath the way we would prefer.

[jourdain]

We could remove the matrix stuff from vtkMath and force the user to use gl-matrix or the matrixBuilder instead.

[aylward]

Ken mentioned that vtkMath provides methods that don't exist for gl-matrix and vtkMath provides compatibility with vtk C++.

See also #612

More discussion is needed - triaged.