Skip to content

Comments

Fix intervaltree min max#3

Merged
BenFrantzDale merged 4 commits intoFormlabs:masterfrom
BenFrantzDale:fix-intervaltree-min-max
Apr 3, 2019
Merged

Fix intervaltree min max#3
BenFrantzDale merged 4 commits intoFormlabs:masterfrom
BenFrantzDale:fix-intervaltree-min-max

Conversation

@BenFrantzDale
Copy link

No description provided.

IntervalStartCmp());
if (!ivals.empty()) {
center = (minmaxStart.first->start + minmaxStop.second->stop) / 2;
{ // This scope is because afterward we sort and so these two iterators
Copy link
Author

Choose a reason for hiding this comment

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

This is just scoping

@@ -160,15 +164,27 @@ class IntervalTree {

Scalar min() const {
assert(!empty());
Copy link
Author

Choose a reason for hiding this comment

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

These are the fixes.

return Catch::Session().run( argc, argv );
}

TEST_CASE( "Test min/max" ) {
Copy link
Author

Choose a reason for hiding this comment

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

This tests the fixes.

Copy link

@will-formlabs will-formlabs left a comment

Choose a reason for hiding this comment

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

Looks good

Copy link

@dbiswas2808 dbiswas2808 left a comment

Choose a reason for hiding this comment

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

LGTM. Just a thought, is it a good idea to just a cache the min and max when we are building the tree for each level?

@BenFrantzDale BenFrantzDale merged commit c1f7048 into Formlabs:master Apr 3, 2019
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.

3 participants