Opened 13 months ago

Closed 7 months ago

#26718 closed enhancement (fixed)

Upgrade to three.js r100

Reported by: slelievre Owned by:
Priority: major Milestone: sage-8.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) Commit: 18d0e74679a627d411a4e596d58072047030abe5
Dependencies: #27568 Stopgaps:

Description (last modified by slelievre)

This ticket is to upgrade to three.js r100.

Links

Our last upgrade of our threejs package was to Three.js r80 in #22226 in 2017-01.

In a sage-support 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.4-4, in previous versions the online version doesn't work either).

Attachments (2)

threejs-r100.tar.gz (139.7 KB) - added by paulmasson 8 months ago.
two_spheres.png (60.2 KB) - added by egourgoulhon 8 months ago.
threejs plot of 2 spheres

Download all attachments as: .zip

Change History (21)

comment:1 Changed 13 months ago by arojas

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.

comment:2 Changed 11 months ago by slelievre

Three.js r100 is out.

comment:3 follow-up: Changed 10 months ago by slelievre

  • Description modified (diff)
  • Milestone changed from sage-8.5 to sage-wishlist
  • 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 follow-ups: Changed 10 months ago by slelievre

  • Description modified (diff)

Replying to arojas:

The breakage happened in r91.

Are there any hints in the three.js r90--r91 migration guide?

comment:5 in reply to: ↑ 4 Changed 10 months ago by arojas

Replying to slelievre:

Replying to arojas:

The breakage happened in r91.

Are there any hints in the three.js r90--r91 migration guide?

The first bad build is https://github.com/mrdoob/three.js/commit/e5012e237dd1f05182dc1879347e7cb05bbebf48

comment:6 in reply to: ↑ 3 Changed 10 months ago by novoselt

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 8 months ago by paulmasson

Replying to slelievre:

Replying to arojas:

The breakage happened in r91.

Are there any hints in the three.js r90--r91 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 8 months ago by paulmasson

  • Dependencies set to #27568

comment:9 Changed 8 months ago by paulmasson

  • Branch set to u/paulmasson/26718

comment:10 Changed 8 months ago by paulmasson

  • Authors set to Paul Masson
  • Cc egourgoulhon added
  • Commit set to 9b54017e077cb2b5351f7276f84220360608b43e
  • Milestone changed from sage-wishlist to sage-8.8
  • Status changed from new to needs_review

Please test existing notebooks with multiple transparent objects to confirm proper rendering of them.


New commits:

9b54017Upgrade Three.js to r100

Changed 8 months ago by paulmasson

comment:11 Changed 8 months ago by egourgoulhon

  • 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 8 months ago by git

  • Commit changed from 9b54017e077cb2b5351f7276f84220360608b43e to 18d0e74679a627d411a4e596d58072047030abe5

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

18d0e74Fix centering of points and lines

comment:13 Changed 8 months ago by paulmasson

  • Status changed from needs_work to needs_review

Forgot to correct the centering for points and lines. Should work now.

Changed 8 months ago by egourgoulhon

threejs plot of 2 spheres

comment:14 Changed 8 months ago by egourgoulhon

  • 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: threejs plot of 2 spheres)

  • 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 follow-up: Changed 8 months ago by paulmasson

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 follow-up: Changed 7 months ago by vbraun

  • Status changed from positive_review to needs_work

Where is the tarball?

comment:17 in reply to: ↑ 16 Changed 7 months ago by paulmasson

  • Status changed from needs_work to positive_review

Replying to vbraun:

Where is the tarball?

Just after this comment

comment:18 in reply to: ↑ 15 Changed 7 months ago by egourgoulhon

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 7 months ago by vbraun

  • Branch changed from u/paulmasson/26718 to 18d0e74679a627d411a4e596d58072047030abe5
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.