Revise the fix when raster-fade-duration is 0 #2501
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Ping me when you want this reviewed as currently it is in draft. Looks good otherwise. |
zhangyiatmicrosoft
changed the title
HarelM
approved these changes
May 17, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Launch Checklist
Initialized fadeEndTime with 0 so all everything else afterwards is easier to read and interpret and remove test case for undefined.
Applied most changes discussed in issue PR 2455 and fadeEndTime #2480 and tested manually with samples there.
Updated render test case "raster-masking\overlapping-zoom":
-- This test case is quite complicated in that it loads at 14, zoom to 15 and then zoom to 13, whereas only 1 tile is available for each zoom. The result is not easy to understand so I added debug line to help. expected.png content is otherwise the same as before and expected_ubuntu22 had old baseline and faulty, also updated.
Result is counter-intuitive because at that spot zoom13 has no tile, and zoom14 is displayed. This is verified by directly rendering zoom 13 to begin with without any fade duration and zoom operations.
Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
Briefly describe the changes in this PR.
Link to related issues.
Add an entry to
CHANGELOG.mdunder the## mainsection.