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

DV: Update tile selection to defer replace refinement until global selection #11

Merged
merged 2 commits into from
Aug 15, 2024

Conversation

Kaapp
Copy link

@Kaapp Kaapp commented Aug 13, 2024

  • There was a bug where we were assigning the same _replacedTileId to all tiles in a tileset with a REFINE replacement type.
  • This meant that in our platform when we would try to select the highest priority tile or group from each tileset as a minimum, we would get the root tile for ADD replacements and the whole traversal for REFINE replacements.
  • Instead, this PR ensures that we return the entire hierarchy for both types, and updates the GroupedTilesArray to allow merging of two GroupedTilesArray objects by performing the replacement at that level.
  • This means that our platform can query for the root tile or group for each tileset and correctly get just that data as a minimum to render. Leaving the remaining data in a second GroupedTilesArray, we can then use the updated API to merge in the remaining tiles up to our rendering budget.

N.B. There are some lerna changes that seemed required now that we are running on yarn 4 w/ corepack internally.

@Kaapp Kaapp requested review from quipo, Gmadges and Rennzie August 13, 2024 11:16
@Kaapp Kaapp self-assigned this Aug 13, 2024
@Kaapp
Copy link
Author

Kaapp commented Aug 13, 2024

I won't merge this, or the corresponding platform PR, until we've gone through guinea pigs testing, but not expecting anything major so prepping these PRs ahead of time.

@Kaapp Kaapp merged commit f7e79b4 into master Aug 15, 2024
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.

4 participants