sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.41k stars 475 forks source link

Three.js: future of examples/js files #30620

Open 53959995-fd20-47f5-85e6-5e769b863d1f opened 4 years ago

53959995-fd20-47f5-85e6-5e769b863d1f commented 4 years ago

Since Three.js has recently gone completely modular, the examples/js folder is scheduled for deletion at the end of 2020. This creates problems for the files taken from there for use in SageMath, in particular OrbitControls.js.


References:

How to publish a npm package (properly) · TypeScript+React Primer http://typescript-react-primer.loyc.net/publish-npm-package.html

How to Bundle JavaScript With Rollup — Step-by-Step Tutorial · Learn With Jason https://www.learnwithjason.dev/blog/learn-rollup-js/

How To Deploy An NPM Package. Using Typescript and Rollup.js | by Raymundo Martinez | Level Up Coding https://levelup.gitconnected.com/how-to-deploy-an-npm-package-d75843fb77f1

jsDelivr - A free, fast, and reliable CDN for open source https://www.jsdelivr.com/

Modifying npm packages - the right way https://teambrookvale.com.au/articles/modifying-npm-packages-the-right-way

Creating and publishing scoped public packages | npm Documentation https://docs.npmjs.com/creating-and-publishing-scoped-public-packages

NPM Install with just GitHub | Better world by better software https://glebbahmutov.com/blog/npm-install-with-just-github/

Forking, Modifying, and Publishing NPM Packages https://brandontle.com/blog/forking-modifying-and-publishing-npm-packages/

CC: @jcamp0x2a

Component: graphics

Issue created by migration from https://trac.sagemath.org/ticket/30620

53959995-fd20-47f5-85e6-5e769b863d1f commented 4 years ago
comment:1

From #26410 as asked by Joshua Campbell:

Out of curiosity, do we know how we're going to handle this? As I understand, modules can't be loaded from the filesystem. So...

53959995-fd20-47f5-85e6-5e769b863d1f commented 4 years ago
comment:2

If it comes to maintaining a separate non-modular set of files, I am committed to providing OrbitControls.js at the very least. I'm also hoping to prod Mr.doob into adding some basic controls to the main library.

53959995-fd20-47f5-85e6-5e769b863d1f commented 4 years ago
comment:3

After some thought, clearly the best way forward is just to build Three.js ourselves from the source code. I've already done that before when contributing a minor fix. That way we can include any features we need, either now or in the future. The only question is where to host the repository for transparency.

Please hold off on working on the upgrade to r120, since this way will save the extra work of modifying the current build process.

21c3cf6e-6c50-4e8b-9831-5a03f9a599c1 commented 4 years ago
comment:4

Replying to @paulmasson:

After some thought, clearly the best way forward is just to build Three.js ourselves from the source code. I've already done that before when contributing a minor fix. That way we can include any features we need, either now or in the future.

So, we could bundle OrbitControls.js and the other examples we need directly inside the three.min.js that gets generated? So just one javascript file to include on the page? That would be nice if so!

Would this repository be just for Three.js itself, or would the template's code get moved into there as well? The idea of a dead simple template that just calls an external main function with all the SAGE_* stuff is somewhat appealing. Would be convenient to be able to work on the viewer outside of Sage.

Also, how does this relate to #30123, the proposed jupyter-threejs-minimal pypi package? Is that package what we're talking about here as well?


The only question is where to host the repository for transparency.

GitHub seems to be the direction Sage is headed. I think I saw a ticket about replacing patchbot with GitHub actions, for example, and there's already a bunch of repositories under https://github.com/sagemath. Maybe we can get a repository under there, too. If not, something under your profile?


Please hold off on working on the upgrade to r120, since this way will save the extra work of modifying the current build process.

Will do.

53959995-fd20-47f5-85e6-5e769b863d1f commented 4 years ago
comment:5

Replying to @jcamp0x2a:

Replying to @paulmasson:

After some thought, clearly the best way forward is just to build Three.js ourselves from the source code. I've already done that before when contributing a minor fix. That way we can include any features we need, either now or in the future.

