Opened 2 years ago

Closed 2 years ago

#29809 closed enhancement (fixed)

Three.js: Upgrade to r117

Reported by: Paul Masson Owned by:
Priority: major Milestone: sage-9.2
Component: graphics Keywords: threejs
Cc: ​egourgoulhon, Joshua Campbell, François Bissey Merged in:
Authors: Paul Masson Reviewers: Joshua Campbell
Report Upstream: N/A Work issues:
Branch: 375a187 (Commits, GitHub, GitLab) Commit: 375a187b0c9f83c4296cfad6b0797018e900534f
Dependencies: Stopgaps:

Status badges

Description (last modified by Paul Masson)

The change to the template avoids a console warning that is new for this version.

The other console warning about OrbitControls cannot be removed because it represents a major change to the Three.js library that will take some effort to work around, but that does not need to be addressed until the beginning of next year.

Upgraded library is in attachments to this ticket.

Attachments (1)

threejs-r117.tar.gz (156.4 KB) - added by Paul Masson 2 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 2 years ago by Paul Masson

Description: modified (diff)

comment:2 Changed 2 years ago by Paul Masson

Branch: u/paulmasson/threejs-r117

Changed 2 years ago by Paul Masson

Attachment: threejs-r117.tar.gz added

comment:3 Changed 2 years ago by Paul Masson

Commit: 375a187b0c9f83c4296cfad6b0797018e900534f
Status: newneeds_review

New commits:

375a187Upgrade to Three.js r117

comment:4 Changed 2 years ago by François Bissey

Cc: François Bissey added

comment:5 Changed 2 years ago by Frédéric Chapoton

maybe you should add the new "upstream_url" field in checksums.ini ?

Something like this, adapted :

upstream_url=https://github.com/numpy/numpy/releases/download/vVERSION/numpy-VERSION.tar.gz

comment:6 in reply to:  5 Changed 2 years ago by Paul Masson

Replying to chapoton:

maybe you should add the new "upstream_url" field in checksums.ini ?

Something like this, adapted :

upstream_url=https://github.com/numpy/numpy/releases/download/vVERSION/numpy-VERSION.tar.gz

The upstream zip file includes the entire project. We are only using a small fraction of it, so the numbers wouldn't match.

Version 0, edited 2 years ago by Paul Masson (next)

comment:7 Changed 2 years ago by Joshua Campbell

Please forgive my ignorance of the process of updating a sage package, but I ran into build errors due to the zip file not existing on the mirrors (like the patch bot is currently reporting). Is this expected until the ticket gets merged or some other step is taken to sync them up? I was able to get it to build correctly by manually moving the zip into the upstream/ directory.

I verified that the two files within the zip (three.min.js and OrbitControls.js) are indeed from the r117 release and that the listed checksums match.

I had a collection of Jupyter notebooks on hand from prior work containing all of the examples in the plot3d reference documentation, so I did a quick side-by-side comparison with the old and new Three.js versions. I didn't observe any visual discrepancies, and the camera controls function the same as before.

I also noticed the new console warning regarding OrbitControls.js as expected.

Was the other console warning you mentioned due to Three.js expecting a boolean value but json.useFlatShading being null/undefined? I think !!json.useFlatShading would also work to coerce it to be boolean, but that's a minor nitpick.

comment:8 Changed 2 years ago by Paul Masson

Joshua, copying the zip file to upstream is exactly what is needed in this circumstance, since it won't be copied to the mirrors until the ticket is approved. If you don't notice any problems in your notebooks then we're good to go.

comment:9 Changed 2 years ago by Joshua Campbell

Reviewers: Joshua Campbell
Status: needs_reviewpositive_review

Ok, thank you. Just wanted to make sure that was expected. I've given a positive review.

comment:10 in reply to:  9 Changed 2 years ago by Paul Masson

Replying to gh-jcamp0x2a:

Ok, thank you. Just wanted to make sure that was expected. I've given a positive review.

Thanks!

comment:11 Changed 2 years ago by Volker Braun

Branch: u/paulmasson/threejs-r117375a187b0c9f83c4296cfad6b0797018e900534f
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.