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

ratio="autot" fails to correctly generate the initial ratio #1795

Closed
chaosphere2112 opened this issue Jan 15, 2016 · 14 comments

Comments

@chaosphere2112
Copy link
Contributor

commented Jan 15, 2016

If you plot something with ratio="autot" as the very first action you take with a canvas, the plot defaults to using the default window size, rather than the size that the canvas will actually have. This is because the call to determine the ratio happens prior to any calls to createRenWin() in the backend, so default values are retrieved using the canvasinfo() function.

Any future plots (or ones triggered by canvas.update()) will be the correct proportions.

@chaosphere2112 chaosphere2112 self-assigned this Jan 15, 2016

@chaosphere2112 chaosphere2112 added this to the 2.4 milestone Jan 15, 2016

@chaosphere2112

This comment has been minimized.

Copy link
Contributor Author

commented Jan 15, 2016

I've sort-of got a fix for this worked out; it needs work, though. I'm running a full test suite on it and sending it out the the dashboard.

@chaosphere2112

This comment has been minimized.

Copy link
Contributor Author

commented Jan 19, 2016

@doutriaux1

This comment has been minimized.

Copy link
Member

commented Jan 19, 2016

looks like the diags doesn't set the bg dims and throws the tests off.

@chaosphere2112

This comment has been minimized.

Copy link
Contributor Author

commented Jan 19, 2016

Does diags use Multi? Because almost all of the multi tests failed as well

@chaosphere2112

This comment has been minimized.

Copy link
Contributor Author

commented Jan 19, 2016

@doutriaux1 So Multi was using "x.portrait" and "x.landscape", which triggered a call to canvasinfo prior to plot, which meant that bg wasn't set. I'll wager it's something similar in the diagnostics.

https://open.cdash.org/viewTest.php?onlyfailed&buildid=4197441

@doutriaux1

This comment has been minimized.

Copy link
Member

commented Jan 19, 2016

@chaosphere2112 , according to @mcenerney1 diags are using Multi

@chaosphere2112 chaosphere2112 modified the milestones: 2.4.1, 2.4 Jan 20, 2016

@chaosphere2112

This comment has been minimized.

Copy link
Contributor Author

commented Jan 20, 2016

Going to open a PR for this, though it's not quite ready for release. Changed milestone to 2.4.1.

@doutriaux1

This comment has been minimized.

Copy link
Member

commented Jan 21, 2016

@chaosphere2112 I changed the milestone to 2.6 if it makes it to 2.4.1 ok, but 2.4.1 is a uvcmetrics targeted release.

@chaosphere2112

This comment has been minimized.

Copy link
Contributor Author

commented Jan 21, 2016

@chaosphere2112

This comment has been minimized.

Copy link
Contributor Author

commented Feb 5, 2016

@doutriaux1 @aashish24 Since this is a pretty minor issue, I'm going to backburner it for now while I get my GUI on. I'll come back to the PR in a month.

@aashish24

This comment has been minimized.

Copy link
Contributor

commented Feb 5, 2016

Roger that @chaosphere2112

@danlipsa

This comment has been minimized.

Copy link
Contributor

commented Apr 8, 2016

@chaosphere2112 This seems to work in my tests. Can you try it with the master branch . I fixed something that sounds like your problem in
#1896

@aashish24 aashish24 assigned danlipsa and unassigned chaosphere2112 May 25, 2016

@aashish24

This comment has been minimized.

Copy link
Contributor

commented May 25, 2016

@doutriaux1 can you confirm if this is fixed?

@chaosphere2112

This comment has been minimized.

Copy link
Contributor Author

commented May 25, 2016

Works!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.