So, we could bundle OrbitControls.js and the other examples we need directly inside the three.min.js that gets generated? So just one javascript file to include on the page? That would be nice if so!

Yup, just one file with whatever we need. I've already got OrbitControls included in a custom build, but need to iron out some details of the process. I'm getting files much too large at the end.

Would this repository be just for Three.js itself, or would the template's code get moved into there as well? The idea of a dead simple template that just calls an external main function with all the SAGE_* stuff is somewhat appealing. Would be convenient to be able to work on the viewer outside of Sage.

I should think the template needs to stay in Sage proper, since it is critical for the viewer. Feel free to elaborate more on what you have in mind. How would moving things out of Sage simplify the development process? Simply because you wouldn't have to run make build?

Also, how does this relate to #30123, the proposed jupyter-threejs-minimal pypi package? Is that package what we're talking about here as well?

The single file, plus the license, would indeed be the contents of the package.

The only question is where to host the repository for transparency.

GitHub seems to be the direction Sage is headed. I think I saw a ticket about replacing patchbot with GitHub actions, for example, and there's already a bunch of repositories under https://github.com/sagemath. Maybe we can get a repository under there, too. If not, something under your profile?

GitHub of course, but the question is how easy it will be to interface with the SageMath organization.

mkoeppe commented 4 years ago
comment:6

Replying to @paulmasson:

Replying to @jcamp0x2a:

Also, how does this relate to #30123, the proposed jupyter-threejs-minimal pypi package? Is that package what we're talking about here as well?

The single file, plus the license, would indeed be the contents of the package.

You could also consider making it a Python source package (with setup.py) such that setup.py build builds that single file; and then publish both an sdist and a wheel to PyPI.

21c3cf6e-6c50-4e8b-9831-5a03f9a599c1 commented 4 years ago
comment:7

Replying to @paulmasson:

I should think the template needs to stay in Sage proper, since it is critical for the viewer. Feel free to elaborate more on what you have in mind. How would moving things out of Sage simplify the development process? Simply because you wouldn't have to run make build?

Upon thinking it over, I retract my suggestion. But to be thorough, my idea was that threejs_template.html would remain in Sage, but it would be extremely minimal. Something like:

<!DOCTYPE html>
<html>
<head>
<title></title>
<meta charset="utf-8">
<meta name=viewport content="width=device-width, user-scalable=no, minimum-scale=1.0, maximum-scale=1.0">
<link rel="stylesheet" href="https://cdn.jsdelivr.net/gh/sagemath/threejs-viewer@r120/build/threejs-viewer.min.css"/>
</head>
<body>
<script src="https://cdn.jsdelivr.net/gh/sagemath/threejs-viewer@r120/build/threejs-viewer.min.js"></script>
<script>
initViewer( {
    options: SAGE_OPTIONS, 
    bounds: SAGE_BOUNDS, 
    lights: SAGE_LIGHTS, 
    ambient: SAGE_AMBIENT,
    texts: SAGE_TEXTS,
    points: SAGE_POINTS,
    lines: SAGE_LINES,
    surfaces: SAGE_SURFACES
} );
</script>
<!-- UI elements to be inserted by initViewer -->
</body>
</html>

It would keep the generated files smaller and maybe improve start-up time depending on how the browser caches AST/bytecode. Probably both extremely negligible, but perhaps would be noticeable with Jupyter notebooks containing a bunch of 3d plots. I've had to split such notebooks up in the past due to how long they would take to load. It may also open the door for other projects besides Sage to use the viewer as a front-end, if there's any interest in that.

But... it would mean duplication of tickets/issues for things requiring changes to both the viewer and Sage including updating the SPKG version each time, less visibility to other Sage developers, and yet one more place to have to keep up with besides Trac and sage-devel.

53959995-fd20-47f5-85e6-5e769b863d1f commented 4 years ago
comment:8

Joshua, keep in mind that browsers can only display a limited number of WebGL instances at once, typically 16. A notebook, being a browser, will lose instances once that limit is exceeded. There is a way to fix that using IntersectionObserver, but that would need to be done outside of Sage.

