Opened 8 months ago

Closed 2 months ago

#32560 closed defect (fixed)

Add auto_scaling option to threejs plots

Reported by: gh-LaisRast Owned by:
Priority: minor Milestone: sage-9.6
Component: graphics Keywords: threejs, aspect_ratio
Cc: slelievre, egourgoulhon, paulmasson Merged in:
Authors: Laith Rastanawi Reviewers: Matthias Koeppe
Report Upstream: N/A Work issues:
Branch: 89f101e (Commits, GitHub, GitLab) Commit: 89f101edbae884df2d4e41016e0ea7ca3f8befed
Dependencies: Stopgaps:

Status badges

Description (last modified by slelievre)

Up to Sage 9.5.beta4, threejs_template.html can automatically scale down the z-axis if the last entry of aspect_ratio is set to 1 (whether because the user did not specify a value, or because the user specified a value identical to the default value) and the plot extends a lot more in the z direction than in the x and y directions.

For instance, the following example

sage: cubes = sum([cube((0, 0, i)) for i in range(0, 6)])
sage: cubes.show(aspect_ratio=[1, 1, 1])

shows a stack of cubes squeezed along the z-axis, rather than a stack of undistorted cubes.

This behavior is unexpected after explicitly setting the aspect ratio.

In the file threejs_template.html, the lines responsible for this behavior are:

    var autoAspect = 2.5;
    if ( zRange > autoAspect * rRange && a[2] === 1 ) a[2] = autoAspect * rRange / zRange;

That automatic scaling down was introduced in #12402. Quoting from that ticket:

There is already automatic scaling in the z-direction to avoid the tall thin box that appears in the current version on the cell server (and SMC), but that can easily be overridden.

It is worth mentioning that one way to overwrite this behavior is to do:

sage: plot.show(aspect_ratio=[1, 1, 0.99999])

This ticket fixes this behavior and introduces a new threejs-specific plot option.

The new option is called auto_scaling (default: [False, False, False]) and takes a list or a tuple of three booleans; set to True to automatically scale down the corresponding direction if it is too large.

Change History (13)

comment:1 Changed 8 months ago by guenterrote

This example shows the distortion in a more conspicuous way:

sage: plot = sum([cube((0,0,i)) for i in range(0,10,2)])
....: plot.show(aspect_ratio=[1,1,1])

Apparently this automatic distortion was inserted as a hack to deal with some specific example that looked bad? It should be removed!

If such an automatic scaling feature is at all desired, it should be triggered by something like aspect_ratio=[1,1,"automatic"]

Last edited 8 months ago by guenterrote (previous) (diff)

comment:2 Changed 8 months ago by guenterrote

  • Type changed from PLEASE CHANGE to defect

comment:3 Changed 8 months ago by gh-LaisRast

It turns out this behavior is documented. In the current version of reference/plot3d/threejs.html (see page https://doc.sagemath.org/html/en/reference/plot3d/threejs.html) it says

aspect_ratio – (default: [1,1,1]) list or tuple of three numeric values; z-aspect is automatically reduced when large but can be overridden

However, I still believe it should not be the default behavior. Following Günter's suggestion, we can do

    var autoAspect = 2.5;
-    if ( zRange > autoAspect * rRange && a[2] === 1 ) a[2] = autoAspect * rRange / zRange;
+    if ( zRange > autoAspect * rRange && a[2] === "automatic" ) a[2] = autoAspect * rRange / zRange;

comment:4 Changed 8 months ago by gh-LaisRast

  • Description modified (diff)

comment:5 Changed 7 months ago by gh-LaisRast

  • Authors set to Laith Rastanawi
  • Branch set to public/32560
  • Commit set to 36813911e27d5c31842016a28fe582c03e28497a
  • Description modified (diff)
  • Status changed from new to needs_review

comment:6 Changed 7 months ago by git

  • Commit changed from 36813911e27d5c31842016a28fe582c03e28497a to 89f101edbae884df2d4e41016e0ea7ca3f8befed

Branch pushed to git repo; I updated commit sha1. New commits:

89f101econsider the correct diameter

comment:7 Changed 7 months ago by slelievre

  • Cc slelievre added
  • Description modified (diff)
  • Summary changed from unexpected behavior of aspect_ratio in threejs template to Add auto_scaling option to threejs plots

comment:8 Changed 7 months ago by gh-LaisRast

  • Description modified (diff)

comment:9 Changed 7 months ago by slelievre

  • Description modified (diff)

comment:10 Changed 5 months ago by mkoeppe

  • Milestone changed from sage-9.5 to sage-9.6

Stalled in needs_review or needs_info; likely won't make it into Sage 9.5.

comment:11 Changed 3 months ago by mkoeppe

  • Cc egourgoulhon paulmasson added

comment:12 Changed 3 months ago by mkoeppe

  • Reviewers set to Matthias Koeppe
  • Status changed from needs_review to positive_review

This is a nice improvement.

comment:13 Changed 2 months ago by vbraun

  • Branch changed from public/32560 to 89f101edbae884df2d4e41016e0ea7ca3f8befed
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.