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.

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

Merge Threejsfundamentals into three.js #22807

Merged
merged 860 commits into from
Nov 24, 2021
Merged

Merge Threejsfundamentals into three.js #22807

merged 860 commits into from
Nov 24, 2021

Conversation

greggman
Copy link
Contributor

I worked with @mrdoob to get the threejsfundamentals article integrated with three.js, they appear under /manual as in https://threejs.org/manual. The hope is that they'd be useful as a start of a three.js manual. I think @mrdoob has thoughts on separating API docs from non-API docs and plans to integrate the non-API docs here?

The number of commits are large because I didn't want to discard the history of contributors for all the people that fixed typos and added translations. I don't know if it's possible to squash and keep their individual names or if there is some other way to lower the number. I did run some tools to try to remove the history of all files that were in the original repo but not used here.

The final commit has these notes:


Integrate the threejsfundamentals articles into three's repo

Notes:

  • organization

    manual
     +-en
     | +-article1.html
     | +-article2.html
     | +-...etc...
     +-ja
     | +-article1.html
     | +-article2.html
     | +-...etc...
     +-...
     +-3rdparty
     | +-monaco-editor
     +-resources (images etc for the articles)
     +-examples (the html files for each working example)
       +-example1.html
       +-example2.html
       +-resources (the image etc for examples)
    
  • the articles use a special version of prettify.js in manual/resources/prettify.js

    This version of prettify checks the first character of each line where '+' = line added,
    '-' line deleted, '*' line changed. I bring this up just if you want to use some other
    colorizer for code it will need to handle that.

  • manual/3rdparty/monaco-editor is the text editor used for live editing
    It's large and maybe you'd like to serve it from CDN?

  • the live editor export and three versions

    The live editor has an export button to jsfiddle/codepen/codesandbox/stackoverflow.
    On threejsfundamenals.org all samples were pegs to a specific version and
    all versions stayed available.
    .
    This way if a user exports an example it keeps working because it
    links to the version that worked at the time of export.

    In this version the examples all reference build/three.module.js and when exported
    those links are converted to skypack.dev links at the current version. The current version
    is determined by fetching https://raw.githubusercontent.com/mrdoob/three.js/master/package.json.
    If there is a better way to get the version you can update /manual/examples/resources/editor-settings.js.
    Like, if you want exports to link to https://threejs.org/build/three.modules.js then just edit
    /manual/examples/resources/editor-settings.js and make fixJSForCodeSite just return its input.
    (eg: function fixJSForCodeSite(url) { return url; }).
    When exported they reference https://threejs.org/build/three.module.js whichs means
    when three updates their exported stuff will break.

  • Path hacks

    The examples contain valid non qualified links. For example import * as THREE from '../../build/three.module.js'.
    In order to actually run the samples in the editor without a server side component the samples are loaded via fetch
    and the inserted into a blob. No URLs are relative to blobs so the URLs need to converted to fully qualified.
    In other words, import * as THREE from '../../build/three.module.js' needs to become
    import * as THREE from 'https://threejs.org/build/three.module.js'.

    The process to do this is a bunch of hacky regular expressions in /manual/examples/resources/editor-settings.js
    in fixSourceLinks. The "proper" way would require a full HTML parser and JavaScript parser which is way too much
    code but, using regular experssions is brittle.

    If you look at the expressions you can see it's searching for things like <script src="??">, <img src="???">,
    import bla '???' and for three.js it's also searching for loader.load in various forms.

    The point is, if you add a new sample you need to write it in a way that these expression will find your non qualified links
    and fix them for you or add new expression. For things like loader.load('some/unqualified/path') it works but if
    it's loader.load(someVariable) then it won't work.

    The solution I chose you can put /* threejs.org: url */ at the end of a line. This expression looks for the last quoted string
    on the line and assumes it's a link. Example

    const images = [
      { name: 'dog', path: 'some/unqualified/path/husky.jpg', }  /* threejs.org: url */
      { name: 'cat', path: 'some/unqualified/path/siamese.jpg', }  /* threejs.org: url */
      { name: 'bat', path: 'some/unqualified/path/vampire.jpg', }  /* threejs.org: url */
    ];
    for (const {name, path} of images) {
      loader.load(path);
      ...
    

    Now the script will convert these paths when loading them into the editor. The comment will be stripped so the user's
    does not see it in the editor.

  • Known Article Issues

    • optimize-lots-of-objects-animated was broken by r134.

      The point of the article was to show how to use morph targets to animate a 1000 cubes like the webgl-globe
      and fix a long standing bug in the webgl-globe that it doesn't animate the colors of the cubes.
      It just uses colors of the first set of cubes values which means it shows the wrong represtation of everything
      but the first set.

      With the new way morph targets work it looks like it's very non trivial to add vertex color morph targets.
      If there is still an easy way to do it I'm happy to update the aritcle
      but as it is, you should probably just delete it.

    • align-html-elements-to-3d was written before I knew about CSS2DRender

      I've never had time to re-write it and I thought explaining the math might be useful. It's up to you
      whether or not you want to keep it.

    • all the samples use rAF directly

      I'm guessing you'd prefer setAnimationLoop at this point

    • all of the samples follow the advice of the responsive.html article

      This means they all assume the user used CSS to set the size of the canvas.
      They always pass false as the 3rd parameter to Renderer.setSize

@donmccurdy
Copy link
Collaborator

donmccurdy commented Nov 10, 2021

This is exciting to see, @greggman! 🎉

Path hacks...

I had this problem on a project recently, and ended up using esbuild to solve it. The downside is a 2.2 MB (gzipped) WASM dependency, but in my case I needed TypeScript -> JavaScript transpilation too, so regular expressions were basically out of the question. I've written that solution up here: evanw/esbuild#1377. Another thing I'd tried that could be helpful, although I didn't end up using it myself, was parse-imports.

@Mugen87
Copy link
Collaborator

Mugen87 commented Nov 10, 2021

The PR introduces 39 lgtm errors, see https://lgtm.com/projects/g/mrdoob/three.js/rev/pr-3190f703a30408c8f1a14ebc0a2c2bcc9a3b5e21. Would be great to clean them up before merging.

@greggman greggman force-pushed the test branch 2 times, most recently from 5effa81 to e5fdd20 Compare November 10, 2021 12:12
@greggman
Copy link
Contributor Author

I fixed the LGTM issues that were actually errors. I'm not sure if you require the remaining ones to be fixed or suppressed or if they're fine as is. I'd prefer not to change the function table code. The regex is only working on small strings so that's a non-issue. The script one is also a non-issue

Notes:

* organization

  ```
  manual
   +-en
   | +-article1.html
   | +-article2.html
   | +-...etc...
   +-ja
   | +-article1.html
   | +-article2.html
   | +-...etc...
   +-...
   +-3rdparty
   | +-monaco-editor
   +-resources (images etc for the articles)
   +-examples (the html files for each working example)
     +-example1.html
     +-example2.html
     +-resources (the image etc for examples)
  ```

* the articles use a special version of `prettify.js` in `manual/resources/prettify.js`

  This version of prettify checks the first character of each line where '+' = line added,
  '-' line deleted, '*' line changed. I bring this up just if you want to use some other
  colorizer for code it will need to handle that.

* manual/3rdparty/monaco-editor is the text editor used for live editing
  It's large and maybe you'd like to serve it from CDN?

* the live editor export and three versions

  The live editor has an export button to jsfiddle/codepen/codesandbox/stackoverflow.
  On threejsfundamenals.org all samples were pegs to a specific version and
  [all versions stayed available](https://github.com/gfxfundamentals/threejsfundamentals/tree/master/threejs/resources/threejs).
.
  This way if a user exports an example it keeps working because it
  links to the version that worked at the time of export.

  In this version the examples all reference `build/three.module.js` and when exported
  those links are converted to skypack.dev links at the current version. The current version
  is determined by fetching `https://raw.githubusercontent.com/mrdoob/three.js/master/package.json`.
  If there is a better way to get the version you can update `/manual/examples/resources/editor-settings.js`.
  Like, if you want exports to link to `https://threejs.org/build/three.modules.js` then just edit
  `/manual/examples/resources/editor-settings.js` and make `fixJSForCodeSite` just return its input.
  (eg: `function fixJSForCodeSite(url) { return url; }`).
  When exported they reference `https://threejs.org/build/three.module.js` whichs means
  when three updates their exported stuff will break.

* Path hacks

  The examples contain valid non qualified links. For example `import * as THREE from '../../build/three.module.js'`.
  In order to actually run the samples in the editor without a server side component the samples are loaded via fetch
  and the inserted into a blob. No URLs are relative to blobs so the URLs need to converted to fully qualified.
  In other words,  `import * as THREE from '../../build/three.module.js'` needs to become
  `import * as THREE from 'https://threejs.org/build/three.module.js'`.

  The process to do this is a bunch of hacky regular expressions in `/manual/examples/resources/editor-settings.js`
  in `fixSourceLinks`. The "proper" way would require a full HTML parser and JavaScript parser which is way too much
  code but, using regular experssions is brittle.

  If you look at the expressions you can see it's searching for things like `<script src="??">`, `<img src="???">`,
  `import bla '???'` and for three.js it's also searching for `loader.load` in various forms.

  The point is, if you add a new sample you need to write it in a way that these expression will find your non qualified links
  and fix them for you or add new expression. For things like `loader.load('some/unqualified/path')` it works but if
  it's `loader.load(someVariable)` then it won't work.

  The solution I chose you can put `/* threejs.org: url */` at the end of a line. This expression looks for the last quoted string
  on the line and assumes it's a link. Example

  ```
  const images = [
    { name: 'dog', path: 'some/unqualified/path/husky.jpg', }  /* threejs.org: url */
    { name: 'cat', path: 'some/unqualified/path/siamese.jpg', }  /* threejs.org: url */
    { name: 'bat', path: 'some/unqualified/path/vampire.jpg', }  /* threejs.org: url */
  ];
  for (const {name, path} of images) {
    loader.load(path);
    ...
  ```

  Now the script will convert these paths when loading them into the editor. The comment will be stripped so the user's
  does not see it in the editor.

* Known Article Issues

  * optimize-lots-of-objects-animated was broken by r134.

    The point of the article was to show how to use morph targets to animate a 1000 cubes like the webgl-globe
    and fix a long standing bug in the webgl-globe that it doesn't animate the colors of the cubes.
    It just uses colors of the first set of cubes values which means it shows the wrong represtation of everything
    but the first set.

    With the new way morph targets work it looks like it's very non trivial to add vertex color morph targets.
    If there is still an easy way to do it I'm happy to update the aritcle
    but as it is, you should probably just delete it.

  * align-html-elements-to-3d was written before I knew about `CSS2DRender`

    I've never had time to re-write it and I thought explaining the math might be useful. It's up to you
    whether or not you want to keep it.

  * all the samples use rAF directly

    I'm guessing you'd prefer `setAnimationLoop` at this point

  * all of the samples follow the advice of the `responsive.html` article

    This means they all assume the user used CSS to set the size of the canvas.
    They always pass `false` as the 3rd parameter to `Renderer.setSize`
@mrdoob mrdoob added this to the r135 milestone Nov 18, 2021
@mrdoob mrdoob merged commit e0f5976 into mrdoob:dev Nov 24, 2021
@mrdoob
Copy link
Owner

mrdoob commented Nov 24, 2021

Thanks!

@mrdoob
Copy link
Owner

mrdoob commented Nov 26, 2021

I've implemented the docs styles as best as I can: 4519858
I hope I didn't break too many things...

@greggman
Copy link
Contributor Author

greggman commented Nov 26, 2021

One thing kind of broke? The style was meant to be where images and diagrams can be larger than the paragraphs of text.

       bla bla bla bla
       bla bla bloo bla
       bla bloo bla bla

some  diagram  that  is  easier
to see if it's  larger than the 
paragraphs       of        text

       bla bla bla bla
       bla bla bloo bla
       bla bloo bla bla

In particular, the live code editors are now arguably too small to be used as they're now constrained by the "container" div.

old:

Screen Shot 2021-11-26 at 00 38 37

new:

Screen Shot 2021-11-25 at 23 39 14

Of course they're still functional at that small size, it's just not much room to edit and see the results at the same time

Screen Shot 2021-11-25 at 23 41 33

Of course it's your choice. Just thought I'd point out the change

@greggman
Copy link
Contributor Author

Also, fullscreen on the editor broke

Screen Shot 2021-11-26 at 00 35 17

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.