Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

incorrect normalization of cos(rotation), sin(rotation) in world coordinate transforms #152

Open
snoeyink opened this issue May 26, 2022 · 1 comment

Comments

@snoeyink
Copy link

snoeyink commented May 26, 2022

World coordinate transformation calculations based on rotation/tilt are not just rotating, but also incorrectly scaling axes based on these angles.
This would appear to be the source of Plots.jl issues 1911 & 3445. The documentation for gr_setspace suggests that both rotation and tilt angles be limited to [0,90] (also suggested for Python and Ruby). Julia's PLots.jl now enforces this for the GR.jl backend but this 1) should not be necessary for rotation, and 2) still has objects shrinking from rotation 0 to 45, before growing again to rotation 90.
Consider the net effect of these lines: gr/gr.c:1200-1208.

  r = arc(rotation);
  wx.a1 = cos(r);
  wx.a2 = sin(r);
  a = (xmax - xmin) / (wx.a1 + wx.a2);
  wx.b = xmin;
  wx.a1 = a * wx.a1 / (xmax - xmin);
  wx.a2 = a * wx.a2 / (ymax - ymin);

Since constants xmax-xmin = ymax-ymin = 1, this sets

wx.a1 = cos(r) / (cos(r) + sin(r))
wx.a2 = sin(r) / (cos(r) + sin(r))

Note how this would divide by zero at rotation = 135 and 215, and be negative for angles in between. For 45 degrees, it divides by 1.412; for pure rotation, one should normalize instead by sqrt(cos(r)^2 + sin(r)^2) = 1, so I believe the correction is just to remove the division by the sum of a1+a2. I'd be happy to correct this and make a pull request, but I'm not sure I know what is intended geometrically. The tilt normalization will also need similar adjustment, as will code in at least two other places: gr3/gr3_gr.c:183-200 and gr/gr.c:7199-7204, which also takes absolute values that will cause discontinuities not in the other projections.

@jheinen
Copy link
Collaborator

jheinen commented May 27, 2022

Thank you very much for the suggestion.

In the meantime, there is an improved 3D representation in GR3, but I have not yet incorporated it in Plots.jl. The limitation regarding the angles no longer exists here, and a perspective projection is also supported.

Here is an example:

using GR
surface(peaks())

Screenshot 2022-05-27 at 09 04 53

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants