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

Do not apply #12467 change to Icons #13955

Merged
merged 2 commits into from
Aug 12, 2022
Merged

Conversation

mike-000
Copy link
Contributor

@mike-000 mike-000 commented Aug 8, 2022

Fixes #13954

Also fixed the rendering test which might have picked this up, but was not using the correct pixel ratio.

Compare the fixed version https://deploy-preview-13955--ol-site.netlify.app/examples/icon-scale.html
with https://openlayers.org/en/v6.15.1/examples/icon-scale.html
on a mobile device or with browser zoom

@github-actions
Copy link

github-actions bot commented Aug 8, 2022

📦 Preview the examples and docs from this branch here: https://deploy-preview-13955--ol-site.netlify.app/.

pixelRatio * imageScale[0],
pixelRatio * imageScale[1],
];
pixelRatio = 1;
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if this is the correct fix. It would be batter if icons would be created with the pixel ratio of the destination context, then they would also appear crisper on high dpi devices.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, images with color applied are available at higher dpi. The ImageBuilder uses the image style's getPixelRatio() method to determine what is available, so that can also be used here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to use high dpi images where available.

Copy link
Member

@ahocevar ahocevar left a comment

Choose a reason for hiding this comment

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

Much better now. Thanks, @mike-000

@ahocevar ahocevar merged commit 3e707d4 into openlayers:main Aug 12, 2022
@mike-000 mike-000 deleted the immediate-icons branch August 12, 2022 12:03
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.

ol.style.Icon doubles when using vector Context.drawFeature
2 participants