Opened 9 months ago
Closed 4 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: |
Description (last modified by )
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 9 months ago by
comment:2 Changed 9 months ago by
- Type changed from PLEASE CHANGE to defect
comment:3 Changed 9 months ago by
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 9 months ago by
- Description modified (diff)
comment:5 Changed 9 months ago by
- Branch set to public/32560
- Commit set to 36813911e27d5c31842016a28fe582c03e28497a
- Description modified (diff)
- Status changed from new to needs_review
comment:6 Changed 9 months ago by
- Commit changed from 36813911e27d5c31842016a28fe582c03e28497a to 89f101edbae884df2d4e41016e0ea7ca3f8befed
Branch pushed to git repo; I updated commit sha1. New commits:
89f101e | consider the correct diameter
|
comment:7 Changed 8 months ago by
- 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 8 months ago by
- Description modified (diff)
comment:9 Changed 8 months ago by
- Description modified (diff)
comment:10 Changed 7 months ago by
- 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 4 months ago by
- Cc egourgoulhon paulmasson added
comment:12 Changed 4 months ago by
- Reviewers set to Matthias Koeppe
- Status changed from needs_review to positive_review
This is a nice improvement.
comment:13 Changed 4 months ago by
- Branch changed from public/32560 to 89f101edbae884df2d4e41016e0ea7ca3f8befed
- Resolution set to fixed
- Status changed from positive_review to closed
This example shows the distortion in a more conspicuous way:
Apparently this automatic distortion was inserted as a hack to deal with some specific example that looked bad? It should be removed!