-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Fix ImageTile/DataTile source hidpi reprojection #16280
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
Conversation
📦 Preview the website for this branch here: https://deploy-preview-16280--ol-site.netlify.app/. |
9a549ea
to
03170aa
Compare
8090cdc
to
3f18bae
Compare
3f18bae
to
578d8b7
Compare
578d8b7
to
4f3c6ca
Compare
b52f82c
to
18791a2
Compare
9d42449
to
79ff6fa
Compare
@ahocevar This is ready for review as it fixes the immediate bugs. In In |
In common with other hidpi rendering tests a27fba6 only demonstrates that when correctly configured the output is correct in a standard dpi screenshot (i.e. all of the source tile is used and all the reprojected data is exported in the reproj tile image). There are currently no rendering tests which demonstrate the map canvas is genuinely hidpi. If and much sharper with d983fa2 particularly noticeable with the labels in the top right quadrant. A regular rendering test would not detect the difference in a standard dpi screenshot. A test which scaled the map div might work if it was compliant with the limitations of #14199
|
examples/xyz-retina.js
Outdated
tileSize: 512, | ||
tileGrid: createXYZ({tileSize: 256, maxZoom: 22}), |
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.
Why is this needed?
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.
It is equivalent to specifying tilePixelRatio: 2
in ol/source/XYZ
. It is not needed in that example any more than tilePixelRatio
was when the example used ol/source/XYZ
as there is no reprojection, but as then it is the correct way to specify the source in case it is reprojected.
Since DataTile sources can have different pixel ratios in each dimension there is no tilePixelRatio
option, see #13648. However, I do think having a tilePixelRatio
option for ol/source/ImageTile
would be a nice future enhancement, where unless both tileSize
and tileGrid
were specified they could be calculated and passed to super()
.
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.
I have now added a tilePixelRatio
option so the source can be set up in a simpler way consistent with ol/source/XYZ
(based on the code from #16279 which ensures data sizes are integer even if tile grid sizes are not). The new hidpi map rendering test is using it.
961f4c0
to
67a6d8b
Compare
7d82d57
to
3e41540
Compare
Fixes #16278
Fixes the ImageTile source problems described in #16278, which dated from #15905.
Updates the source setup in the example so it would work if reprojected.
Also fixes the hidpi DataTile problem described in #16278 which was introduced by #15860, adds a test and update results for two others where more pixels are made available in reprojection.
Incorrect reprojection when a hidpi ImageTile source is not defined as hidpi is also fixed.