Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Browse files
Browse the repository at this point in the history
Fix WMS 1.3. compliance: distort image if width/height ratio of bbox …
…is different to WIDTH/HEIGHT of requested image Not decrease but increase image
- Loading branch information
d44f1eb
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 @mhugent and @pka, I have made this commit to fix an issue in QGIS Server. We test it in QGIS as a WMS client with @elpaso.
It's seems the fix introduces an issue with OpenLayers3. I have tested it with OpenLayers2 and don't have any issue.
Do you have any other client which use distortion ?
d44f1eb
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.
This 'fix' breaks the OGC compliance of QGIS server! You can run the OGC test suite to see so, there is a visual test showing two pictures with the following question: 'The two pictures above should depict the same scene, but the picture on the right should be stretched so it is twice as tall as the picture on the left. Are the pictures correct?'. With the original changes, this is the case. With this 'fix', the picture on the right is only half the size of the left picture (but it should be stretched to be double size).
d44f1eb
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.
Thanks @mhugent but without this fix, QGIS Desktop can't use QGIS Server as a WMS 1.3.0 provider. Do we have an issue in QGIS Desktop WMS provider ?
With @yjacolin, we produced the same request with MapServer and GeoServer, and with my fix we get the same rendering.
During the hackfest, @vmora has working on reproduce the cite tests. But don't have enough time to do it.
If QGIS Desktop WMS provider has no issue with distort images, my fix is not enough, if we want QGIS Server compliance and rendering as others.
d44f1eb
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.
QGIS desktop (WMS client) sometimes makes requests where width/height ratio and bbox do not match. So yes, this is a bug and it should be fixed.
d44f1eb
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.
would make sense to have two set of tests?
the 2 test suite would be good to test against at leaset two map servers (e.g. qgis server and mapserver/geoserver). In this way we could be able to find "endogamy" related errors like this one.
d44f1eb
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.
The project https://github.com/camptocamp/ms_perfs can help to compare images produces by QGIS Server 2.18, QGIS Server 3, MapServer and GeoServer. It provides the same images from the same layers.
During the French QGIS User meetings, @yjacolin has presented this project http://www.agrotic.org/blog/wp-content/uploads/2016/12/02_performance_qgis_server.pdf. On slide 11 to 13, there are examples of distort images provided by MapServer, GeoServer and QGIS Server before this commit. You can see that QGIS Server does not provide an image in the same way as others.
So to pass CITE tests, we have to enhance this commit and not just revert it.
d44f1eb
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.
@rldhont for the OL3 part I've suggested a patch that seems to mitigate the problem.
We were facing serious issues caused by roundings and the new QGIS strict behaviour. Changing tis rounding strategy has solved it.
d44f1eb
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.
just as a followp @rldhont I've committed a PR to OL3 that is going to be merged that solves the problem at least for this client + QGIS Server
d44f1eb
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 @giohappy I have updated the way QGIS Server stretched image #3906
We used the same method as MapServer, and I fixed an issue due to reverse axis in EPSG:4326. I hope all will be ok.
d44f1eb
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 @rldhont no problem, other clients will benefit from it. OL3 will behave correctly (coherent axpect ratio for BBOX and image sizes) independently of the server strategy.
d44f1eb
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.
PS: and we needed it to work for QGIS Server LTR (2.14.10) which is affected by the issue.
d44f1eb
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.
Will this fix trigger new point releases?