Opened 2 years ago
Closed 2 years ago
#26718 closed enhancement (fixed)
Upgrade to three.js r100
Reported by:  slelievre  Owned by:  

Priority:  major  Milestone:  sage8.8 
Component:  packages: standard  Keywords:  upgrade, threejs, three.js 
Cc:  arojas, fbissey, novoselt, schilly, slelievre, was, egourgoulhon  Merged in:  
Authors:  Paul Masson  Reviewers:  Eric Gourgoulhon 
Report Upstream:  N/A  Work issues:  
Branch:  18d0e74 (Commits, GitHub, GitLab)  Commit:  18d0e74679a627d411a4e596d58072047030abe5 
Dependencies:  #27568  Stopgaps: 
Description (last modified by )
This ticket is to upgrade to three.js r100.
Links
 three.js releases: https://github.com/mrdoob/three.js/releases
 three.js migration guide: https://github.com/mrdoob/three.js/wiki/MigrationGuide
Our last upgrade of our threejs package was to Three.js r80 in #22226 in 201701.
In a sagesupport discussion about 3D plotting on Arch Linux, Antonio Rojas points out that:
The three.js version shipped by Arch is too new and not supported by Sage. Either use jsmol (which is still the default), or use the online version of three.js (viewer='threejs', online=True) (with sagemath 8.44, in previous versions the online version doesn't work either).
Attachments (2)
Change History (21)
comment:1 Changed 2 years ago by
comment:2 Changed 2 years ago by
Three.js r100 is out.
comment:3 followup: ↓ 6 Changed 2 years ago by
 Description modified (diff)
 Milestone changed from sage8.5 to sagewishlist
 Summary changed from Upgrade to three.js r98 to Upgrade to three.js r100
Has this problem already been dealt with on SageCell or CoCalc, by any chance?
comment:4 followups: ↓ 5 ↓ 7 Changed 2 years ago by
 Description modified (diff)
Replying to arojas:
The breakage happened in r91.
Are there any hints in the three.js r90r91 migration guide?
comment:5 in reply to: ↑ 4 Changed 2 years ago by
Replying to slelievre:
Replying to arojas:
The breakage happened in r91.
Are there any hints in the three.js r90r91 migration guide?
The first bad build is https://github.com/mrdoob/three.js/commit/e5012e237dd1f05182dc1879347e7cb05bbebf48
comment:6 in reply to: ↑ 3 Changed 2 years ago by
Replying to slelievre:
Has this problem already been dealt with on SageCell or CoCalc, by any chance?
CoCalc has independent threejs code. This used to be the case with SageMathCell, but since it leads to incompatibilities between different interfaces and adds maintenance load, I was very happy to ditch it when threejs got available in Sage itself. I have no plans to change it back ;)
comment:7 in reply to: ↑ 4 Changed 2 years ago by
Replying to slelievre:
Replying to arojas:
The breakage happened in r91.
Are there any hints in the three.js r90r91 migration guide?
In the first line: Geometry().center() has changed behavior.
There is some cleanup that needs to happen in the Three.js template before this upgrade. Then I’ll take care of this ticket.
comment:8 Changed 2 years ago by
 Dependencies set to #27568
comment:9 Changed 2 years ago by
 Branch set to u/paulmasson/26718
comment:10 Changed 2 years ago by
 Cc egourgoulhon added
 Commit set to 9b54017e077cb2b5351f7276f84220360608b43e
 Milestone changed from sagewishlist to sage8.8
 Status changed from new to needs_review
Please test existing notebooks with multiple transparent objects to confirm proper rendering of them.
New commits:
9b54017  Upgrade Three.js to r100

Changed 2 years ago by
comment:11 Changed 2 years ago by
 Reviewers set to Eric Gourgoulhon
 Status changed from needs_review to needs_work
In a jupyter notebook, the following command
show(line3d([(1,2,3), (1,0,2), (3,1,4), (2,1,2)]), viewer='threejs')
results in a black box. If one click on it, one obtains an empty 3D frame, with no line drawn.
comment:12 Changed 2 years ago by
 Commit changed from 9b54017e077cb2b5351f7276f84220360608b43e to 18d0e74679a627d411a4e596d58072047030abe5
Branch pushed to git repo; I updated commit sha1. New commits:
18d0e74  Fix centering of points and lines

comment:13 Changed 2 years ago by
 Status changed from needs_work to needs_review
Forgot to correct the centering for points and lines. Should work now.
comment:14 Changed 2 years ago by
 Status changed from needs_review to positive_review
Sorry for the delay in replying and thanks for the update. I've checked it in both python2 and python3 Sage. Everything seems OK. Some issues are not fixed by the upgrade to r100:
 the code
g = sphere(color='red', opacity=0.2) g += sphere((0.5,0,0), color='lightgrey', opacity=0.4) show(g, viewer='threejs')
results in a truncated grey sphere, with some spurious shaded layers, cf. the attached image: )
 the code
icosahedron(viewer='threejs')
yields a "rounded" icosahedron.
These issues were already there in r80 and I understand that it is not the scope of this ticket to fix them. So positive review and thanks for the upgrade!
comment:15 followup: ↓ 18 Changed 2 years ago by
Eric, thanks for the review. I have several enhancements in mind but will wait until this ticket is merged in the next beta.
The icosahedron can be fixed with flat shading. I've already identified a spot in the code that should work for all Platonic solids.
The problem with the spheres is just part of how transparency works in WebGL. If you rotate the figure you'll see that the red sphere is truncated itself from certain angles. WebGL renders transparent objects as if located entirely at the center of the object. Extended intersecting transparent objects will always experience these sorts of issues. I don't think there's much I can do about that.
comment:16 followup: ↓ 17 Changed 2 years ago by
 Status changed from positive_review to needs_work
Where is the tarball?
comment:17 in reply to: ↑ 16 Changed 2 years ago by
 Status changed from needs_work to positive_review
comment:18 in reply to: ↑ 15 Changed 2 years ago by
Replying to paulmasson:
Eric, thanks for the review. I have several enhancements in mind but will wait until this ticket is merged in the next beta.
OK very good.
The icosahedron can be fixed with flat shading. I've already identified a spot in the code that should work for all Platonic solids.
The problem with the spheres is just part of how transparency works in WebGL. If you rotate the figure you'll see that the red sphere is truncated itself from certain angles. WebGL renders transparent objects as if located entirely at the center of the object. Extended intersecting transparent objects will always experience these sorts of issues. I don't think there's much I can do about that.
Thanks for these explanations.
comment:19 Changed 2 years ago by
 Branch changed from u/paulmasson/26718 to 18d0e74679a627d411a4e596d58072047030abe5
 Resolution set to fixed
 Status changed from positive_review to closed
The breakage happened in r91. Since that version, plots only show a black box at first. If you click and drag on the black box you get the axes (which rotate correctly), but no plot.