Mr.doob himself is working on refining the build process so the output files are not unnecessarily bloated. That will probably be a feature of r121.

Matthias, glad you've joined this conversation. Once the JavaScript side of the build process is under control, we'll want your input on how to integrate that into the Sage workflow.

21c3cf6e-6c50-4e8b-9831-5a03f9a599c1 commented 4 years ago
comment:9

Replying to @paulmasson:

Joshua, keep in mind that browsers can only display a limited number of WebGL instances at once, typically 16. A notebook, being a browser, will lose instances once that limit is exceeded. There is a way to fix that using IntersectionObserver, but that would need to be done outside of Sage.

Ah, I was unaware of such a limit. I doubt I'd ever have that many onscreen at a time, though, usually 3 at most. I've got a couple notebooks with over 40 Three.js plots that work fine. Not the snappiest, but I can scroll all the way down and back up without issue, and even the camera state is preserved, so perhaps FireFox is tearing down / reinitializing the instances in the background as I scroll.

53959995-fd20-47f5-85e6-5e769b863d1f commented 4 years ago
comment:10

Joshua, I'm attaching a preliminary version of the minified build to this ticket so you can experiment with it. The build depends on features recently added to r121, so we'll wait for that release before updating things here. The build requires having Node/npm installed and goes like this:

git clone --depth=1 --branch master --single-branch https://github.com/mrdoob/three.js.git
export { OrbitControls } from '../examples/jsm/controls/OrbitControls.js';
export { Line2 } from '../examples/jsm/lines/Line2.js';
export { LineGeometry } from '../examples/jsm/lines/LineGeometry.js';
export { LineMaterial } from '../examples/jsm/lines/LineMaterial.js';
export { LineSegments2 } from '../examples/jsm/lines/LineSegments2.js';
export { LineSegmentsGeometry } from '../examples/jsm/lines/LineSegmentsGeometry.js';

Matthias, since this requires non-Sage software and manual manipulation, I don't see how to automate it nicely. With the next upgrade we'll need to have a new threejs-sage repository in the SageMath organization, since we'll want the custom build to be available in online CDNs. How does one go about requesting this?

53959995-fd20-47f5-85e6-5e769b863d1f commented 4 years ago

Minified Three.js with orbit controls and fat lines

mkoeppe commented 4 years ago
comment:11

Attachment: three.min.js.gz

From the recipe - adding the 5 lines to src/Three.js - does this not just mean that you want to maintain a git branch with these lines added and merge in the new upstream releases when they become available?

The result of npm run build - is the idea to just publish it to npmjs.org (under a different name to indicate that it is a fork)?

53959995-fd20-47f5-85e6-5e769b863d1f commented 4 years ago
comment:12

Replying to @mkoeppe:

From the recipe - adding the 5 lines to src/Three.js - does this not just mean that you want to maintain a git branch with these lines added and merge in the new upstream releases when they become available?

We can do it that way as well, if it is more transparent. It would mean however that I would have to create new release tags different from those of Three.js, which could be confusing. Also, the default compressed files provided by GitHub for each release contain the entire current repository, which is huge for Three.js so not useful.

I was planning to have the repository just contain the current minimized file along with the license and the instructions above. Then a new release would automatically have the upstream zipped file that is currently being manually added to upgrade tickets. The downside of this process is that the work happens on a remote machine, mine or someone else's, so there's the somewhat hidden element of editing the src/Three.js file. Since the build has to happen remotely anyway, there will need to be an element of trust.

Even if we make Node.js a standard package, I don't see how to automate the whole process, including git commits, but feel free to help figure that out. Joshua, what are your thoughts on the process?

The result of npm run build - is the idea to just publish it to npmjs.org (under a different name to indicate that it is a fork)?

This step doesn't publish anything: it builds the release files on the local machine.

mkoeppe commented 4 years ago
comment:13

In general I think it is best to have a source repositories for sources, not build artifacts, and to publish build artifacts using the distribution infrastructure that is standard for the language used.

