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

Carli/1988 giant file jigsaw error #2219

Merged
merged 14 commits into from
Aug 22, 2023

Conversation

crocka
Copy link
Contributor

@crocka crocka commented Jul 31, 2023

Description

This pr fixes #1988. Associated backend PR is #1288. The pendingSynchronisedTiles map in the TileService failed to clean up itself with each render, causing messy tiles to be rendered during panning and zooming. Also, syncId and tileCount are added to the RasterSyncMessage because there was not any robust method to determine whether the required tiles in a sync group have been transmitted, causing missing tiles during animation. Lastly, the pr fixed the blinking of the vector overlay during animation due to racing condition that caused image rendering before all the vector overlay tiles have been transmitted.

Checklist

For linked issues (if there are):

  • assignee and label added
  • ZenHub issue connection, board status, and estimate updated

For the pull request:

  • reviewers and assignee added
  • ZenHub estimate, milestone, and release (if needed) added
  • e2e test passing / corresponding fix added
  • changelog updated / no changelog update needed
  • protobuf updated to the latest dev commit / no protobuf update needed
  • BackendService unchanged / BackendService changed and corresponding ICD test fix added

@crocka crocka added this to the v4.0-stable milestone Jul 31, 2023
@kswang1029 kswang1029 added the requiring backend For issues or pull requests that require work in both frontend and backend label Aug 2, 2023
Copy link
Collaborator

@kswang1029 kswang1029 left a comment

Choose a reason for hiding this comment

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

the solution with sync_id works nicely as far as I can tell. No missing tiles after animation stopping, or during animation playback. Vector overlay is rendered properly during/after animating as well. 👍 Thanks @crocka and @pford.

Copy link
Collaborator

@YuHsuan-Hwang YuHsuan-Hwang left a comment

Choose a reason for hiding this comment

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

Nice work. 👍

src/services/TileService.ts Outdated Show resolved Hide resolved
@YuHsuan-Hwang
Copy link
Collaborator

Ready for merge. @crocka Please update the change log, thanks.
I wonder if this PR also fixes missing tile related issues #2026, #954, or #794?

@crocka
Copy link
Contributor Author

crocka commented Aug 21, 2023

I believe that it also resolved issue #2026, but further testing is required to confirm that. @YuHsuan-Hwang

@YuHsuan-Hwang
Copy link
Collaborator

Let's first close 1988 for now then.

@YuHsuan-Hwang YuHsuan-Hwang merged commit 00a8e52 into dev Aug 22, 2023
12 checks passed
@YuHsuan-Hwang YuHsuan-Hwang deleted the carli/1988_giant_file_jigsaw_error branch August 22, 2023 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
requiring backend For issues or pull requests that require work in both frontend and backend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Giant files cause jigsaw error
4 participants