[server] Fix layer order rendering when layers are in groups (#8796)

* [server] Fix layer order rendering when layers are in groups Fixes #20810 * Update test images * [server] Fix order of layers in subgroups basically: layers in WMS is specified in a comma separated list where leftmost layer is drawn first (it's at the bottom) and QGIS layer order is exactly the opposite: first layer in the TOP (topmost) is drawn last (it is at the top). For this reason the server reverses the list before passing it to the renderer. * Fix order of layers in test * Update src/server/services/wms/qgswmsrenderer.cpp
qgis · Jan 7, 2019 · 1ba6b97 · 1ba6b97
1 parent 54a5fae
commit 1ba6b97
Show file tree

Hide file tree

Showing 10 changed files with 1,998 additions and 31 deletions.
diff --git a/src/server/services/wms/qgswmsrenderer.cpp b/src/server/services/wms/qgswmsrenderer.cpp
@@ -2860,6 +2860,8 @@ namespace QgsWms
       }
       else if ( mLayerGroups.contains( nickname ) )
       {
+        // Reverse order of layers from a group
+        QList<QgsMapLayer *> layersFromGroup;
         for ( QgsMapLayer *layer : mLayerGroups[nickname] )
         {
           if ( !mRestrictedLayers.contains( layerNickname( *layer ) ) )
@@ -2872,9 +2874,10 @@ namespace QgsWms
                 throw QgsMapServiceException( QStringLiteral( "StyleNotDefined" ), QStringLiteral( "Style \"%1\" does not exist for layer \"%2\"" ).arg( style, layerNickname( *layer ) ) );
               }
             }
-            layers.insert( 0, layer );
+            layersFromGroup.push_front( layer );
           }
         }
