Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

split renderSync and transition nudging to allow client view syncing #1813

Closed
wants to merge 1 commit into from

Conversation

incanus
Copy link
Contributor

@incanus incanus commented Jul 1, 2015

Picking up from stuff learned in #1125 (comment), this breaks the current renderSync into two parts — the actual render call and a nudgeTransitions which moves a step in the Transform, possibly calling Map::update if needed.

The reason for this is needing to sync client-side view (i.e. UIView objects) positions with the GL map, and that step needing to come in between these two to remain in sync.

Todo:

  • Review terminology used (@jfirebaugh @kkaefer?)
  • Review for other possible side effects
  • Update other platforms besides iOS if approach is 👌

@incanus incanus added refactor ⚠️ DO NOT MERGE Work in progress, proof of concept, or on hold labels Jul 1, 2015
@incanus incanus added this to the iOS Beta 3 milestone Jul 1, 2015
@incanus
Copy link
Contributor Author

incanus commented Jul 1, 2015

Per chat, @kkaefer says we should probably snapshot time and use that to be consistent through the render of a frame and update of transitions, instead of having varying notions of time throughout. This PR is more of a workaround of that.

@jfirebaugh
Copy link
Contributor

@incanus Instead of splitting this up, can you implement this by updating to latest master and doing the syncing when the view receives notifyMapChange with MapChangeDidFinishRenderingFrame or MapChangeDidFinishRenderingFrameFullyRendered? This notification is sent at the relevant point in time for UIView syncing.

@incanus
Copy link
Contributor Author

incanus commented Jul 1, 2015

Duh, yeah, great idea. We'll just need some tests to make sure that the transitions stay after the closing notification, though.

@jfirebaugh jfirebaugh closed this Jul 2, 2015
@jfirebaugh jfirebaugh deleted the split-render-transitions branch July 2, 2015 00:01
@incanus
Copy link
Contributor Author

incanus commented Jul 6, 2015

Just talked this out with @kkaefer@jfirebaugh, this won't work, because we do everything synchronously. So the order of operations is:

  1. Post "will render" notification.
  2. Render frame.
  3. Post "did render" notification.
  4. Don't call out to update view syncing, because we're synchronous here.
  5. Update the transform
  6. ...
  7. At some point, update view syncing, which is out of sync because the transform has been modified.

@kkaefer is going to look today & tomorrow into #1813 (comment) to solve this.

@jfirebaugh
Copy link
Contributor

By "synchronous" you mean the notification is dispatched from the rendering thread, while the view syncing needs to happen on the main thread?

@incanus
Copy link
Contributor Author

incanus commented Jul 6, 2015

Yeah, sorry, the post-render-post-update cycle all happens inline in the map thread, and the view update needs to happen on the main thread in between steps 3 and 4.

@incanus
Copy link
Contributor Author

incanus commented Jul 8, 2015

It might make sense to go with this spilt for b3 for an easy win and circle back on the refactor work.

@incanus
Copy link
Contributor Author

incanus commented Jul 8, 2015

Going to apply this to the release-v0.5.0 branch for b3; we can revisit globally after hitting #1813 (comment).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants