[fricas-devel] BUG: viewpoint$ThreeDimensionalViewport + METAPATCHES + SUGGESTED FEATURE

[Riccardo GUIDA]

I'm sorry I do not yet have a developer framework setup (virtual machine, git ...) so you must content yourselves with an untested METAPATCH. ric

PROBLEM:
Crazy behavior of viewpoint function invoked by command line.

BUG:
Evident confusion among radians and degrees in the implementation of viewpoint.

CONVENTIONS:
A viewpoint is a data record inside ThreeDimensionalViewport and has type

V ==> Record( theta : SF, phi : SF, scale : SF, scaleX : SF, scaleY : SF, scaleZ : SF, deltaX : SF, deltaY : SF )
https://github.com/fricas/fricas/blob/master/src/algebra/view3D.spad#L39

where, believing to the ++ descriptions, theta is the longitude and phi the latitude (Mmmm... physicists use the opposite convention).

Conversion factors degrees to radians are defined by
degrees := pi()$F / 180.0
degreesSF := pi()$SF / 180
https://github.com/fricas/fricas/blob/master/src/algebra/view3D.spad#L401

From the use of the most primitive function rotate one understands that theta and phi in V are in radians and Float
https://github.com/fricas/fricas/blob/master/src/algebra/view3D.spad#L801

I do not really understand the difference among the detailed actions
of rotate (line 801) and viewpint (Line 703) because depend on the server.
I *guess* that for given theta, phi they act in the same way, while viewpoint (Line 703) updates all the fields.

It would be nice to double check these conventions but I'm unable to dive in ViewportServer$Lisp

METAPATCHES:

1) https://github.com/fricas/fricas/blob/master/src/algebra/view3D.spad#L728

radian/degree mismatch: delete the two occurrences of * degreesSF below:

viewpoint (viewport : %, Theta : F, Phi : F) : Void ==
viewport.viewpoint.theta := convert(Theta)@SF * degreesSF
viewport.viewpoint.phi := convert(Phi)@SF * degreesSF

2)
https://github.com/fricas/fricas/blob/master/src/algebra/view3D.spad#L742

2.1: Replace 180.0 by %pi and 90.0 by %pi/2 below
2.2: The two occurrences of * degrees in rotate must be deleted.
(Modification 2.2 is superseeded by 4.2 below)

viewpoint (viewport : %, X : F, Y : F, Z : F) : Void ==
Theta : F
Phi : F
if (X = 0$F) and (Y = 0$F) then
Theta := 0$F
if (Z>=0$F) then
Phi := 0$F
else
Phi := 180.0
else
Theta := asin(Y/(R := sqrt(X*X+Y*Y)))
if (Z = 0$F) then
Phi := 90.0
else
Phi := atan(Z/R)
rotate(viewport, Theta * degrees, Phi * degrees)

2.3 The corresponding ++ description is wrong
https://github.com/fricas/fricas/blob/master/src/algebra/view3D.spad#L242

The corresponding ++ description is wrong:

viewpoint : (%, F, F, F) -> Void
++ viewpoint(v, rotx, roty, rotz) sets the rotation about the x-axis
++ to be \spad{rotx} radians, sets the rotation about the y-axis
++ to be \spad{roty} radians, and sets the rotation about the z-axis
++ to be \spad{rotz} radians, for the viewport v, which is of the
++ domain \spadtype{ThreeDimensionalViewport} and displays v with
++ the new view position.

If I understand correctly the new description should be

viewpoint : (%, F, F, F) -> Void
++ viewpoint(v, x, y, z) sets the viewpoint to cartesian coordinates (x,y,z)
++ for the viewport v, which is of the
++ domain \spadtype{ThreeDimensionalViewport} and displays v with
++ the new view position.

3)
You may safely delete the lines 425 to 431 because arcsinTemp and arctanTemp are unused.
(Can one call them from other files?)

https://github.com/fricas/fricas/blob/master/src/algebra/view3D.spad#L425

4) SUGGESTED FEATURE: let ALL viewpoint functions to force replotting the viewport.

4.1) for the viewpoint at line 720, 724: add at the end the line
viewpoint(viewport,viewport.viewpoint)

4.2) for the viewpoint at line 732 the replotting should be forced, but to be homogeneous I would replace