To make the build process convenient and transparent, one can use GitHub Actions to automate the process. https://docs.github.com/en/free-pro-team@latest/actions/guides/publishing-nodejs-packages

mkoeppe commented 4 years ago
comment:14

Upstream uses tags like r120, and you could be using r120-sage or something like this.

21c3cf6e-6c50-4e8b-9831-5a03f9a599c1 commented 4 years ago
comment:15

Replying to @paulmasson:

Joshua, I'm attaching a preliminary version of the minified build to this ticket so you can experiment with it. The build depends on features recently added to r121, so we'll wait for that release before updating things here. The build requires having Node/npm installed and goes like this:

  • perform a shallow clone of the target branch with
git clone --depth=1 --branch master --single-branch https://github.com/mrdoob/three.js.git
  • add the following lines to src/Three.js after the other exports
export { OrbitControls } from '../examples/jsm/controls/OrbitControls.js';
export { LineGeometry } from '../examples/jsm/lines/LineGeometry.js';
export { LineMaterial } from '../examples/jsm/lines/LineMaterial.js';
export { LineSegments2 } from '../examples/jsm/lines/LineSegments2.js';
export { LineSegmentsGeometry } from '../examples/jsm/lines/LineSegmentsGeometry.js';
  • cd three.js
  • npm install
  • npm run build

Thanks Paul. I found that I had to run npm run build-closure in order to build three.min.js in addition to three.js and three.module.js. I also added a line to export Line2:

export { Line2 } from '../examples/jsm/lines/Line2.js';

I tested this with my fat-lines branch from #26410 and didn't run into any issues. I particularly like how I was able to simplify the threejs_scripts and threejs_offline_scripts methods due to everything being bundled into that one three.min.js file.

53959995-fd20-47f5-85e6-5e769b863d1f commented 4 years ago
comment:16

Replying to @jcamp0x2a:

Replying to @paulmasson:

Joshua, I'm attaching a preliminary version of the minified build to this ticket so you can experiment with it. The build depends on features recently added to r121, so we'll wait for that release before updating things here. The build requires having Node/npm installed and goes like this:

  • perform a shallow clone of the target branch with
git clone --depth=1 --branch master --single-branch https://github.com/mrdoob/three.js.git
  • add the following lines to src/Three.js after the other exports
export { OrbitControls } from '../examples/jsm/controls/OrbitControls.js';
export { LineGeometry } from '../examples/jsm/lines/LineGeometry.js';
export { LineMaterial } from '../examples/jsm/lines/LineMaterial.js';
export { LineSegments2 } from '../examples/jsm/lines/LineSegments2.js';
export { LineSegmentsGeometry } from '../examples/jsm/lines/LineSegmentsGeometry.js';
  • cd three.js
  • npm install
  • npm run build

Thanks Paul. I found that I had to run npm run build-closure in order to build three.min.js in addition to three.js and three.module.js. I also added a line to export Line2:

export { Line2 } from '../examples/jsm/lines/Line2.js';

I tested this with my fat-lines branch from #26410 and didn't run into any issues. I particularly like how I was able to simplify the threejs_scripts and threejs_offline_scripts methods due to everything being bundled into that one three.min.js file.

Yeah, I overlooked Line2, so I've edited the instructions above. The Closure compiler is already gone from r121dev so you're building against r120. Since you've got a build I'll remove my attachment. Bet your final build is twice what it should be, since that was only fixed in the current dev branch...

53959995-fd20-47f5-85e6-5e769b863d1f commented 4 years ago
comment:17

Replying to @mkoeppe:

In general I think it is best to have a source repositories for sources, not build artifacts, and to publish build artifacts using the distribution infrastructure that is standard for the language used.

To make the build process convenient and transparent, one can use GitHub Actions to automate the process. https://docs.github.com/en/free-pro-team@latest/actions/guides/publishing-nodejs-packages

Matthias, I'm willing to commit to building the minified file for each release, since I need to do that for my own projects anyway. I am not going to spend my time on a complicated automated system, since I don't even use Sage on a day-to-day basis. Constructing such a system should fall to someone who benefits materially from using Sage.

