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

Simplify and move Stencil reference functions #241

Merged
merged 3 commits into from Jan 16, 2023

Conversation

Tristramg
Copy link
Collaborator

This pull request is more a question of style concerning stencil_reference_value_2d and stencil_reference_value_3d

The first commits changes a bit how the pattern matching is written. I believe it’s a bit more “rusty” and we have the guarantee the matching is complete.

The second commit moves those functions as a member of WorldTileCoord instead of TileViewPattern. I’m still missing the bigger picture of all those parts. The observation was that no information of TileViewPattern is required to do the computation, but I’ll perfectly understand that there are broader implications.

@Tristramg Tristramg changed the title Stencil reference Simplify and move Stencil reference functions Jan 15, 2023
@maxammann
Copy link
Collaborator

Historically they moved a little bit around in code. So thanks for moving them to the right location.

I think the 2d is not used anymore. I think this would be a good PR to just remove it and document via a commit why it was removed. Here is why:

The 2d version of calculating the stencil value is only valid for cases where only tiles from exactly one zoom level are displayed at once. As soon as multiple zoom levels are presented at once, the stencil value needs to be distinct across zoom levels.

Can you remove it and commit with that message?

maxammann
maxammann previously approved these changes Jan 15, 2023
The 2d version of calculating the stencil value is only valid for cases where only tiles from exactly one zoom level are displayed at once.
As soon as multiple zoom levels are presented at once, the stencil value needs to be distinct across zoom levels
@Tristramg
Copy link
Collaborator Author

Function removed! Thank you for the context

@maxammann maxammann merged commit 021376a into maplibre:main Jan 16, 2023
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.

None yet

2 participants