Skip to content

849 prep fill area cleanup#877

Merged
doutriaux1 merged 3 commits intomasterfrom
849_prepFillAreaCleanup
Nov 11, 2014
Merged

849 prep fill area cleanup#877
doutriaux1 merged 3 commits intomasterfrom
849_prepFillAreaCleanup

Conversation

@alliepiper
Copy link
Copy Markdown
Contributor

@doutriaux1 @aashish24

Had an hour to kill and took a stab at optimizing the vcs scalar bar implementation. Ended up cutting the time down from 0.262s to 0.89s on my machine.

Ref #849

David C. Lonie added 2 commits November 6, 2014 15:49
Rather than allocating (and rendering!) a unique polydata + mapper + actor
combo for each vertical slice of the color bar, create a single, multicell
polydata that gets rendered at once in a single actor/mapper.

This speeds up the execution of prepFillArea using the test script in #849
from 0.25s to 0.09s on my machine.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This replaces the a.append(a[-1]) line, since it seems to me that if these don't match, it's a bug. I can change this back if it is needed, though it isn't causing any test failures this way.

If we revert it, make sure not to revert the bug I fixed in the first patch of this branch.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@dlonie I think what the code was trying to before was filling the list to make their length match. Using the last elements. I think your change would break things like:
fa.x = [.25,.75]
fa.y = [1,]

But you are probably right it is probably better to simply error exit in such cases.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If having different lengths of x/y are supported, then yes, we should revert it. But otherwise, padding out the arrays with the last value will mask errors and produce invalid results, rather than let the user know something went wrong.

I'm not sure what you mean by removing the lines that create the points, which lines are you referring to?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Forget my comment about lines gone, I didn't see the upper bit of the code. And yes let's not approve the array padding. Running ctest on your branch and approving.

doutriaux1 added a commit that referenced this pull request Nov 11, 2014
@doutriaux1 doutriaux1 merged commit 9473ac5 into master Nov 11, 2014
@doutriaux1 doutriaux1 deleted the 849_prepFillAreaCleanup branch November 18, 2014 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants