Opened 7 years ago

Closed 7 years ago

#17292 closed defect (fixed)

Make transparency work in jmol

Reported by: kcrisman Owned by:
Priority: blocker Milestone: sage-6.4
Component: notebook Keywords:
Cc: novoselt, gutow, vbraun, strogdon, ppurka Merged in:
Authors: Karl-Dieter Crisman Reviewers: Jonathan Gutow, Andrey Novoseltsev
Report Upstream: Fixed upstream, in a later stable release. Work issues:
Branch: 63b2a0b (Commits, GitHub, GitLab) Commit: 63b2a0b449d046e909e16f1ef8367b658a878ae1
Dependencies: Stopgaps:

Status badges

Description (last modified by kcrisman)

See here, where it is pointed out that we need to upgrade the platformspeed variable in the js in order for things to work properly for transparency in jsmol.

Upstream package at http://sage.math.washington.edu/home/kcrisman/sagenb-0.11.1.tar

Change History (44)

comment:1 Changed 7 years ago by kcrisman

The Jmol wiki says

HTML5 is significantly slower than Java. For large systems it is strongly advised to use the set platformSpeed setting (or allow the user to set that). Smaller molecules (<10,000 atoms) are handled quite well. Large molecules (>20,000 atoms; see table at right) may be slow to load and process.

Also note it says IE is "unworkably slow" with the jsmol option, though I don't know how authoritative this is.

At the very least, maybe we need a little documentation for this somewhere, in addition to setting this. I don't think we want to get in the business of checking whether a plot has transparency and then sending a message to Jmol...

comment:2 Changed 7 years ago by kcrisman

  • Report Upstream changed from N/A to Workaround found; Bug reported upstream.

comment:3 Changed 7 years ago by kcrisman

Hold up, is it only while spinning/moving that this is not supported? I can confirm this. Maybe in that case this could be downgraded. What do you guys think?

Update: In fact, this also happens whenever right-clicking on the menu. In addition, the "animation" menu is whited-out. Finally, it even seems to happen with the java, not just jsmol version. Is that possible?

comment:4 Changed 7 years ago by kcrisman

And we couldn't do it dynamically easily in any case because there is some horrendous minification that goes on with notebook_dynamic.js which is where the relevant file is included.

comment:5 Changed 7 years ago by kcrisman

The animation menu still is whited-out with speed 7, though now the transparency indeed works. Plots with many points are indeed perhaps even slower, and are truly much faster with the java. That said, I don't know that they are enough slower for me to notice that much, because they are just very slow without java. (This only applies to the really big plot points, of course, not defaults.)

Maybe this needs some really obvious documentation to "use java for big plots" in the plot3d documentation, maybe even in the "Help" link that one clicks for the notebook. I don't want to go back to Sage 3d graphics being unusable on many browsers. And this was truly a one-character change.

comment:6 follow-up: Changed 7 years ago by novoselt

Personally I find it extremely annoying that transparency changes when I click on a plot, since it is not just hiding some parts, it is greatly changing/flashing the whole thing.

On a more objective note - the point of transparency is to be able to see things which are hidden otherwise to get a better sense of the plot. And the point of spinning the plot around is the same. With this new setting you can only benefit from one or the other but not both.

Regarding speed issues - can we get some examples of Sage plots which we should "test"? Just getting things slow with many points is a bit pointless - for what plots it is really necessary to increase the number of points?

comment:7 in reply to: ↑ 6 Changed 7 years ago by kcrisman

On a more objective note - the point of transparency is to be able to see things which are hidden otherwise to get a better sense of the plot. And the point of spinning the plot around is the same. With this new setting you can only benefit from one or the other but not both.

Yes. Well, changing the platform speed will be the way to go, then. Can you take care of the sagenb package for this iteration?

Regarding speed issues - can we get some examples of Sage plots which we should "test"? Just getting things slow with many points is a bit pointless - for what plots it is really necessary to increase the number of points?

No no, I always use something like

var('y')
plot3d(sin(x^2+y^2),(x,0,10),(y,0,10))

where an increased number of points makes a very noticeable difference.

comment:8 follow-up: Changed 7 years ago by novoselt

Playing some more, platformspeed 6 is the first one which shows the plot at all during rotation, 7 includes transparency, and 8 includes antialiasing as well. Which is greatly affecting shape and size of "thick curves" and "fat points". In fact, with 8 I can see no differences while rotating and while not rotating and I think this should be the default (it certainly was the default before). I don't know what is changing between 8 and 10.

