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

Bug fixes #116

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

Bug fixes #116

wants to merge 19 commits into from

Conversation

embeddedmz
Copy link

I mainly fixed this issue #114 and other problems related to vtkMapMarkerSet (IDs are not unique, DeleteMarker bugs/leaks, recompute clusters, etc...).

There's also an important fix I made in vtkFeatureLayer that allows VTK smart pointers to correctly free the instances of it (replaced the overriden Delete by Unregister as smart pointers don't use Delete to decrement the reference count).

The commits comments provides a more exhaustive description of changes.

Thanks.

Amine Mzoughi and others added 11 commits March 26, 2019 12:34
Fixed UI objects not freed in destructor.
Fixed compiler warnings.
… to correctly destroy the objects and prevent leaks.

Fixed RemoveFeature not coded correctly.
Replaced Features vector pointers by vtkSmartPointers.
…he multithreaded OSM layer.

Fixed compiler warning.
…s after calling DeleteMarker.

Fixed bugs and leaks in DeleteMarker.
Added DeleteAllMarkers method.
Removed unused header files inclusions.
Replaced preprocessor macros by global and file scoped variables and/or enums.
Fixed paletteSize computed incorrectly.
Fixed compiler warnings.
Added a debugging method to dump nodes table.
Fixed 'Observer' not freed in destructor.
Refactored RecomputeClusters to preserve marker IDs.
Replaced copies of node table set by references.
Fixed some comments.
Removed semicolons after VTK Get/Set macros.
@johnkit
Copy link
Contributor

johnkit commented Aug 23, 2019

@embeddedmz Thank you for another major contribution. I will try to review in the next few days. Is there a simple test program that demonstrates the delete-marker problem?

@embeddedmz
Copy link
Author

embeddedmz commented Aug 23, 2019

Hi @johnkit

You can simply take the weather station application, add this code in void qtWeatherStations::showStations() :
// remove old markers for (const auto& id : StationMap) { if (!this->MapMarkers->DeleteMarker(id.first)) { std::cout << "Error: unable to delete marker " << id.first << std::endl; } }
before this instruction (deletion of old stations marker is missing... by the way, this code is already available in the pull request commits)
this->StationMap.clear();

Then, compile and launch the program, create some stations, (7) they will be displayed correctly, then create another number of stations (test with numbers != 7).

In short, the problem is that the IDs are not generated uniquely : if you have 3 markers (0, 1, 2) and then you decide to delete marker 1, the next marker you will create (the fourth one since the beginning) will have 2 as an ID, but 2 is already assigned (to the third created marker).

The variable used to assign IDs to added markers must not be decremented when we delete a marker. Moreover, using a map is better than using a vector to store marker nodes : when a marker is erased, that creates fragmentation in std::vector that needs to be handled and if it's handled, we can't access marker nodes with their IDs directly (that's why a map is more adequate).

@embeddedmz
Copy link
Author

Recently, I noticed that vtkMap is unable to download map tiles from openstreetmap server. When I specified a user agent (e.g. vtkMap), I was again able to download the tiles.

I will add the commit in this pull request !

@embeddedmz
Copy link
Author

Hi @johnkit
I made a small change in core/CMakeLists.txt to fix libcurl include dir not being set for examples projects in Visual Studio 2019 (generated with the latest version of CMake).
This pull request is getting bigger and bigger and the last commit is not even related to the others.
I hope that you can find the time to merge the commits so I can create a new pull request.
Thanks.

@embeddedmz
Copy link
Author

@johnkit To remove the ugly discontinuities between tiles (or any image textured planes in VTK), multi sampling must be disabled AND vtkTexture's interpolation must not be activated. The map is now displayed correctly. I didn't find a way to keep MSAA and display tiles without tampering their borders.

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