21c3cf6e-6c50-4e8b-9831-5a03f9a599c1 commented 4 years ago
comment:18

Replying to @paulmasson:

Yeah, I overlooked Line2, so I've edited the instructions above. The Closure compiler is already gone from r121dev so you're building against r120. Since you've got a build I'll remove my attachment. Bet your final build is twice what it should be, since that was only fixed in the current dev branch...

Yes, it went from ~2.5MB to ~670KB after checking out the dev branch. I will note, though, that the revision check in DisplayManager.threejs_scripts no longer works in the dev branch. The minified code has gone from looking like:

yframeTrack=po;N.QuaternionLinearInterpolant=Er;N.REVISION="120";N.RGBADepthPacking=3201;N.RGBAFormat=1023;N.

... to looking like:

yframeTrack=lc,t.QuaternionLinearInterpolant=cc,t.REVISION=e,t.RGBADepthPacking=3201,t.RGBAFormat=L,t.RGBAInt

Trying to figure out what e is, and relying on it being stable, would probably not be a good idea. So perhaps we should include the package.json or a new plaintext file containing the revision number in the tarball that we can key off.

53959995-fd20-47f5-85e6-5e769b863d1f commented 4 years ago
comment:19

Replying to @jcamp0x2a:

Replying to @paulmasson:

Yeah, I overlooked Line2, so I've edited the instructions above. The Closure compiler is already gone from r121dev so you're building against r120. Since you've got a build I'll remove my attachment. Bet your final build is twice what it should be, since that was only fixed in the current dev branch...

Yes, it went from ~2.5MB to ~670KB after checking out the dev branch. I will note, though, that the revision check in DisplayManager.threejs_scripts no longer works in the dev branch. The minified code has gone from looking like:

yframeTrack=po;N.QuaternionLinearInterpolant=Er;N.REVISION="120";N.RGBADepthPacking=3201;N.RGBAFormat=1023;N.

... to looking like:

yframeTrack=lc,t.QuaternionLinearInterpolant=cc,t.REVISION=e,t.RGBADepthPacking=3201,t.RGBAFormat=L,t.RGBAInt

Trying to figure out what e is, and relying on it being stable, would probably not be a good idea. So perhaps we should include the package.json or a new plaintext file containing the revision number in the tarball that we can key off.

Just sent Mr.doob a message asking about this change. You might want to read #26434 for some background on what distribution managers think about this (and what nearly happened...)

mkoeppe commented 4 years ago
comment:20

Replying to @paulmasson:

Replying to @mkoeppe:

From the recipe - adding the 5 lines to src/Three.js - does this not just mean that you want to maintain a git branch with these lines added and merge in the new upstream releases when they become available?

We can do it that way as well, if it is more transparent.

Yes, I think whenever source are modified, that should be tracked in a source repository.

I can create one for you in https://github.com/sagemath/ -- what should the repository be named?

If you are concerned about the size of the repository, we could create a shallow clone (omit the history of the upstream repository).

53959995-fd20-47f5-85e6-5e769b863d1f commented 4 years ago
comment:21

Replying to @mkoeppe:

Replying to @paulmasson:

Replying to @mkoeppe:

From the recipe - adding the 5 lines to src/Three.js - does this not just mean that you want to maintain a git branch with these lines added and merge in the new upstream releases when they become available?

We can do it that way as well, if it is more transparent.

Yes, I think whenever source are modified, that should be tracked in a source repository.

I can create one for you in https://github.com/sagemath/ -- what should the repository be named?

If you are concerned about the size of the repository, we could create a shallow clone (omit the history of the upstream repository).

A shallow clone of the repository still ends up with a release of about 650 MB, since it includes all examples, documentation and auxiliary code. All the end user needs is a single 657 kB file. I'm not interested in shoveling excess data at users.

Here is an example of what I'm proposing, since I need it for my own use:

https://github.com/paulmasson/threejs-with-controls

This release contains only what the user needs, along with the original license and instructions for performing the build. Anyone who doubts the integrity of the build can reproduce it and compare files.

mkoeppe commented 4 years ago
comment:22

I have some naive questions.

