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: Consolidate Surface Code #27568

Closed
paulmasson mannequin opened this issue Mar 29, 2019 · 11 comments
Closed

Three.js: Consolidate Surface Code #27568

paulmasson mannequin opened this issue Mar 29, 2019 · 11 comments
Labels
c: graphics p: major / 3 t: enhancement
Milestone
sage-8.8

Comments

@paulmasson
Copy link
Mannequin

paulmasson mannequin commented Mar 29, 2019

For efficiency of future updates, the code for adding surfaces to scenes is consolidated in this commit. That way there will only be one function to adjust as needed. In fact part of this code will be changed when addressing #26718.

CC: @egourgoulhon @fchapoton

Component: graphics

Keywords: threejs

Author: Paul Masson

Branch/Commit: 53b69df

Reviewer: Eric Gourgoulhon

Issue created by migration from https://trac.sagemath.org/ticket/27568
@paulmasson paulmasson mannequin added this to the sage-8.8 milestone Mar 29, 2019
@paulmasson paulmasson mannequin added the p: major / 3 label Mar 29, 2019
@paulmasson
Copy link
Mannequin Author

paulmasson mannequin commented Mar 29, 2019

Author: Paul Masson

@paulmasson paulmasson mannequin added c: graphics labels Mar 29, 2019
@paulmasson
Copy link
Mannequin Author

paulmasson mannequin commented Mar 29, 2019

Branch: u/paulmasson/three_js___consolidate_surface_code

@paulmasson
Copy link
Mannequin Author

paulmasson mannequin commented Mar 29, 2019

New commits:

53b69dfConsolidate surface code

@paulmasson
Copy link
Mannequin Author

paulmasson mannequin commented Mar 29, 2019

Commit: 53b69df

@paulmasson

This comment has been minimized.

Sign in to view
@paulmasson paulmasson mannequin changed the title Three.js - Consolidate Surface Code Three.js: Consolidate Surface Code Mar 29, 2019
@paulmasson paulmasson mannequin added the s: needs review label Mar 29, 2019
@paulmasson

This comment has been minimized.

Sign in to view
@egourgoulhon
Copy link
Member

comment:5

LGTM (tested with dodecahedron() and plot3d).

@egourgoulhon
Copy link
Member

Reviewer: Eric Gourgoulhon

@embray
Copy link
Contributor

embray commented Apr 2, 2019

Changed keywords from none to threejs

@embray
Copy link
Contributor

embray commented Apr 2, 2019

comment:7

Glad to see this already done. I was just thinking of doing the same.

@vbraun
Copy link
Member

vbraun commented Apr 3, 2019

Changed branch from u/paulmasson/three_js___consolidate_surface_code to 53b69df

@vbraun vbraun closed this as completed in e2af051 Apr 3, 2019
@slel slel mentioned this issue May 6, 2019
Closed
@embray embray mentioned this issue Jan 29, 2023
Open
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: enhancement
Projects
None yet
Milestone
sage-8.8
Development

No branches or pull requests
3 participants
@embray @vbraun @egourgoulhon