Skip to content
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

add circle vector tile layer support #41584

Merged
merged 4 commits into from
Feb 19, 2021

Conversation

PeterPetrik
Copy link
Contributor

fix #41529

Screen Shot 2021-02-16 at 13 57 45

@PeterPetrik
Copy link
Contributor Author

PeterPetrik commented Feb 16, 2021

@nyalldawson not able to find any unittests for this part of code, shall I create a new tests for this or is it ok to go without tests?

@github-actions github-actions bot added this to the 3.18.0 milestone Feb 16, 2021
@nicogodet
Copy link
Member

It seems small compared to rendered circle on ign website.
image

https://geoservices.ign.fr/documentation/services_betas/vecteur-tuile.html

And some typo in your code "cicle" instead of "circle"

@nyalldawson
Copy link
Collaborator

@PeterPetrik

not able to find any unittests for this part of code, shall I create a new tests for this or is it ok to go without tests?

They are all here: https://github.com/qgis/QGIS/blob/master/tests/src/python/test_qgsmapboxglconverter.py

Copy link
Collaborator

@nyalldawson nyalldawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, thanks!

src/core/vectortile/qgsmapboxglstyleconverter.cpp Outdated Show resolved Hide resolved
src/core/vectortile/qgsmapboxglstyleconverter.cpp Outdated Show resolved Hide resolved
src/core/vectortile/qgsmapboxglstyleconverter.cpp Outdated Show resolved Hide resolved
src/core/vectortile/qgsmapboxglstyleconverter.cpp Outdated Show resolved Hide resolved
src/core/vectortile/qgsmapboxglstyleconverter.cpp Outdated Show resolved Hide resolved
@nyalldawson
Copy link
Collaborator

It seems small compared to rendered circle on ign website.

Probably a result of setting the qgis symbol size as the circle radius, when it needs the diameter

@nyalldawson nyalldawson added the Frozen Feature freeze - Do not merge! label Feb 17, 2021
@nyalldawson nyalldawson modified the milestones: 3.18.0, 3.18.1 Feb 19, 2021
@nyalldawson nyalldawson added backport release-3_18 and removed Frozen Feature freeze - Do not merge! labels Feb 19, 2021
@nyalldawson nyalldawson reopened this Feb 19, 2021
@nyalldawson nyalldawson merged commit da5f9db into qgis:master Feb 19, 2021
github-actions bot pushed a commit that referenced this pull request Feb 19, 2021
* fix #41529: add circle vector tile layer support
github-actions bot pushed a commit that referenced this pull request Feb 19, 2021
* fix #41529: add circle vector tile layer support
nyalldawson pushed a commit that referenced this pull request Feb 20, 2021
* fix #41529: add circle vector tile layer support
@PeterPetrik PeterPetrik deleted the circle_vector_tiles branch October 19, 2021 06:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Skipping unknown layer type circle in vector tiles
3 participants