With 200 points your sin plot works OK for me and while on 6 it may be faster, it is not at all the difference between OK and unusable. This is on a fast desktop (4GHz cores) and I'll try to test on a slower machine as well, however I am for default of 8 regardless of outcomes.

As for packaging the next version - I am hesitant to commit since I tend to lag behind...

comment:9 follow-up: Changed 7 years ago by strogdon

Just noticed this. Should the individuals listed opposite Authors: really be opposite Cc:

comment:10 Changed 7 years ago by novoselt

  • Authors novoselt, gutow, vbraun, strogdon deleted
  • Cc novoselt gutow vbraun strogdon added

comment:11 in reply to: ↑ 9 Changed 7 years ago by kcrisman

Just noticed this. Should the individuals listed opposite Authors: really be opposite Cc:

Wow, I really must have been in a hurry when I opened this one! Sorry.

comment:12 in reply to: ↑ 8 Changed 7 years ago by kcrisman

Playing some more, platformspeed 6 is the first one which shows the plot at all during rotation, 7 includes transparency, and 8 includes antialiasing as well. Which is greatly affecting shape and size of "thick curves" and "fat points". In fact, with 8 I can see no differences while rotating and while not rotating and I think this should be the default (it certainly was the default before). I don't know what is changing between 8 and 10.

I'm not sure it's changing anything.

With 200 points your sin plot works OK for me and while on 6 it may be faster, it is not at all the difference between OK and unusable. This is on a fast desktop (4GHz cores) and I'll try to test on a slower machine as well, however I am for default of 8 regardless of outcomes.

It's definitely very, very slow for me on my 2.3 GHz i7. Probably also depends on the browser. (By the way, for some reason I can use Java in Chrome; maybe it's just Chromium where it doesn't work.) I really wouldn't go above the 7 in case it really does do something. Among other reasons, because the doc for this at http://chemapps.stolaf.edu/jmol/jsmol/test2.htm suggests that for tablets or other mobile devices one will have problems with higher values.

comment:13 Changed 7 years ago by gutow

I'm pretty sure I picked 6 because that was the only thing that worked OK on tablets. Unfortunately, I do not have a test server set up with 6.4.rc1 on which to retest this. Here's a way we can all do device checking and report back. I have some web pages set up that have complex enough things that we should be able to tell.

  1. go to hybrid orbitals
  2. select a different orbital in each popup
  3. set two of them to transparent
  4. set another to mesh
  5. use the right-click menu to bring up the console
  6. test "set platformspeed = X", where X = 6 through 8.
  7. report back with device details.

(may not work on all touch devices as I have had trouble getting the equivalent of a right-click)

My first report: i3 portable that tops out at 1.8 GHZ (usually runs at 800 MHz) 8 or higher is fine on. Tested in Firefox and Chromium on Ubuntu 14.04.

comment:14 Changed 7 years ago by gutow

Better test page Standard JSmol test page

  1. select pmesh "data/sage.pmesh" from about 2/3 of the way down the second column of links.
  2. try the different platforms speeds by clicking on the numbers below the image

comment:15 Changed 7 years ago by gutow

Original Ipad: 7 is acceptable, 8 is not.

comment:16 Changed 7 years ago by novoselt

Thanks for the test, Jonathan!

On my over TEN YEARS OLD laptop with Athlon 64 1.8GHz (and GeForce MX440 Go videocard) it works just fine with 8. On the other hand, three.js on this machine is completely unusable since WebGL is not supported and canvas is incredibly slow (many seconds to render a frame).

While supporting old mobile devices is noble, I think that complicated 3D plots with a lot of points are more likely to be viewed on computers with adequate resources.

Note also that the current situation (for a year) is that rotatable 3D plots are not working at all on all devices. With 8 we will get a great renderer for desktops/laptops and perhaps more recent tablets as well. And while with 7 situation is satisfactory as well, I still would prefer not to compromise - spinning manually or automatically is the most natural way to look at 3D plots and the higher quality we can achieve the better.

comment:17 Changed 7 years ago by gutow

What about handheld devices like phones, many are less capable then my original Ipad? Anybody got an iphone 4, droid mini, of galaxy III?

Last edited 7 years ago by gutow (previous) (diff)

comment:18 Changed 7 years ago by vbraun

Let's make plots work first on real computers, please. Some of use use Sage for their work. I doubt that anybody is writing a paper or teaching classes on their iPhone 4.

It would be easy enough to add a javascript to pick out mobile devices depending on media query and degrade display, e.g.

var fast = window.matchMedia( "(min-width: 500px)" );

with a suitable try/catch for browser that don't support it.

comment:19 Changed 7 years ago by vbraun

Also, jsmol knows how long it takes to render each frame, right? It should just dynamically adjust the rendering quality to achieve >=30fps

comment:20 follow-up: Changed 7 years ago by vbraun

IMHO this is not a blocker, more of an annoyance.

comment:21 in reply to: ↑ 20 Changed 7 years ago by kcrisman

IMHO this is not a blocker, more of an annoyance.

Once I realized that it was only for rotation, I was thinking of downgrading to critical, but Andrey's point was actually fairly compelling.

Andrey and Jonathan, we can either fix this fast or dither about choices and not get it in. I think 7 is a good compromise but I don't think we are going to have the energy to put in some extra javascript, or at least I won't.


I doubt that anybody is writing a paper or teaching classes on their iPhone 4.

Volker, you wrote the Android app - how does it deal with 3d images? (I don't have iOS or Android so I can't test this, and the current Google Play page doesn't have an example pictured.) If the apps using cell server aren't doing 3d graphics anyway, then I wouldn't worry about it, it's relatively less frequent that people are doing intense graphics on such platforms, I imagine, since sagenb isn't optimized for mobile devices (at all).

Incidentally, the recent reviews at the Android app site are fairly negative - that's sad, because from everything I heard it was actually pretty awesome. I don't think sagecell has been having problems - or has it?

Also, jsmol knows how long it takes to render each frame, right? It should just dynamically adjust the rendering quality to achieve >=30fps

Jonathan can report that upstream...

comment:22 follow-up: Changed 7 years ago by vbraun

Java applets are not supported on Android.

comment:23 in reply to: ↑ 22 Changed 7 years ago by kcrisman

Java applets are not supported on Android.

Thanks, I didn't think about it that way. And on iOS I guess you need various workarounds.

So in other words, before the only option for 3d plotting in Sage on most tablets was canvas3d or Tachyon, neither of which are particularly good for that type of thing (spinning etc.). Now, presumably we will be able to support them, even if pretty darn slow.

So come to consensus on 7 or 8, given those constraints, and then we can always try to add something later to detect browsers - if really necessary.

comment:24 Changed 7 years ago by gutow

  • Branch set to u/gutow/new_jmol

comment:25 Changed 7 years ago by gutow

  • Commit set to a529db2a9e792b4cdc952976335122aa9ce64219
  • Status changed from new to needs_review

The commit I just pushed sets the default speed to 8. Test please.


Last 10 new commits:

4636610Cache bust for jsmol (including on Chrome)
7ca7bc6Merge pull request #3 from vbraun/new_jmol
f0d7870Correct introspection js
34a6a00CSS for introspect dialog style
b00705bUpdate CSS for styling dialog
4b16304Update sass for introspect dialog change
72cc162Merge branch 'testjsmol' of https://github.com/kcrisman/sagenb into kcrisman-css
2417e48Final changes for CSS to work properly on introspection popout
6ba5861Merge pull request #1 from kcrisman/testjsmol
a529db2Improve JSmol/Jmol rendering during spin "set platformspeed=8"

comment:26 Changed 7 years ago by gutow

Failed automerge suggests I missed some more recent commits (not sure how). The last one is the one-liner we need. It cannot conflict with anything else.

comment:27 Changed 7 years ago by kcrisman

You can't do it from here anyway, it has to be upstream, and then you can link to https://github.com/sagemath/sagenb/issues/254 from there. I'm sorry, it continues to be very annoying that the sagenb is "upstream", I know...

Anyway, I can just make the commit myself there as well without having to do any pushing, if you like.

comment:28 Changed 7 years ago by kcrisman

  • Authors set to Karl-Dieter Crisman
  • Report Upstream changed from Workaround found; Bug reported upstream. to Fixed upstream, in a later stable release.
  • Reviewers set to Jonathan Gutow, Andrey Novoseltsev
  • Status changed from needs_review to needs_work
  • Work issues set to new sagenb release

Okay, confirmed and done at this commit.

Now we just need a new sagenb release, probably waiting on https://github.com/sagemath/sagenb/issues/256 to be fixed.

comment:29 follow-up: Changed 7 years ago by novoselt

Regarding Android app: there were issues caused/exposed by my server setup changes that Volker has fixed at https://github.com/sagemath/sagecell/commit/8e5c8722e03bcd0bcd7a9fa67cf9c925ad99f7bf

SageCell uses Java Jmol by default (meaning it is not working almost anywhere) and has some three.js support separate from SMC. Not sure what will happen when I merge all these changes, but probably JS Jmol will not just work magically ;-)