rotate(viewport, Theta, Phi)

by

viewpoint (viewport, Theta, Phi)

4.3) Delete in the ++ strings the statements
++ The new
++ dimensions are not displayed until the function
++ \spadfun{makeViewport3D} is executed again for v.

[Waldek Hebisch]

Post by Riccardo GUIDA
I'm sorry I do not yet have a developer framework setup (virtual machine, git ...) so you must content yourselves with an untested METAPATCH. ric
Crazy behavior of viewpoint function invoked by command line.
Evident confusion among radians and degrees in the implementation of viewpoint.
A viewpoint is a data record inside ThreeDimensionalViewport and has type
V ==> Record( theta : SF, phi : SF, scale : SF, scaleX : SF, scaleY : SF, scaleZ : SF, deltaX : SF, deltaY : SF )
https://github.com/fricas/fricas/blob/master/src/algebra/view3D.spad#L39
where, believing to the ++ descriptions, theta is the longitude and phi the latitude (Mmmm... physicists use the opposite convention).
Conversion factors degrees to radians are defined by
degrees := pi()$F / 180.0
degreesSF := pi()$SF / 180
https://github.com/fricas/fricas/blob/master/src/algebra/view3D.spad#L401
From the use of the most primitive function rotate one understands that theta and phi in V are in radians and Float
https://github.com/fricas/fricas/blob/master/src/algebra/view3D.spad#L801
I do not really understand the difference among the detailed actions
of rotate (line 801) and viewpint (Line 703) because depend on the server.
I *guess* that for given theta, phi they act in the same way, while viewpoint (Line 703) updates all the fields.
It would be nice to double check these conventions but I'm unable to dive in ViewportServer$Lisp
1) https://github.com/fricas/fricas/blob/master/src/algebra/view3D.spad#L728
viewpoint (viewport : %, Theta : F, Phi : F) : Void ==
2)
https://github.com/fricas/fricas/blob/master/src/algebra/view3D.spad#L742
2.1: Replace 180.0 by %pi and 90.0 by %pi/2 below
2.2: The two occurrences of * degrees in rotate must be deleted.
(Modification 2.2 is superseeded by 4.2 below)
viewpoint (viewport : %, X : F, Y : F, Z : F) : Void ==
Theta : F
Phi : F
if (X = 0$F) and (Y = 0$F) then
Theta := 0$F
if (Z>=0$F) then
Phi := 0$F
else
Phi := 180.0
else
Theta := asin(Y/(R := sqrt(X*X+Y*Y)))
if (Z = 0$F) then
Phi := 90.0
else
Phi := atan(Z/R)
rotate(viewport, Theta * degrees, Phi * degrees)

The changes above are clear.

Post by Riccardo GUIDA
2.3 The corresponding ++ description is wrong
https://github.com/fricas/fricas/blob/master/src/algebra/view3D.spad#L242
viewpoint : (%, F, F, F) -> Void
++ viewpoint(v, rotx, roty, rotz) sets the rotation about the x-axis
++ to be \spad{rotx} radians, sets the rotation about the y-axis
++ to be \spad{roty} radians, and sets the rotation about the z-axis
++ to be \spad{rotz} radians, for the viewport v, which is of the
++ domain \spadtype{ThreeDimensionalViewport} and displays v with
++ the new view position.
If I understand correctly the new description should be
viewpoint : (%, F, F, F) -> Void
++ viewpoint(v, x, y, z) sets the viewpoint to cartesian coordinates (x,y,z)
++ for the viewport v, which is of the
++ domain \spadtype{ThreeDimensionalViewport} and displays v with
++ the new view position.

Sorry, original description is clearly wrong, but the proposed
replacement is not clear for me. To say the truth my understanding
is that the routine converts coordinates to angle(s) on the sphere,
but ATM I do not see why anybody would want such a routine.
Probably first we need to know what those angles really mean.
Saying longitude and latitude avoids real question: what is their
role in display.

Post by Riccardo GUIDA
3)
You may safely delete the lines 425 to 431 because arcsinTemp and arctanTemp are unused.
(Can one call them from other files?)
https://github.com/fricas/fricas/blob/master/src/algebra/view3D.spad#L425

Clear, they are unused.

Post by Riccardo GUIDA
4) SUGGESTED FEATURE: let ALL viewpoint functions to force replotting the viewport.

