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

Animations will properly resume when resizing #1634

Merged
merged 1 commit into from Oct 27, 2015

Conversation

Projects
None yet
5 participants
@chaosphere2112
Contributor

chaosphere2112 commented Oct 23, 2015

Addresses the buggy part of #1629; performance fix will be a separate PR (I know the source of the issue, there will be an extensive writeup as part of that PR, which I hope to have up by end of day today).

@chaosphere2112 chaosphere2112 added this to the 2.4 milestone Oct 23, 2015

@aashish24

This comment has been minimized.

Contributor

aashish24 commented Oct 26, 2015

@chaosphere2112 I will review it today.

@aashish24

This comment has been minimized.

Contributor

aashish24 commented Oct 27, 2015

@chaosphere2112 I was able to reproduce the bug now working on testing your code.

@aashish24

This comment has been minimized.

Contributor

aashish24 commented Oct 27, 2015

@chaosphere2112 LGTM 👍 I see failures on dashboards but it does not seem to related to this branch.

@aashish24

This comment has been minimized.

Contributor

aashish24 commented Oct 27, 2015

Ship it!

@jbeezley

This comment has been minimized.

Contributor

jbeezley commented Oct 27, 2015

It looks like garant has been failing since about 9/23 (that was the last nightly to pass). Looking back through cdash, my guess is that #1563 is the cause.

FWIW, the output image looks something like the resizing bug that occurs in cdatweb. The axes and text don't look like they have redrawn yet. Another thing I noticed is that this test is much faster on garant than on the other dashboard machines (1.5s vs 5-10s).

@aashish24

This comment has been minimized.

Contributor

aashish24 commented Oct 27, 2015

@jbeezley the axes looks right on my Ubuntu box when I merged Sam's branch into master. Can you post your screen captures?

@jbeezley

This comment has been minimized.

Contributor

jbeezley commented Oct 27, 2015

Yes, it doesn't have anything to do with these changes. Like I said, it has shown up on the nightlies for over a month. For example here: https://open.cdash.org/testDetails.php?test=383120267&build=4070878

@aashish24

This comment has been minimized.

Contributor

aashish24 commented Oct 27, 2015

Yes, it doesn't have anything to do with these changes. Like I said, it has shown up on the nightlies for over a month. For example here: https://open.cdash.org/testDetails.php?test=383120267&build=4070878

Ah, thanks, I misunderstood what you said earlier. That test has been failing for sometime. @doutriaux1 are you going to look at it? Do you think its a more general issue like what @jbeezley is talking about?

aashish24 added a commit that referenced this pull request Oct 27, 2015

Merge pull request #1634 from UV-CDAT/update_while_animating
Animations will properly resume when resizing

@aashish24 aashish24 merged commit e666a03 into master Oct 27, 2015

5 of 8 checks passed

cont-int/LLNL/Linux-crunchy RH6 (FULL) running 'ctest -j12 -D Experimental' (Fri Oct 23 11:54:24 2015)
Details
continuous-integration/kitware-buildbot/uvcdat-garant-linux-release/ Build done.
Details
cont-int/LLNL/Darwin-Mac2 10.10.5 (FULL) running 'In Queue: 1' (Fri Oct 23 11:28:28 2015)
Details
cont-int/LLNL/Darwin-Mac 10.10.5 (LEAN) running 'ctest -j4 -D Experimental' (Fri Oct 23 11:41:03 2015)
Details
cont-int/LLNL/Darwin-Mac1 10.10.5 (NOGUI) running 'ctest -j4 -D Experimental' (Fri Oct 23 12:49:43 2015)
Details
cont-int/LLNL/Linux-oceanonly RH6 (MESA/NOGUI) running 'ctest -j12 -D Experimental' (Fri Oct 23 11:52:39 2015)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@aashish24 aashish24 deleted the update_while_animating branch Oct 27, 2015

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Oct 27, 2015

