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

STL assert while doing union for two polys #45

Open
enduguXL opened this issue Nov 1, 2016 · 15 comments
Open

STL assert while doing union for two polys #45

enduguXL opened this issue Nov 1, 2016 · 15 comments

Comments

@enduguXL
Copy link

enduguXL commented Nov 1, 2016

I suspect that I'm doing something fundamentally wrong.
VS2015. LLVM-vs2014 toolset has the same effect.

`

#define TYPE std::int64_t
mapbox::geometry::wagyu::wagyu<TYPE> Wagyu;

bool bRes;

//A	
mapbox::geometry::linear_ring<TYPE> RingA;

RingA.push_back({ 200986, 661584 });
RingA.push_back({ 100000, 670000 });
RingA.push_back({ 45000,  648310 });
RingA.push_back({ 45715,  125631 });

bRes = Wagyu.add_ring(RingA);

//B
mapbox::geometry::linear_ring<TYPE> RingB;

RingB.push_back({ 45000,   122869 });
RingB.push_back({ 45000,   648310 });
RingB.push_back({ -610000, 390000 });
RingB.push_back({ -650000, 220000 });
RingB.push_back({ -660000, 60000 });
RingB.push_back({ -610000, -320000 });
RingB.push_back({ -409999, -459999 });
RingB.push_back({ -50000,  -550000 });
RingB.push_back({ 150000,  -550000 });
RingB.push_back({ 470000,  -409999 });
RingB.push_back({ 730000,  -50000 });
RingB.push_back({ 750000,  80000 });
RingB.push_back({ 350000,  600000 });
RingB.push_back({ 227060,  656741 });
RingB.push_back({ 62080,   110243 });
RingB.push_back({ 45000,   115399 });
RingB.push_back({ 42933,   116023 });

bRes = Wagyu.add_ring(RingB);

//
mapbox::geometry::multi_polygon<TYPE> OutPolys;
bRes = Wagyu.execute(mapbox::geometry::wagyu::clip_type_union, OutPolys, mapbox::geometry::wagyu::fill_type_non_zero, mapbox::geometry::wagyu::fill_type_non_zero);

`

@flippmoke
Copy link
Member

How are you compiling the code above? I am not having any issue with this code - though I may have different build flags.

https://github.com/mapbox/wagyu/blob/master/Makefile#L3

Are you using -std=c++11?

@enduguXL
Copy link
Author

enduguXL commented Nov 3, 2016

No, because this is GCC switch and I'm using both Visual Studio 2015 Compiler and Clang.
I will try to build it with GCC (on Windows) in next couple of days.

@enduguXL
Copy link
Author

enduguXL commented Nov 8, 2016

I've finally found the time to sort it out.
MinGW (GCC 6.1.0) built code doesn't crashes.
But msvs2015/Clang built does.

It produces "list iterator not incrementable" assert for the following code:

        auto bnd = active_bounds.begin();
        auto bnd_next = std::next(bnd);

(\mapbox\geometry\wagyu\intersect_util.hpp, line 82)
And active_bounds.size() == 0

@flippmoke
Copy link
Member

@enduguXL are you using the latest from master? What geometry/test is this failing on?

@enduguXL
Copy link
Author

enduguXL commented Nov 8, 2016

Yes, I think so (my SVN path is https://github.com/mapbox/wagyu.git/trunk).
This is piece of code that I've posted in the first comment.

flippmoke added a commit that referenced this issue Nov 8, 2016
@flippmoke
Copy link
Member

@enduguXL I just pushed a new fix that hopefully will resolve that issue, can you confirm?

@enduguXL
Copy link
Author

enduguXL commented Nov 8, 2016

Confirming.
This one is resolved.
But same assert triggered in the other place:

template <typename T>
void next_edge_in_bound(active_bound_list_itr<T>& bnd, scanbeam_list<T>& scanbeam) {
    ++((*bnd)->current_edge);
    ++((*bnd)->next_edge);

mapbox\geometry\wagyu\active_bound_list.hpp, line 165
(*bnd)->next_edge is NULL

Looks like the problem is the same: you are incrementing iterators but not checking them for end() / NULL prior to this.
I think if you build with clang you will get the same results.

@flippmoke
Copy link
Member

@enduguXL I use clang, not sure why this isn't triggering on any of my builds...

@flippmoke
Copy link
Member

another fix pushed

@enduguXL
Copy link
Author

enduguXL commented Nov 9, 2016

Are you really sure about clang? The makefile you've posted above is for GCC because compiler options there are in GCC format. For example, clang doesn't understand -std=c++11, it says "unknown argument ignored in clang-cl".

Anyway, thanks for the fix, I'll check it later today when I return home.

@flippmoke
Copy link
Member

@enduguXL - definitely using clang and clang supports -std=c++11
http://clang.llvm.org/cxx_status.html

@enduguXL enduguXL changed the title CRT assert while doing union for two polys STL assert while doing union for two polys Nov 10, 2016
@enduguXL
Copy link
Author

Looks like it's an STL assert in Microsoft's STL implementation. In Release configuration it works, producing no asserts.
So if you not supporting Microsoft Visual Studio, you can close the issue.

@flippmoke
Copy link
Member

flippmoke commented Nov 10, 2016

@enduguXL I am definitely supporting Microsoft Visual Studio, but I do not have a setup for this currently - so I would love to make sure that everything compiles properly!

Microsoft's compilers are typically stricter, so this is often a very good thing!

@enduguXL
Copy link
Author

Sounds good!
Maybe I could fix some of these asserts while you don't have Microsoft environment. When I have time.
What is the best way to contribute? Branch? Patch?

@flippmoke
Copy link
Member

Creating a new pull request is the easiest way by far to contribute any changes to the code.

Additionally we should look into getting wagyu to build on a windows test environment, much like we do with travis currently.

/cc @springmeyer @BergWerkGIS

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

No branches or pull requests

2 participants