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.

Sign up for GitHub

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

Jump to bottom

Three.js: add aspect_ratio multipliers to text3d #22669

Closed
paulmasson mannequin opened this issue Mar 21, 2017 · 6 comments
Closed

Three.js: add aspect_ratio multipliers to text3d #22669

paulmasson mannequin opened this issue Mar 21, 2017 · 6 comments
Labels
c: graphics p: major / 3 t: bug
Milestone
sage-8.0

Comments

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Mar 21, 2017

The multipliers entered using the aspect_ratio keyword are currently not applied to the positions of text3d objects. This is a simple fix that can be tested with the graphics object

sum([text3d('%.1f'%n, (cos(n),sin(n),n), viewer='threejs') for n in [0,0.2,..,8]])

because the viewer has automatic scaling in the z-direction that is not applied to the spiral of numbers.

CC: @novoselt @egourgoulhon

Component: graphics

Author: Paul Masson

Branch/Commit: 1d10aa0

Reviewer: Eric Gourgoulhon

Issue created by migration from https://trac.sagemath.org/ticket/22669
@paulmasson paulmasson mannequin added this to the sage-8.0 milestone Mar 21, 2017
@paulmasson paulmasson mannequin added c: graphics labels Mar 21, 2017
@paulmasson
Copy link
Mannequin Author

paulmasson mannequin commented Mar 21, 2017

Branch: u/paulmasson/22669

@paulmasson
Copy link
Mannequin Author

paulmasson mannequin commented Mar 22, 2017

Commit: 1d10aa0

@paulmasson
Copy link
Mannequin Author

paulmasson mannequin commented Mar 22, 2017

comment:2

Keep in mind that changes to the non-Python template require make even with git trac try

New commits:

1d10aa0Add missing aspect multipliers

@paulmasson paulmasson mannequin added the s: needs review label Mar 22, 2017
@egourgoulhon
Copy link
Member

comment:3

LGTM. Thanks for the fix!

@egourgoulhon
Copy link
Member

Reviewer: Eric Gourgoulhon

@vbraun
Copy link
Member

vbraun commented Apr 3, 2017

Changed branch from u/paulmasson/22669 to 1d10aa0

@vbraun vbraun closed this as completed in f2992ca Apr 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c: graphics p: major / 3 t: bug
Projects
None yet
Milestone
sage-8.0
Development

No branches or pull requests
2 participants
@vbraun @egourgoulhon