@chaosphere2112 looks like we need to tell test_vcs_click_info an actual size of screen to use, my guess is that on garant VTK is actually able to get the screensize (unlike on most other machines) and uses it to set a different value.

@chaosphere2112

This comment has been minimized.

Contributor

chaosphere2112 commented Oct 27, 2015

@doutriaux1 That would make sense... I can do that.

@aashish24

This comment has been minimized.

Contributor

aashish24 commented Oct 27, 2015

@chaosphere2112 looks like we need to tell test_vcs_click_info an actual size of screen to use, my guess is that on garant VTK is actually able to get the screensize (unlike on most other machines) and uses it to set a different value.

or may be its getting a different size since I don't know how VTK would behave differently.

@chaosphere2112

This comment has been minimized.

Contributor

chaosphere2112 commented Oct 27, 2015

@aashish24 It's been very odd. I've got installs where it correctly determines the size and ones where it doesn't (different OSX versions, mostly).

@aashish24

This comment has been minimized.

Contributor

aashish24 commented Oct 27, 2015

@aashish24 It's been very odd. I've got installs where it correctly determines the size and ones where it doesn't (different OSX versions, mostly).

The VTK API returns a pointer so unless the API changed, I don't know how it can return a number. But I haven't look at the code recently.

@chaosphere2112

This comment has been minimized.

Contributor

chaosphere2112 commented Oct 27, 2015

In [1]: import vtk

In [2]: w = vtk.vtkRenderWindow()

In [3]: w.GetScreenSize()
Out[3]: (2560, 1600)

In [4]: w.GetDPI()
Out[4]: 72
@jbeezley

This comment has been minimized.

Contributor

jbeezley commented Oct 27, 2015

On garant:

In [1]: import vtk

In [2]: w = vtk.vtkRenderWindow()

In [3]: w.GetScreenSize()
Out[3]: '_000000000116fac0_p_void'

In [4]: w.GetDPI()
Out[4]: 72
@jbeezley

This comment has been minimized.

Contributor

jbeezley commented Oct 27, 2015

That's with VTK commit 6d67334.

@chaosphere2112

This comment has been minimized.

Contributor

chaosphere2112 commented Oct 27, 2015

@jbeezley

This comment has been minimized.

Contributor

jbeezley commented Oct 27, 2015

Yeah, that was an old build. But using the latest CDAT/VTK@ec6c6ab, I still get

In [3]: w.GetScreenSize()
Out[3]: '_00000000025c9c40_p_void'
@jbeezley

This comment has been minimized.

Contributor

jbeezley commented Oct 27, 2015

This was supposedly fixed in CDAT/VTK@0541472. Maybe some other render window classes need the type hinting added.

@jbeezley

This comment has been minimized.

Contributor

jbeezley commented Oct 27, 2015

For the record, that is using XOpenGLRenderWindow on Ubuntu 14.04.

$ glxinfo | head
name of display: :0
display: :0  screen: 0
direct rendering: Yes
server glx vendor string: NVIDIA Corporation
server glx version string: 1.4

$ modinfo nvidia
filename:       /lib/modules/3.16.0-34-generic/kernel/drivers/video/nvidia.ko
alias:          char-major-195-*
version:        346.47
supported:      external
license:        NVIDIA
alias:          pci:v000010DEd00000E00sv*sd*bc04sc80i00*
alias:          pci:v000010DEd00000AA3sv*sd*bc0Bsc40i00*
alias:          pci:v000010DEd*sv*sd*bc03sc02i00*
alias:          pci:v000010DEd*sv*sd*bc03sc00i00*
depends:        drm
vermagic:       3.16.0-34-generic SMP mod_unload modversions
parm:           NVreg_Mobile:int
parm:           NVreg_ResmanDebugLevel:int
parm:           NVreg_RmLogonRC:int
parm:           NVreg_ModifyDeviceFiles:int
parm:           NVreg_DeviceFileUID:int
parm:           NVreg_DeviceFileGID:int
parm:           NVreg_DeviceFileMode:int
parm:           NVreg_RemapLimit:int
parm:           NVreg_UpdateMemoryTypes:int
parm:           NVreg_InitializeSystemMemoryAllocations:int
parm:           NVreg_UsePageAttributeTable:int
parm:           NVreg_MapRegistersEarly:int
parm:           NVreg_RegisterForACPIEvents:int
parm:           NVreg_CheckPCIConfigSpace:int
parm:           NVreg_EnablePCIeGen3:int
parm:           NVreg_EnableMSI:int
parm:           NVreg_MemoryPoolSize:int
parm:           NVreg_RegistryDwords:charp
parm:           NVreg_RmMsg:charp
parm:           NVreg_AssignGpus:charp

