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.

Sign up for GitHub

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

Jump to bottom

BUG #1740: plotting with bg=0 produces labels off #1862

Merged
merged 3 commits into from
Mar 7, 2016
Merged

BUG #1740: plotting with bg=0 produces labels off #1862

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cb09883
BUG cdat-web #78: VTKPlots.configureEvent is not called
danlipsa Mar 1, 2016
3082858
Merge branch 'master' into labels-off
doutriaux1 Mar 3, 2016
7f555cc
Merge branch 'master' into labels-off
doutriaux1 Mar 4, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions Packages/vcs/Lib/VTKPlots.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@ def createRenWin(self, *args, **kargs):
if not self.bg:
self.createDefaultInteractor(self.renderer)
self.renWin.AddRenderer(self.renderer)
self.renWin.AddObserver("ModifiedEvent", self.configureEvent)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks good to me.

if self.bg:
self.renWin.SetOffScreenRendering(True)
if "open" in kargs and kargs["open"]:
Expand All @@ -357,7 +358,7 @@ def createRenderer(self, *args, **kargs):
return ren

def update(self, *args, **kargs):
self._lastSize = -1
self._lastSize = None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danlipsa have you tested on mac? Mac is VERY different about this, also we need to test within GUI. @chaosphere2112 can you please test on your mac and in your new GUI?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@doutriaux1 No, I haven't. I have my mac with me and I can run the tests. I tried to be very conservative in what I change. :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chaosphere2112 I added a missing png for a test you added. Can you make sure it is the correct image? Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@doutriaux1 There are a bunch of test failures because of small differences in the baselines - that happens on master branch as well. I will push _xx versions shortly. Everything else looks good.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chaosphere2112 For test_vcs_missing_colorname I get 'Cell index must be in the range 0 to 255'. I'll let you fix that. That happens only on mac, on linux it works fine.

if self.renWin:
if self.get3DPlot():
plots_args = []
Expand Down Expand Up @@ -1076,9 +1077,11 @@ def png(self, file, width=None, height=None,
if width is not None and height is not None:
if self.renWin.GetSize() != (width, height):
user_dims = (self.canvas.bgX, self.canvas.bgY, sz[0], sz[1])
self.renWin.SetSize(width, height)
# We need to set canvas.bgX and canvas.bgY before we do renWin.SetSize
# otherwise, canvas.bgX,canvas.bgY will win
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danlipsa what do you mean by canvas.bgX,canvas.bgY will win? if you set it after they should be un-used right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aashish24 In the original order, SetSize triggered code that reset the size to bgX, bgY. Not sure which test has shown this. It is easy to investigate if we want to.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aashish24 In the original order, SetSize triggered code that reset the size to bgX, bgY. Not sure which test has shown this. It is easy to investigate if we want to.

if that's the case why not just call SetSize and not set bgX and bgY

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I belive bgX and bgY is only used when bg=1? @doutriaux1 @danlipsa if yes, that would explain why it would fail in bg=1 and not in bg=0

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if that's the case why not just call SetSize and not set bgX and bgY

Because you will endup, in certain cases, with bgX, bgY, not with the parameters you pass in SetSize

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because you will endup, in certain cases, with bgX, bgY, not with the parameters you pass in SetSize

if SetSize triggers something which set bgX and bgY in some cases which won't be same as the origin args then I think we have problem. I think its worth looking into it. Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if SetSize triggers something which set bgX and bgY in some cases

No, this SetSize triggers code that, in certain cases, sets the size of the window to bgX, bgY rather than the parameters passed to SetSize. The test that fails with the order reversed is vcs_test_png_set_size.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aashish24 This is the stack trace that explains why this happens:
lastSize: 15, 11
size: 814, 606
File "/home/danlipsa/src/uvcdat/testing/vcs/test_png_set_size.py", line 24, in
x.png(fnm,width=15)
File "/home/danlipsa/build/uvcdat/install/lib/python2.7/site-packages/vcs/Canvas.py", line 4773, in png
file, W, H, units, draw_white_background, *_args)
File "/home/danlipsa/build/uvcdat/install/lib/python2.7/site-packages/vcs/VTKPlots.py", line 1085, in png
self.renWin.SetSize(width, height)
File "/home/danlipsa/build/uvcdat/install/lib/python2.7/site-packages/vcs/VTKPlots.py", line 267, in configureEvent
self.canvas.plot(_pargs, render=False, *_key_args[i])
File "/home/danlipsa/build/uvcdat/install/lib/python2.7/site-packages/vcs/Canvas.py", line 2424, in plot
a = self.__plot(arglist, keyargs)
File "/home/danlipsa/build/uvcdat/install/lib/python2.7/site-packages/vcs/Canvas.py", line 3747, in __plot
returned_kargs = self.backend.plot(_arglist, **keyargs)
File "/home/danlipsa/build/uvcdat/install/lib/python2.7/site-packages/vcs/VTKPlots.py", line 518, in plot
self.renWin.SetSize(self.canvas.bgX, self.canvas.bgY)
File "/home/danlipsa/build/uvcdat/install/lib/python2.7/site-packages/vcs/VTKPlots.py", line 227, in configureEvent

self.canvas.bgX = width
self.canvas.bgY = height
self.renWin.SetSize(width, height)
self.configureEvent(None, None)
else:
user_dims = None
Expand Down
1 change: 0 additions & 1 deletion testing/vcs/test_vcs_configurator_resize.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
x.configure()

x.backend.renWin.SetSize(814, 303)
x.backend.renWin.Modified()

fnm = "test_vcs_configurator_resize.png"

Expand Down