53959995-fd20-47f5-85e6-5e769b863d1f commented 4 years ago
comment:23

Replying to @mkoeppe:

I have some naive questions.

  • It seems that deployment is intended to be done via cdn.jsdelivr.net; do you intend to also provide local deployment for use in Sage?

The CDN will only be used for graphics generated with online=True as is currently done. The zip file on the release page will become the upstream package that is equivalent to what is currently being distributed on Sage mirrors. This would of course still be included when building Sage and placed on the hard drive for local off-line access.

  • Since this is a js package, wouldn't people want to be able to install it with npm?

I for one only use Node when absolutely necessary, as it is in this build process, so I'm not terribly concerned with the npm ecosphere. The whole reason for building this vanilla JavaScript package is to relieve end users of having to deal with Node/npm. I don't see what would be gained by adding an option to install from npm when the zip file is already available on GitHub.

mkoeppe commented 4 years ago

Description changed:

--- 
+++ 
@@ -1 +1,32 @@
 Since Three.js has recently gone completely modular, the `examples/js` folder is scheduled for deletion at the end of 2020. This creates problems for the files taken from there for use in SageMath, in particular `OrbitControls.js`. 
+
+
+---
+
+References:
+
+How to publish a npm package (properly) · TypeScript+React Primer
+http://typescript-react-primer.loyc.net/publish-npm-package.html
+
+How to Bundle JavaScript With Rollup — Step-by-Step Tutorial · Learn With Jason
+https://www.learnwithjason.dev/blog/learn-rollup-js/
+
+How To Deploy An NPM Package. Using Typescript and Rollup.js | by Raymundo Martinez | Level Up Coding
+https://levelup.gitconnected.com/how-to-deploy-an-npm-package-d75843fb77f1
+
+jsDelivr - A free, fast, and reliable CDN for open source
+https://www.jsdelivr.com/
+
+Modifying npm packages - the right way
+https://teambrookvale.com.au/articles/modifying-npm-packages-the-right-way
+
+Creating and publishing scoped public packages | npm Documentation
+https://docs.npmjs.com/creating-and-publishing-scoped-public-packages
+
+NPM Install with just [GitHub](../wiki/GitHub) | Better world by better software
+https://glebbahmutov.com/blog/npm-install-with-just-github/
+
+Forking, Modifying, and Publishing NPM Packages
+https://brandontle.com/blog/forking-modifying-and-publishing-npm-packages/
+
+
53959995-fd20-47f5-85e6-5e769b863d1f commented 3 years ago
comment:25

Replying to @jcamp0x2a:

Replying to @paulmasson:

I will note, though, that the revision check in DisplayManager.threejs_scripts no longer works in the dev branch. The minified code has gone from looking like:

yframeTrack=po;N.QuaternionLinearInterpolant=Er;N.REVISION="120";N.RGBADepthPacking=3201;N.RGBAFormat=1023;N.

... to looking like:

yframeTrack=lc,t.QuaternionLinearInterpolant=cc,t.REVISION=e,t.RGBADepthPacking=3201,t.RGBAFormat=L,t.RGBAInt

Trying to figure out what e is, and relying on it being stable, would probably not be a good idea. So perhaps we should include the package.json or a new plaintext file containing the revision number in the tarball that we can key off.

Joshua, the current build of threejs-sage contains the string t.REVISION="122" along with a string revision:"122". Do these occur in your own custom builds on your machine? If so we can delete the version file on the next build.

mkoeppe commented 3 years ago
comment:26

I've added a comment to https://github.com/paulmasson/threejs-sage/issues/1

21c3cf6e-6c50-4e8b-9831-5a03f9a599c1 commented 3 years ago
comment:27

Replying to @paulmasson:

Joshua, the current build of threejs-sage contains the string t.REVISION="122" along with a string revision:"122". Do these occur in your own custom builds on your machine? If so we can delete the version file on the next build.

Yes, both are present in the build I produced. I'm not sure if we should rely on that always being the case, though, since it's not been at least once so far. Not really an objection to removing version, but just thinking we may end up adding it back at some point :)