$ Xorg -version
X.Org X Server 1.16.0
Release Date: 2014-07-16
X Protocol Version 11, Revision 0
Build Operating System: Linux 3.2.0-70-generic x86_64 Ubuntu
Current Operating System: Linux garant 3.16.0-34-generic #47~14.04.1-Ubuntu SMP Fri Apr 10 17:49:16 UTC 2015 x86_64
Kernel command line: BOOT_IMAGE=/boot/vmlinuz-3.16.0-34-generic root=UUID=f6b13ce7-4323-498a-a545-0b6a9c5b7b3b ro quiet splash nouveau.blacklist=1 vt.handoff=7
Build Date: 12 February 2015  11:11:26PM
xorg-server 2:1.16.0-1ubuntu1.2~trusty2 (For technical support please see http://www.ubuntu.com/support)
Current version of pixman: 0.30.2
@aashish24

This comment has been minimized.

Contributor

aashish24 commented Oct 27, 2015

This was supposedly fixed in CDAT/VTK@0541472. Maybe some other render window classes need the type hinting added.

@jbeezley that sounds like a possibility. On my ubuntu as well I am getting a pointer but may for Mac, we have the type hinting set. @dlonie care to look into it?

@allisonvacanti

This comment has been minimized.

Contributor

allisonvacanti commented Oct 27, 2015

Sure enough, looks like the hints file is missing a definition for vtkXOpenGLRenderWindow. Funny, as that's the most frequently used window implementation.

Actually, on second look, it's a typo. There are hints provided for vtkXRenderWindow at the bottom of the file, but that class doesn't exist. It should be vtkXOpenGLRenderWindow.

@jbeezley Can you try patching the hints file on your local build to correct the name and see if that fixes it? I can prepare a patch if that fixes it.

@jbeezley

This comment has been minimized.

Contributor

jbeezley commented Oct 27, 2015

Sure.

@jbeezley

This comment has been minimized.

Contributor

jbeezley commented Oct 27, 2015

Yup, that fixed it.

diff --git a/Wrapping/Tools/hints b/Wrapping/Tools/hints
index c36c49e..78a6dfe 100644
--- a/Wrapping/Tools/hints
+++ b/Wrapping/Tools/hints
@@ -282,7 +282,7 @@ vtkWindowLevelLookupTable             GetMaximumTableValue              307  4
 vtkWindowLevelLookupTable             GetMinimumTableValue              307  4
 vtkXImageWindow                       GetPosition                       304  2
 vtkXImageWindow                       GetSize                           304  2
-vtkXRenderWindow                      GetPosition                       304  2
-vtkXRenderWindow                      GetScreenSize                     304  2
-vtkXRenderWindow                      GetSize                           304  2
+vtkXOpenGLRenderWindow                GetPosition                       304  2
+vtkXOpenGLRenderWindow                GetScreenSize                     304  2
+vtkXOpenGLRenderWindow                GetSize                           304  2
 vtkXYPlotActor                        GetPlotColor                      307  3
@allisonvacanti

This comment has been minimized.

Contributor

allisonvacanti commented Oct 28, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment