-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Fix bug #9360-revival: fix whole layer not rendered (when simplify geometry activated) #1087
Conversation
- stick to C API - enable simplification only with GDAL >=1.9
@ahuarte47 , small clarification: I can reproduce the polygon layer not rendering regression (the second test project) using the qgis master build based on commit 174577b (debian-nightly ppa hosted on qgis.org). That build doesn't include the C++ -> C commit by @jef-n. |
ok @nirvn, thanks, I will test it, |
OGRGeometryH g = OGR_G_CreateGeometry( isaLinearRing ? wkbLinearRing : wkbLineString ); | ||
|
||
for (int i = 0; i < numPoints; i++, points++) | ||
OGR_G_SetPoint( g, i, points->x, points->y, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @jef-n, I released an experimental commit, to show the problem: this 'OGR_G_SetPoint' is a terrible bottleneck because of it does a realloc of one point each new point.
The only solution that I see is disable the simplification on ogr provider side when "__cplusplus" is not enabled, I can do it because of the provider capabalities indicates if it can simplify.
What do you think ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using GEOS.... there is other option:
GEOSCoordSequence* seq = GEOSCoordSeq_create(unsigned int size, unsigned int dims);
...
GEOSGeometry * g = GEOSGeom_createLinearRing(GEOSCoordSequence* s);
char * wkb = GEOSGeomToWKT(const GEOSGeometry* g);
Do you agree ?
NOTE:
OGR provider has not references to GEOS, it is the above option better ? (Disable ogr simplification ifndef "__cplusplus" )
Hi @nirvn, I see the layer disappear for scales ~1:499.973.983 I think that it occurs because of other error not present in my patch. When a layer is rendered, the current map extent is transformed to layer coordinates (using the CRS of the layer), this transformation for so big extreme scales returns empty and any geometry is fetched to be drawed. The error occurs https://github.com/qgis/QGIS/blob/master/src/core/qgsmaprenderer.cpp#L414. I think this error is not caused by my patch, you may please disable simplification and verify the layer disappears? If it is true, we can open one specific issue to fix it
|
@ahuarte47 indeed, the point layer disappears in the extreme zoomed out scales even with simplification disabled. My friend, this is not your problem anymore ;) Will file an issue with a test case project later this week. Keep up the excellent work your doing, all very appreciated. |
Hi @nirvn, thank you very much for your support :-) About this error, it is located in (https://github.com/qgis/QGIS/blob/master/src/core/qgsmaprenderer.cpp#L414): the code tries to transform a big extent from EPSG:4326 (map CRS) to the layer CRS and it returns an empty extent (then any geometry is fetched to be drawed). When the new issue be added, I can try fix it.
|
Ok, I disable ogr simplification ( snif! snif! ) while OGR_G_SetPoints does not exist, I add it to GDAL-OGR! Thanks @jef-n |
Hi @jef-n, my pull request in GDAL has beed merged, I guess that it will included in 1.11 |
Hi @jef-n, Are you agree with the current state of this patch ? Best Regards |
Thanks @jef-n |
Ths patch fixes the bug #9360 when the code was translated from OGRGeometry* to OGRGeometryH classes in commit (3305a6c)
Changes: