CDAT / vcs-js

3 stars 3 forks source link

Clicking a plot crashes python #22

Closed James-Crean closed 6 years ago

James-Crean commented 6 years ago

A single click on a plot will crash python.

Steps to reproduce in vcdat:

James-Crean commented 6 years ago

Ping @danlipsa

danlipsa commented 6 years ago

James, I'll take a look at this today. Sorry, I've been finishing up work on different projects.

danlipsa commented 6 years ago

@scottwittenburg Indeed I still get the segfault when using offscreen hardware rendering. For on-screen hardware rendering it works. I'll look into this next.

doutriaux1 commented 6 years ago

@danlipsa while it's definitely worth fixing. I thought we agreed to use the onscreen rendering version for performance reasons? I would really really really (did I mention really) like for us to get the clear button and files manager working in vcsjs first, so that we can deploy to friendly users on Monday!

danlipsa commented 6 years ago

@doutriaux1 If you as you use hardware rendering, both on-screen and off-screen should work as well. Right now vcs-js uses offscreen by default, and this is where I see the crash. I could changed it to on-screen, but that would mean that users would get an extra window, which would be distracting.

@scottwittenburg Is looking at clear and I think he already has a fix. Not sure what "files manager" is. Is that https://github.com/UV-CDAT/vcs-js/issues/19?

doutriaux1 commented 6 years ago

@danlipsa maybe I'm confused as what we both call "offscreen" in my previous comment I was referring to "mesalib" as offscreen, but I'm pretty sure we decided to go for the version without mesalib. So I guess my question is when you mean "offscreen" here do you mean a non-mesalib version but with bg=True ?

If that's the case then yes we need to fix this right away.

danlipsa commented 6 years ago

"offscreen" here do you mean a non-mesalib version but with bg=True ? Yes. If that's the case then yes we need to fix this right away. Sounds good.

danlipsa commented 6 years ago

@James-Crean @doutriaux1 @scottwittenburg This works with the latest vcs master. Indeed, it does not work (crashes) with 2.12. Can you confirm this?

doutriaux1 commented 6 years ago

Trying right away (on a mac)

doutriaux1 commented 6 years ago

it works for me on my mac, here's my env:

    vcdat:              0.0.5.2017.12.05.22.49.d27d46ac41569cc8856c9b8429822a55970e2f07-py_0          uvcdat/label/nightly
    vcs:                2.12.2018.01.11.10.53.dbb34f3113503cb840fa7ed616ee7e2eb5836885-py27_0         uvcdat/label/nightly
    vcs-js:             0.4.2.12.2018.01.19.11.09.650c2a2d2771ee9531f18855c1965ff26d91a4e8-py27_0     uvcdat/label/nightly
    vtk-cdat:           7.1.0.2.12-py27h5dbf064_0                                                     uvcdat              
danlipsa commented 6 years ago

There are a few commits since 2.12 that are important, at least on Linux:

(2.12) [~/projects/vcs/src (master %=)]$ git log V2.12
commit 44dd0b677121b0bf2ff3419ff21955e8eb805de0 (tag: v2.12, tag: V2.12)
Author: Charles Doutriaux 
Date:   Sun Sep 3 09:21:29 2017 -0700

    allow runt_test to be killed with Ctrl+C (#243)

    * allow runt_test to be killed with Ctrl+C

    * corrected english (such a difficult language)

    * trying w/o the -n

    * no need to downgrade conda anymore

    * ok we need a -n

    * 1 for circleci ?

    * Somehow picks up wrong mesa

    * Update circleci_mac.sh

(2.12) [~/projects/vcs/src (master %=)]$ git log --author=lipsa --since 44dd0b677121b0bf2ff3419ff21955e8eb805de0
commit d3e87ee22580fe25bcb09b0d08eb1f726c11c643
Author: Dan Lipsa 
Date:   Wed Nov 15 19:33:21 2017 -0500

    Add streamline to graphicsmethodlist. (#270)

commit 3114ff11f1f975c5f0e88c904a40cbdafdd2d3ff
Author: Dan Lipsa 
Date:   Thu Sep 7 17:23:02 2017 -0400

    Resize (#250)

    * Fix test_vcs_window_resize.py

    * Add VTKVCSBackend.setsize

    When we call renWin.SetSize we also need to call
    self.configureEvent to reposition the labels.
    A new size event is not sent automatically.

commit 1d87df29b8581013a0502745701c6d88780f065c
Author: Dan Lipsa 
Date:   Thu Sep 7 12:52:37 2017 -0400

    No bg window (#248)

    * Use a generic interactor for vcs-js

    * Do not listen to ModifiedEvent.

    ConfigureEvent should be enough to give us changes in size.
    ModifiedEvent is for the VTK object being modified. This caused
    the image to never be ready in vcs-js and bg=1.

    * Remove printout.

    * Fix flake.

The commit on Sep 7 about the bg window did it I think.

doutriaux1 commented 6 years ago

@danlipsa your log above reminded me of something, I'm not positive about it being listed in issues: the way the system is setup, it can only accept ONE variable per cell/plot, hence we cannot support streamlines or vector plots.

danlipsa commented 6 years ago

@doutriaux1 No, its not that. We do have vectors in the vcs-js demo. I think what we don't support yet (even if I believe this worked at one point) is double plotting over the same area. Such as one plot with box fill and over it a vector plot.

doutriaux1 commented 6 years ago

well, tell me how to do a vector plot in vcdat then. I tried dragging "u" then "v" (from clt.nc) and then a vector plot in it. I get nothing.

danlipsa commented 6 years ago

Not sure. @James-Crean @mattben ? We do have vector plots in the vcs-js demo. https://github.com/UV-CDAT/vcs-js/blob/650c2a2d2771ee9531f18855c1965ff26d91a4e8/demo/demo.js#L243

doutriaux1 commented 6 years ago

@James-Crean a quick look at the code, it "seems" we should be able to send more than one variable: https://github.com/UV-CDAT/vcdat/blob/master/frontend/src/js/components/Canvas.jsx#L46-L68