comment:30 in reply to: ↑ 29 Changed 7 years ago by kcrisman

Regarding Android app: there were issues caused/exposed by my server setup changes that Volker has fixed at https://github.com/sagemath/sagecell/commit/8e5c8722e03bcd0bcd7a9fa67cf9c925ad99f7bf

Nice - probably good to change it soon, given all the angry reviews showing up on the app website.

SageCell uses Java Jmol by default (meaning it is not working almost anywhere) and has some three.js support separate from SMC. Not sure what will happen when I merge all these changes, but probably JS Jmol will not just work magically ;-)

I think it should, right? I forgot about the three.js stuff.

comment:31 follow-up: Changed 7 years ago by novoselt

Change what soon? sagecell.sagemath.org has been running the fixed version shortly after Volker's fix and Android app was updated a couple days after it. My understanding is that it is working now.

And given that !SageNB and SageCell have different resulting HTML and SageCell patches graphics code in Sage, I do not expect new Jmol work there right away. But we'll see!

comment:32 in reply to: ↑ 31 Changed 7 years ago by kcrisman

Change what soon? sagecell.sagemath.org has been running the fixed version shortly after Volker's fix and Android app was updated a couple days after it. My understanding is that it is working now.

Oh, I didn't realize that - I guess I saw a review from October there, but it looks like you must have fixed it shortly thereafter.

And given that !SageNB and SageCell have different resulting HTML and SageCell patches graphics code in Sage, I do not expect new Jmol work there right away. But we'll see!

Hmm, that may be. Let me know (not on this ticket, email) and I can test if you have a demo server at some point).

comment:33 Changed 7 years ago by kcrisman

  • Branch u/gutow/new_jmol deleted
  • Commit a529db2a9e792b4cdc952976335122aa9ce64219 deleted

comment:34 Changed 7 years ago by kcrisman

  • Component changed from graphics to notebook

comment:35 Changed 7 years ago by vbraun

Whats the plan for making a new sagenb release?

comment:36 Changed 7 years ago by kcrisman

  • Cc ppurka added

Plan is to do it today; I was waiting for the review of https://github.com/sagemath/sagenb/pull/259 though ppurka seemed to think there was something else "wrong" (but not badly) with dist.sh...

comment:37 follow-up: Changed 7 years ago by ppurka

I already fixed dist.sh in sagenb master.

comment:38 in reply to: ↑ 37 Changed 7 years ago by kcrisman

I already fixed dist.sh in sagenb master.

Oh! I didn't see that, sagenb doesn't give me auto-notifications except on my own stuff - I assume that is configurable, though.

comment:39 Changed 7 years ago by kcrisman

  • Branch set to u/kcrisman/17292
  • Commit set to 63b2a0b449d046e909e16f1ef8367b658a878ae1
  • Description modified (diff)
  • Status changed from needs_work to needs_review
  • Work issues new sagenb release deleted

New commits:

63b2a0bUpgrade sagenb to version 0.11.1

comment:40 Changed 7 years ago by novoselt

On the branch of the ticket:

$ sage -i http://sage.math.washington.edu/home/kcrisman/sagenb-0.11.1.tar
...
Finished extraction
/home/novoselt/sage/src/bin/sage-spkg: line 598: cd: sagenb-0.11.1.tar: No such file or directory
Error: after extracting, the directory sagenb-0.11.1.tar does not exist

Am I doing something wrong?

comment:41 follow-up: Changed 7 years ago by vbraun

You need to download the file yourself and put it into the upstream/ folder

comment:42 Changed 7 years ago by novoselt

  • Status changed from needs_review to positive_review

It works! (As expected ;-))

comment:43 in reply to: ↑ 41 Changed 7 years ago by kcrisman

You need to download the file yourself and put it into the upstream/ folder

But I think that syntax used to work, correct?

It works! (As expected ;-))

Let's hope there are finally no more issues!

comment:44 Changed 7 years ago by vbraun

  • Branch changed from u/kcrisman/17292 to 63b2a0b449d046e909e16f1ef8367b658a878ae1
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.