{"action":"created","comment":{"url":"https://api.github.com/repos/qgis/QGIS/pulls/comments/193606632","pull_request_review_id":126610983,"id":193606632,"node_id":"MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDE5MzYwNjYzMg==","diff_hunk":"@@ -439,59 +430,61 @@ void QgsZonalStatistics::statisticsFromMiddlePointTest( const QgsGeometry &poly,\n   }\n }\n \n-void QgsZonalStatistics::statisticsFromPreciseIntersection( const QgsGeometry &poly, int pixelOffsetX,\n-    int pixelOffsetY, int nCellsX, int nCellsY, double cellSizeX, double cellSizeY, const QgsRectangle &rasterBBox, FeatureStats &stats )\n+void QgsZonalStatistics::statisticsFromPreciseIntersection( const QgsGeometry &poly, int nCellsX, int nCellsY, double cellSizeX, double cellSizeY, const QgsRectangle &rasterBBox, FeatureStats &stats )\n {\n   stats.reset();\n \n-  double currentY = rasterBBox.yMaximum() - pixelOffsetY * cellSizeY - cellSizeY / 2;\n+  double currentY = rasterBBox.yMaximum() - cellSizeY / 2;\n   QgsGeometry pixelRectGeometry;\n \n   double hCellSizeX = cellSizeX / 2.0;\n   double hCellSizeY = cellSizeY / 2.0;\n   double pixelArea = cellSizeX * cellSizeY;\n   double weight = 0;\n \n-  QgsRectangle featureBBox = poly.boundingBox().intersect( &rasterBBox );\n-  QgsRectangle intersectBBox = rasterBBox.intersect( &featureBBox );\n+  std::unique_ptr< QgsGeometryEngine > polyEngine( QgsGeometry::createGeometryEngine( poly.constGet( ) ) );\n+  if ( !polyEngine )\n+  {\n+    return;\n+  }\n+  polyEngine->prepareGeometry();\n \n-  QgsRasterBlock *block = mRasterProvider->block( mRasterBand, intersectBBox, nCellsX, nCellsY );\n+  std::unique_ptr< QgsRasterBlock > block( mRasterInterface->block( mRasterBand, rasterBBox, nCellsX, nCellsY ) );\n   for ( int i = 0; i < nCellsY; ++i )\n   {\n-    double currentX = rasterBBox.xMinimum() + cellSizeX / 2.0 + pixelOffsetX * cellSizeX;\n+    double currentX = rasterBBox.xMinimum() + cellSizeX / 2.0;\n     for ( int j = 0; j < nCellsX; ++j )\n     {\n-      if ( !validPixel( block->value( i, j ) ) )\n-      {\n-        continue;\n-      }\n-\n-      pixelRectGeometry = QgsGeometry::fromRect( QgsRectangle( currentX - hCellSizeX, currentY - hCellSizeY, currentX + hCellSizeX, currentY + hCellSizeY ) );\n-      if ( !pixelRectGeometry.isNull() )\n+      double pixelValue = block->value( i, j );\n+      if ( validPixel( pixelValue ) && !block->isNoData( i, j ) )\n       {\n-        //intersection\n-        QgsGeometry intersectGeometry = pixelRectGeometry.intersection( poly );\n-        if ( !intersectGeometry.isNull() )\n+        pixelRectGeometry = QgsGeometry::fromRect( QgsRectangle( currentX - hCellSizeX, currentY - hCellSizeY, currentX + hCellSizeX, currentY + hCellSizeY ) );\n+        // GEOS intersects tests on prepared geometry is MAGNITUDES faster than calculating the intersection itself,\n+        // so we first test to see if there IS an intersection before doing the actual calculation\n+        if ( !pixelRectGeometry.isNull() && polyEngine->intersects( pixelRectGeometry.constGet() ) )\n         {\n-          double intersectionArea = intersectGeometry.area();\n-          if ( intersectionArea >= 0.0 )\n+          //intersection\n+          QgsGeometry intersectGeometry = pixelRectGeometry.intersection( poly );\n+          if ( !intersectGeometry.isEmpty() )\n           {\n-            weight = intersectionArea / pixelArea;\n-            stats.addValue( block->value( i, j ), weight );\n+            double intersectionArea = intersectGeometry.area();\n+            if ( intersectionArea > 0.0 )\n+            {\n+              weight = intersectionArea / pixelArea;\n+              stats.addValue( pixelValue, weight );\n+            }\n           }\n         }\n-        pixelRectGeometry = QgsGeometry();\n       }\n       currentX += cellSizeX;\n     }\n     currentY -= cellSizeY;\n   }\n-  delete block;\n }\n \n-bool QgsZonalStatistics::validPixel( float value ) const\n+bool QgsZonalStatistics::validPixel( double value ) const\n {\n-  return !( value == mInputNodataValue || std::isnan( value ) );\n+  return !std::isnan( value );","path":"src/analysis/vector/qgszonalstatistics.cpp","position":330,"original_position":330,"commit_id":"c4f960b5de0aa584ee47e71283a20b153e903642","original_commit_id":"c4f960b5de0aa584ee47e71283a20b153e903642","user":{"login":"nirvn","id":1728657,"node_id":"MDQ6VXNlcjE3Mjg2NTc=","avatar_url":"https://avatars0.githubusercontent.com/u/1728657?v=4","gravatar_id":"","url":"https://api.github.com/users/nirvn","html_url":"https://github.com/nirvn","followers_url":"https://api.github.com/users/nirvn/followers","following_url":"https://api.github.com/users/nirvn/following{/other_user}","gists_url":"https://api.github.com/users/nirvn/gists{/gist_id}","starred_url":"https://api.github.com/users/nirvn/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nirvn/subscriptions","organizations_url":"https://api.github.com/users/nirvn/orgs","repos_url":"https://api.github.com/users/nirvn/repos","events_url":"https://api.github.com/users/nirvn/events{/privacy}","received_events_url":"https://api.github.com/users/nirvn/received_events","type":"User","site_admin":false},"body":"@nyalldawson , \"does this have to be a function?\" :smile: ","created_at":"2018-06-07T01:48:33Z","updated_at":"2018-06-07T01:48:34Z","html_url":"https://github.com/qgis/QGIS/pull/7190#discussion_r193606632","pull_request_url":"https://api.github.com/repos/qgis/QGIS/pulls/7190","author_association":"CONTRIBUTOR","_links":{"self":{"href":"https://api.github.com/repos/qgis/QGIS/pulls/comments/193606632"},"html":{"href":"https://github.com/qgis/QGIS/pull/7190#discussion_r193606632"},"pull_request":{"href":"https://api.github.com/repos/qgis/QGIS/pulls/7190"}},"in_reply_to_id":193606060},"pull_request":{"url":"https://api.github.com/repos/qgis/QGIS/pulls/7190","id":192964671,"node_id":"MDExOlB1bGxSZXF1ZXN0MTkyOTY0Njcx","html_url":"https://github.com/qgis/QGIS/pull/7190","diff_url":"https://github.com/qgis/QGIS/pull/7190.diff","patch_url":"https://github.com/qgis/QGIS/pull/7190.patch","issue_url":"https://api.github.com/repos/qgis/QGIS/issues/7190","number":7190,"state":"open","locked":false,"title":"Zonal stats fixes and improvements","user":{"login":"nyalldawson","id":1829991,"node_id":"MDQ6VXNlcjE4Mjk5OTE=","avatar_url":"https://avatars1.githubusercontent.com/u/1829991?v=4","gravatar_id":"","url":"https://api.github.com/users/nyalldawson","html_url":"https://github.com/nyalldawson","followers_url":"https://api.github.com/users/nyalldawson/followers","following_url":"https://api.github.com/users/nyalldawson/following{/other_user}","gists_url":"https://api.github.com/users/nyalldawson/gists{/gist_id}","starred_url":"https://api.github.com/users/nyalldawson/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nyalldawson/subscriptions","organizations_url":"https://api.github.com/users/nyalldawson/orgs","repos_url":"https://api.github.com/users/nyalldawson/repos","events_url":"https://api.github.com/users/nyalldawson/events{/privacy}","received_events_url":"https://api.github.com/users/nyalldawson/received_events","type":"User","site_admin":false},"body":"This PR fixes a bunch of identified issues with the zonal stats code, and greatly speeds up the algorithm in the case of small polygons vs pixel sizes. Lots of new tests too!\r\n","created_at":"2018-06-06T10:12:46Z","updated_at":"2018-06-07T01:48:34Z","closed_at":null,"merged_at":null,"merge_commit_sha":"dd4291261f9d7d095c73e1ec5cbd06b7ec1164bd","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[],"milestone":null,"commits_url":"https://api.github.com/repos/qgis/QGIS/pulls/7190/commits","review_comments_url":"https://api.github.com/repos/qgis/QGIS/pulls/7190/comments","review_comment_url":"https://api.github.com/repos/qgis/QGIS/pulls/comments{/number}","comments_url":"https://api.github.com/repos/qgis/QGIS/issues/7190/comments","statuses_url":"https://api.github.com/repos/qgis/QGIS/statuses/c4f960b5de0aa584ee47e71283a20b153e903642","head":{"label":"nyalldawson:zonal_stats","ref":"zonal_stats","sha":"c4f960b5de0aa584ee47e71283a20b153e903642","user":{"login":"nyalldawson","id":1829991,"node_id":"MDQ6VXNlcjE4Mjk5OTE=","avatar_url":"https://avatars1.githubusercontent.com/u/1829991?v=4","gravatar_id":"","url":"https://api.github.com/users/nyalldawson","html_url":"https://github.com/nyalldawson","followers_url":"https://api.github.com/users/nyalldawson/followers","following_url":"https://api.github.com/users/nyalldawson/following{/other_user}","gists_url":"https://api.github.com/users/nyalldawson/gists{/gist_id}","starred_url":"https://api.github.com/users/nyalldawson/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nyalldawson/subscriptions","organizations_url":"https://api.github.com/users/nyalldawson/orgs","repos_url":"https://api.github.com/users/nyalldawson/repos","events_url":"https://api.github.com/users/nyalldawson/events{/privacy}","received_events_url":"https://api.github.com/users/nyalldawson/received_events","type":"User","site_admin":false},"repo":{"id":8490349,"node_id":"MDEwOlJlcG9zaXRvcnk4NDkwMzQ5","name":"QGIS","full_name":"nyalldawson/QGIS","owner":{"login":"nyalldawson","id":1829991,"node_id":"MDQ6VXNlcjE4Mjk5OTE=","avatar_url":"https://avatars1.githubusercontent.com/u/1829991?v=4","gravatar_id":"","url":"https://api.github.com/users/nyalldawson","html_url":"https://github.com/nyalldawson","followers_url":"https://api.github.com/users/nyalldawson/followers","following_url":"https://api.github.com/users/nyalldawson/following{/other_user}","gists_url":"https://api.github.com/users/nyalldawson/gists{/gist_id}","starred_url":"https://api.github.com/users/nyalldawson/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nyalldawson/subscriptions","organizations_url":"https://api.github.com/users/nyalldawson/orgs","repos_url":"https://api.github.com/users/nyalldawson/repos","events_url":"https://api.github.com/users/nyalldawson/events{/privacy}","received_events_url":"https://api.github.com/users/nyalldawson/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/nyalldawson/QGIS","description":"QGIS is a free, open source, cross platform (lin/win/mac) geographical information system (GIS)","fork":true,"url":"https://api.github.com/repos/nyalldawson/QGIS","forks_url":"https://api.github.com/repos/nyalldawson/QGIS/forks","keys_url":"https://api.github.com/repos/nyalldawson/QGIS/keys{/key_id}","collaborators_url":"https://api.github.com/repos/nyalldawson/QGIS/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/nyalldawson/QGIS/teams","hooks_url":"https://api.github.com/repos/nyalldawson/QGIS/hooks","issue_events_url":"https://api.github.com/repos/nyalldawson/QGIS/issues/events{/number}","events_url":"https://api.github.com/repos/nyalldawson/QGIS/events","assignees_url":"https://api.github.com/repos/nyalldawson/QGIS/assignees{/user}","branches_url":"https://api.github.com/repos/nyalldawson/QGIS/branches{/branch}","tags_url":"https://api.github.com/repos/nyalldawson/QGIS/tags","blobs_url":"https://api.github.com/repos/nyalldawson/QGIS/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/nyalldawson/QGIS/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/nyalldawson/QGIS/git/refs{/sha}","trees_url":"https://api.github.com/repos/nyalldawson/QGIS/git/trees{/sha}","statuses_url":"https://api.github.com/repos/nyalldawson/QGIS/statuses/{sha}","languages_url":"https://api.github.com/repos/nyalldawson/QGIS/languages","stargazers_url":"https://api.github.com/repos/nyalldawson/QGIS/stargazers","contributors_url":"https://api.github.com/repos/nyalldawson/QGIS/contributors","subscribers_url":"https://api.github.com/repos/nyalldawson/QGIS/subscribers","subscription_url":"https://api.github.com/repos/nyalldawson/QGIS/subscription","commits_url":"https://api.github.com/repos/nyalldawson/QGIS/commits{/sha}","git_commits_url":"https://api.github.com/repos/nyalldawson/QGIS/git/commits{/sha}","comments_url":"https://api.github.com/repos/nyalldawson/QGIS/comments{/number}","issue_comment_url":"https://api.github.com/repos/nyalldawson/QGIS/issues/comments{/number}","contents_url":"https://api.github.com/repos/nyalldawson/QGIS/contents/{+path}","compare_url":"https://api.github.com/repos/nyalldawson/QGIS/compare/{base}...{head}","merges_url":"https://api.github.com/repos/nyalldawson/QGIS/merges","archive_url":"https://api.github.com/repos/nyalldawson/QGIS/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/nyalldawson/QGIS/downloads","issues_url":"https://api.github.com/repos/nyalldawson/QGIS/issues{/number}","pulls_url":"https://api.github.com/repos/nyalldawson/QGIS/pulls{/number}","milestones_url":"https://api.github.com/repos/nyalldawson/QGIS/milestones{/number}","notifications_url":"https://api.github.com/repos/nyalldawson/QGIS/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/nyalldawson/QGIS/labels{/name}","releases_url":"https://api.github.com/repos/nyalldawson/QGIS/releases{/id}","deployments_url":"https://api.github.com/repos/nyalldawson/QGIS/deployments","created_at":"2013-02-28T23:20:44Z","updated_at":"2017-09-11T19:40:55Z","pushed_at":"2018-06-07T00:23:53Z","git_url":"git://github.com/nyalldawson/QGIS.git","ssh_url":"git@github.com:nyalldawson/QGIS.git","clone_url":"https://github.com/nyalldawson/QGIS.git","svn_url":"https://github.com/nyalldawson/QGIS","homepage":"http://qgis.org","size":1276169,"stargazers_count":2,"watchers_count":2,"language":"C++","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":1,"mirror_url":null,"archived":false,"open_issues_count":0,"license":{"key":"gpl-2.0","name":"GNU General Public License v2.0","spdx_id":"GPL-2.0","url":"https://api.github.com/licenses/gpl-2.0","node_id":"MDc6TGljZW5zZTg="},"forks":1,"open_issues":0,"watchers":2,"default_branch":"master"}},"base":{"label":"qgis:master","ref":"master","sha":"b2274258a4240abe1335a038bb70f66a9aadabbc","user":{"login":"qgis","id":483444,"node_id":"MDEyOk9yZ2FuaXphdGlvbjQ4MzQ0NA==","avatar_url":"https://avatars3.githubusercontent.com/u/483444?v=4","gravatar_id":"","url":"https://api.github.com/users/qgis","html_url":"https://github.com/qgis","followers_url":"https://api.github.com/users/qgis/followers","following_url":"https://api.github.com/users/qgis/following{/other_user}","gists_url":"https://api.github.com/users/qgis/gists{/gist_id}","starred_url":"https://api.github.com/users/qgis/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/qgis/subscriptions","organizations_url":"https://api.github.com/users/qgis/orgs","repos_url":"https://api.github.com/users/qgis/repos","events_url":"https://api.github.com/users/qgis/events{/privacy}","received_events_url":"https://api.github.com/users/qgis/received_events","type":"Organization","site_admin":false},"repo":{"id":1690480,"node_id":"MDEwOlJlcG9zaXRvcnkxNjkwNDgw","name":"QGIS","full_name":"qgis/QGIS","owner":{"login":"qgis","id":483444,"node_id":"MDEyOk9yZ2FuaXphdGlvbjQ4MzQ0NA==","avatar_url":"https://avatars3.githubusercontent.com/u/483444?v=4","gravatar_id":"","url":"https://api.github.com/users/qgis","html_url":"https://github.com/qgis","followers_url":"https://api.github.com/users/qgis/followers","following_url":"https://api.github.com/users/qgis/following{/other_user}","gists_url":"https://api.github.com/users/qgis/gists{/gist_id}","starred_url":"https://api.github.com/users/qgis/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/qgis/subscriptions","organizations_url":"https://api.github.com/users/qgis/orgs","repos_url":"https://api.github.com/users/qgis/repos","events_url":"https://api.github.com/users/qgis/events{/privacy}","received_events_url":"https://api.github.com/users/qgis/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/qgis/QGIS","description":"QGIS is a free, open source, cross platform (lin/win/mac) geographical information system (GIS)","fork":false,"url":"https://api.github.com/repos/qgis/QGIS","forks_url":"https://api.github.com/repos/qgis/QGIS/forks","keys_url":"https://api.github.com/repos/qgis/QGIS/keys{/key_id}","collaborators_url":"https://api.github.com/repos/qgis/QGIS/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/qgis/QGIS/teams","hooks_url":"https://api.github.com/repos/qgis/QGIS/hooks","issue_events_url":"https://api.github.com/repos/qgis/QGIS/issues/events{/number}","events_url":"https://api.github.com/repos/qgis/QGIS/events","assignees_url":"https://api.github.com/repos/qgis/QGIS/assignees{/user}","branches_url":"https://api.github.com/repos/qgis/QGIS/branches{/branch}","tags_url":"https://api.github.com/repos/qgis/QGIS/tags","blobs_url":"https://api.github.com/repos/qgis/QGIS/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/qgis/QGIS/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/qgis/QGIS/git/refs{/sha}","trees_url":"https://api.github.com/repos/qgis/QGIS/git/trees{/sha}","statuses_url":"https://api.github.com/repos/qgis/QGIS/statuses/{sha}","languages_url":"https://api.github.com/repos/qgis/QGIS/languages","stargazers_url":"https://api.github.com/repos/qgis/QGIS/stargazers","contributors_url":"https://api.github.com/repos/qgis/QGIS/contributors","subscribers_url":"https://api.github.com/repos/qgis/QGIS/subscribers","subscription_url":"https://api.github.com/repos/qgis/QGIS/subscription","commits_url":"https://api.github.com/repos/qgis/QGIS/commits{/sha}","git_commits_url":"https://api.github.com/repos/qgis/QGIS/git/commits{/sha}","comments_url":"https://api.github.com/repos/qgis/QGIS/comments{/number}","issue_comment_url":"https://api.github.com/repos/qgis/QGIS/issues/comments{/number}","contents_url":"https://api.github.com/repos/qgis/QGIS/contents/{+path}","compare_url":"https://api.github.com/repos/qgis/QGIS/compare/{base}...{head}","merges_url":"https://api.github.com/repos/qgis/QGIS/merges","archive_url":"https://api.github.com/repos/qgis/QGIS/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/qgis/QGIS/downloads","issues_url":"https://api.github.com/repos/qgis/QGIS/issues{/number}","pulls_url":"https://api.github.com/repos/qgis/QGIS/pulls{/number}","milestones_url":"https://api.github.com/repos/qgis/QGIS/milestones{/number}","notifications_url":"https://api.github.com/repos/qgis/QGIS/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/qgis/QGIS/labels{/name}","releases_url":"https://api.github.com/repos/qgis/QGIS/releases{/id}","deployments_url":"https://api.github.com/repos/qgis/QGIS/deployments","created_at":"2011-05-02T08:06:26Z","updated_at":"2018-06-07T01:33:51Z","pushed_at":"2018-06-07T01:33:39Z","git_url":"git://github.com/qgis/QGIS.git","ssh_url":"git@github.com:qgis/QGIS.git","clone_url":"https://github.com/qgis/QGIS.git","svn_url":"https://github.com/qgis/QGIS","homepage":"http://qgis.org","size":1418076,"stargazers_count":2059,"watchers_count":2059,"language":"C++","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":1254,"mirror_url":null,"archived":false,"open_issues_count":52,"license":{"key":"gpl-2.0","name":"GNU General Public License v2.0","spdx_id":"GPL-2.0","url":"https://api.github.com/licenses/gpl-2.0","node_id":"MDc6TGljZW5zZTg="},"forks":1254,"open_issues":52,"watchers":2059,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/qgis/QGIS/pulls/7190"},"html":{"href":"https://github.com/qgis/QGIS/pull/7190"},"issue":{"href":"https://api.github.com/repos/qgis/QGIS/issues/7190"},"comments":{"href":"https://api.github.com/repos/qgis/QGIS/issues/7190/comments"},"review_comments":{"href":"https://api.github.com/repos/qgis/QGIS/pulls/7190/comments"},"review_comment":{"href":"https://api.github.com/repos/qgis/QGIS/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/qgis/QGIS/pulls/7190/commits"},"statuses":{"href":"https://api.github.com/repos/qgis/QGIS/statuses/c4f960b5de0aa584ee47e71283a20b153e903642"}},"author_association":"CONTRIBUTOR"}}
{"actor":{"display_login":"nirvn"}}