+        layers.append( layersFromGroup );
       }
       else
       {

diff --git a/tests/src/python/test_qgsserver.py b/tests/src/python/test_qgsserver.py
@@ -184,9 +184,9 @@ def _img_diff(self, image, control_image, max_diff, max_size_diff=QSize()):
             control.setSizeTolerance(max_size_diff.width(), max_size_diff.height())
         return control.compareImages(control_image, max_diff), control.report()
 
-    def _img_diff_error(self, response, headers, image, max_diff=100, max_size_diff=QSize()):
+    def _img_diff_error(self, response, headers, image, max_diff=100, max_size_diff=QSize(), unittest_data_path='control_images'):
 
-        reference_path = unitTestDataPath('control_images') + '/qgis_server/' + image + '/' + image + '.png'
+        reference_path = unitTestDataPath(unittest_data_path) + '/qgis_server/' + image + '/' + image + '.png'
         self.store_reference(reference_path, response)
 
         self.assertEqual(

diff --git a/tests/src/python/test_qgsserver_accesscontrol.py b/tests/src/python/test_qgsserver_accesscontrol.py
@@ -217,31 +217,9 @@ def _img_diff(self, image, control_image, max_diff, max_size_diff=QSize()):
         return control.compareImages(control_image), control.report()
 
     def _img_diff_error(self, response, headers, image, max_diff=10, max_size_diff=QSize()):
-
-        reference_path = unitTestDataPath('control_images') + '/qgis_server_accesscontrol/' + image + '/' + image + '.png'
-        self.store_reference(reference_path, response)
-
-        self.assertEqual(
-            headers.get("Content-Type"), "image/png",
-            "Content type is wrong: %s" % headers.get("Content-Type"))
-
-        test, report = self._img_diff(response, image, max_diff, max_size_diff)
-
-        with open(os.path.join(tempfile.gettempdir(), image + "_result.png"), "rb") as rendered_file:
-            encoded_rendered_file = base64.b64encode(rendered_file.read())
-            message = "Image is wrong\n%s\nImage:\necho '%s' | base64 -d >%s/%s_result.png" % (
-                report, encoded_rendered_file.strip().decode('utf8'), tempfile.gettempdir(), image
-            )
-
-        # If the failure is in image sizes the diff file will not exists.
-        if os.path.exists(os.path.join(tempfile.gettempdir(), image + "_result_diff.png")):
-            with open(os.path.join(tempfile.gettempdir(), image + "_result_diff.png"), "rb") as diff_file:
-                encoded_diff_file = base64.b64encode(diff_file.read())
-                message += "\nDiff:\necho '%s' | base64 -d > %s/%s_result_diff.png" % (
-                    encoded_diff_file.strip().decode('utf8'), tempfile.gettempdir(), image
-                )
-
-        self.assertTrue(test, message)
+        super()._img_diff_error(response, headers, image, max_diff=max_diff,
+                                max_size_diff=max_size_diff,
+                                unittest_data_path='qgis_server_accesscontrol')
 
     def _geo_img_diff(self, image_1, image_2):
         if os.name == 'nt':

diff --git a/tests/src/python/test_qgsserver_accesscontrol_wms.py b/tests/src/python/test_qgsserver_accesscontrol_wms.py
@@ -213,7 +213,7 @@ def test_wms_getmap_grp(self):
             "SERVICE": "WMS",
             "VERSION": "1.1.1",
             "REQUEST": "GetMap",
-            "LAYERS": "Hello_grp,Country_grp",
+            "LAYERS": "Country_grp,Hello_grp",
             "STYLES": "",
             "FORMAT": "image/png",
             "BBOX": "-16817707,-6318936.5,5696513,16195283.5",

diff --git a/tests/src/python/test_qgsserver_wms_getmap.py b/tests/src/python/test_qgsserver_wms_getmap.py
@@ -41,9 +41,10 @@
 
 
 class TestQgsServerWMSGetMap(QgsServerTestBase):
-
     """QGIS Server WMS Tests for GetMap request"""
 
+    #regenerate_reference = True
+
     def test_wms_getmap_basic_mode(self):
         # 1 bits
         qs = "?" + "&".join(["%s=%s" % i for i in list({
@@ -355,7 +356,7 @@ def test_wms_getmap_invalid_parameters(self):
         err = b"cannot be converted into a rectangle" in r
         self.assertTrue(err)
 
-        # opacities should be a list of int
+        # opacities should be a list of int
         qs = "?" + "&".join(["%s=%s" % i for i in list({
             "MAP": urllib.parse.quote(self.projectPath),
             "SERVICE": "WMS",
@@ -1330,6 +1331,50 @@ def test_wms_getmap_group(self):
         r_group_sld, _ = self._result(self._execute_request(qs))
         self.assertEqual(r_individual, r_group_sld, 'Individual layers query and SLD group layers query results should be identical')
 
+    def test_wms_getmap_group_regression_20810(self):
+        """A WMS shall render the requested layers by drawing the leftmost in the list
+        bottommost, the next one over that, and so on. Even if the layers are inside groups."""
+
+        qs = "?" + "&".join(["%s=%s" % i for i in list({
+            "MAP": urllib.parse.quote(os.path.join(self.testdata_path, 'test_project_wms_grouped_layers.qgs')),
+            "SERVICE": "WMS",
+            "VERSION": "1.3.0",
+            "REQUEST": "GetMap",
+            "BBOX": "613402.5658687877003,5809005.018114360981,619594.408781287726,5813869.006602735259",
+            "CRS": "EPSG:25832",
+            "WIDTH": "429",
+            "HEIGHT": "337",
+            "LAYERS": "osm,areas and symbols",
+            "STYLES": ",",
+            "FORMAT": "image/png",
+            "DPI": "200",
+            "MAP_RESOLUTION": "200",
+            "FORMAT_OPTIONS": "dpi:200"
+        }.items())])
+
+        r, h = self._result(self._execute_request(qs))
+        self._img_diff_error(r, h, "WMS_GetMap_GroupedLayersUp")
+
+        qs = "?" + "&".join(["%s=%s" % i for i in list({
+            "MAP": urllib.parse.quote(os.path.join(self.testdata_path, 'test_project_wms_grouped_layers.qgs')),
+            "SERVICE": "WMS",
+            "VERSION": "1.3.0",
+            "REQUEST": "GetMap",
+            "BBOX": "613402.5658687877003,5809005.018114360981,619594.408781287726,5813869.006602735259",
+            "CRS": "EPSG:25832",
+            "WIDTH": "429",
+            "HEIGHT": "337",
+            "LAYERS": "areas and symbols,osm",
+            "STYLES": ",",
+            "FORMAT": "image/png",
+            "DPI": "200",
+            "MAP_RESOLUTION": "200",
+            "FORMAT_OPTIONS": "dpi:200"
+        }.items())])
+
+        r, h = self._result(self._execute_request(qs))
+        self._img_diff_error(r, h, "WMS_GetMap_GroupedLayersDown")
+
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/...mages/qgis_server/WMS_GetMap_GroupedLayersDown/WMS_GetMap_GroupedLayersDown.png b/...mages/qgis_server/WMS_GetMap_GroupedLayersDown/WMS_GetMap_GroupedLayersDown.png
diff --git a/...ol_images/qgis_server/WMS_GetMap_GroupedLayersUp/WMS_GetMap_GroupedLayersUp.png b/...ol_images/qgis_server/WMS_GetMap_GroupedLayersUp/WMS_GetMap_GroupedLayersUp.png
diff --git a/...estdata/control_images/qgis_server_accesscontrol/WMS_GetMap/WMS_GetMap_mask.png b/...estdata/control_images/qgis_server_accesscontrol/WMS_GetMap/WMS_GetMap_mask.png
diff --git a/tests/testdata/qgis_server/test_project_wms_grouped_layers.gpkg b/tests/testdata/qgis_server/test_project_wms_grouped_layers.gpkg