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

IconLayer: fix copy texture data when resize #4151

Merged
merged 3 commits into from
Jan 15, 2020
Merged

Conversation

xintongxia
Copy link
Contributor

@xintongxia xintongxia commented Jan 14, 2020

Background

for #4146 and related to #4039

Change List

  • copy texture data without flipping Y when resize
  • increase icon size in render test
  • fix icon edge artifact

@coveralls
Copy link

coveralls commented Jan 15, 2020

Coverage Status

Coverage decreased (-2.5%) to 80.868% when pulling 3a14621 on xx/icon-debug into 0ead9dc on master.

getIcon: d => {
if (d.PLACEMENT === 'SW') {
return {
url: './test/render/golden-images/pointcloud-meter.png',
url: './test/render/golden-images/examples/website_screen-grid.png',
Copy link
Collaborator

@Pessimistress Pessimistress Jan 15, 2020

Choose a reason for hiding this comment

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

Can we not use a golden image as icon? They may change in the future for reasons unrelated to this layer.

You can just reuse the marker pngs with a different id and size.

width: 1024,
height: 1024,
anchorY: 1024,
mask: false
};
},
onAfterRender: ({layers, done}) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this why the test was failing? 😒

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

@xintongxia xintongxia merged commit b2a5ec9 into master Jan 15, 2020
@Pessimistress Pessimistress deleted the xx/icon-debug branch February 13, 2020 21:13
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.

3 participants