Sorry, not replotting is the feature. It is impossible to "undo"
spurious replot. On fast displays spurious replots merely waste
time but on slow ones can create ugly artifacts. So quality
plotting routines must give user control of replotting.

[Riccardo GUIDA]

Post by Waldek Hebisch
Sorry, not replotting is the feature. It is impossible to "undo"
spurious replot. On fast displays spurious replots merely waste
time but on slow ones can create ugly artifacts. So quality
plotting routines must give user control of replotting.

The main point of the proposal was homogeneity of user interface (UI). Presently viewpoint(vp,1,2,3) and rotate(vp,3.14/4.0,3.14/4.0) actually redraw so homogeneity of UI and your feature argument would imply to shut-off these anti-feature... but mine was a weak suggestion, so it is ok to stay here.

Some unsolicited thoughts "pour parler":

I assume that guaranteeing a user-friendly and as-simple-as-possible UI is among FriCAS mandatory targets. I hope for the survival of FriCAS that this is the case :)

Viewport UI is not very homogeneous among 2D and 3D: many names, semantics, and options are different.

I'm happy to see that there are other packages in development (Scene and GnuDraw) but again new UI and, especially for Scene, new concepts and new names (sadly violating the full-words naming convention used in Axiom/FriCAS).

Would not be useful to have a few categories that impose a minimal "2D, 3D scientific plotting" UI that past, present, and future drawing packages must honor (and of course extend if they wish)? This way a user might smoothly move from one package to another, and learn the whole details of a package if more functionalities were needed. It would encourage developers to test new ways without burdening the users.

Just as a stupid example (with no pretensions, one has to think carefully to names, functionalities, options,....)

Plot2DShowCategory():Category with
plot2DFunction
plot2DCurve
plot2DPoints
show2DPlots

Plot2DFormats ==> Union(SVG, Postscript, GnuPlot,....)

Plot2DToXCategory( F:Plot2DFormats ):Category
plot2DFunctionTo
plot2DCurveTo
plot2DPointsTo
show2DPlotsTo

... analog in 3d ...


end of "pour parler".

Post by Waldek Hebisch
but ATM I do not see why anybody would want such a routine.

Well, as a benchmark of FriCAS present scientific graphical capabilities I was trying to reproduce
Loading Image...
with the same parameters:

ViewPoint -> {-2, -2.5, 1}

ie position (x,y,z) of an observer looking toward the object.

Post by Waldek Hebisch
Probably first we need to know what those angles really mean.
Saying longitude and latitude avoids real question: what is their
role in display.

You are right. The problem is that src/graph is a jungle...

I'm wondering if the time that it would take to understand all the details of src/graph and maintain it is not comparable to that that an experienced C/C++ programmer (which excludes me) could take to reconstruct from scratch a minimal viewport interface (possibly with more than 9 plots allowed and vector graphics...), maybe extracting it form an open source project, ie the Qt based https://en.wikipedia.org/wiki/FreeMat

Anyway, at a first sight I would say that a necessary condition is the understanding of the omnipresent global macro ROTATE and friends:

void
ROTATE(float xxR[4][4])
{
xxR[0][0]= -(cosTheta); xxR[0][1]= -(-sinTheta*cosPhi); xxR[0][2]= -(sinTheta*sinPhi); xxR[0][3]= 0.0;
xxR[1][0]= -(sinTheta); xxR[1][1]= -(cosTheta*cosPhi); xxR[1][2]= -(-cosTheta*sinPhi); xxR[1][3]= 0.0;
xxR[2][0]= 0.0; xxR[2][1]= -(sinPhi); xxR[2][2]= -(cosPhi); xxR[2][3]= 0.0;
xxR[3][0]= 0.0; xxR[3][1]= 0.0; xxR[3][2]= 0.0; xxR[3][3]= -(1.0);
}

https://github.com/fricas/fricas/blob/master/src/graph/view3D/transform3d.c#L90


The problem is that a search gives more definitions of these sinTheta/cosTheta/...:
https://github.com/fricas/fricas/search?q=cosTheta&unscoped_q=cosTheta

I guess that the one which interest us are in
https://github.com/fricas/fricas/blob/master/src/graph/view3D/viewport3d.c

more on this later....

Riccardo