theia-ide / sprotty

A next-gen web-based graphics framework
Apache License 2.0
138 stars 23 forks source link

Fix protocol for model updates #48

Closed JanKoehnlein closed 7 years ago

JanKoehnlein commented 7 years ago

This is how an update currently works

Apart from the in my opinion unnecessary UpdateModel / RequestModel handshake whcih makes the entire thing pretty slow (it is an update only!), we forget existing layout information resulting in a notable flickering.

The server should not let go the bounds information but attach the one from the previous layout to the new model. This will avoid the flickering of the unchanged parts of the diagram. We could nevertheless mark all bounds as invalid (other than setting it to empty) such that the BoundsUpdater can recalculate them and issue a SetBounds if they have changed (e.g. when a node has to be resized because its content has changed).

In addition, we should also introducing a separate command / action pair for diagram updates, e.g. Reconcile, replacing SetModel and giving us the chance to calculate a nice transition. If we stick to RequestModel, it should contain a flag indicating whether the result should be a SetModel or a Reconcile.

JanKoehnlein commented 7 years ago

Pushed a fix to master, that definitely needs more polishing. Main idea is to copy the layout information from the last model to the new one on the server, and using a flag revalidateBounds to signal to the client to update the bounds using the DOM information and only cause a new SetBoundsAction if the bounds differ.

JanKoehnlein commented 7 years ago

This is the current event flow

  1. server: send SetModelAction(pos:0, dim: 0)
  2. client: viewer renders (pos:0, dim: 0)
  3. client: bounds updater fires SetBounds(0, dim)
  4. client: send SetBoundsAction(0, dim)
  5. client: viewer renders (0, dim)
  6. client: bounds updater detects equal bounds and stops this fork
  7. server: sends (pos, dim)
  8. client: viewer renders (pos, dim)

5 and 6 are unnecessary and cause some strange, invalid intermediate state on the client

It seems we need more state in the protocol or a different one, e.g.

  1. server: send RequestBoundsAction(pos:0, dim: 0)
  2. client: viewer (pre-)renders (pos:0, dim: 0)
  3. client: bounds updater fires SetBounds(0, dim)
  4. client: send SetBounds(0, dim)
  5. --- obsolete --- client does not handle SetBounds
  6. --- obsolete ---
  7. server: sends SetModelAction(pos, dim)
  8. client: viewer renders (pos, dim)

Open questions:

spoenemann commented 7 years ago

The event flow is now optimized for different scenarios. The model source is responsible to trigger the events in correct order. We'll document this in the Wiki soon.