Fix rendering if 3D viewport is hidden initially

[philippotto]

URL of deployed dev instance (used for testing):

• https://___.webknossos.xyz

Steps to test:

• open a dataset or annotation
• re-arrange the viewports so that the 3d viewport is not visible
• wait a few seconds so the layout is persisted (this is not done via the save button)
• reload the page (previous layout should be visible)
• ensure that all viewports look good (despite the 3d viewport being hidden)
• use "reset layout" and test that all mouse bindings work as expected

Issues:

• fixes Rendering is broken if 3D viewport is hidden initially #5430

---

• Updated (unreleased) changelog
• Updated (unreleased) migration guide if applicable
• Updated documentation if applicable
• Adapted wk-connect if datastore API changes
• Needs datastore update after deployment
• Ready for review

[philippotto]

Waiting for the 3d viewport did not succeed here, if it's hidden initially. In that case, the mouse handlers were not set up. Since, there is a waitForElementWithId two lines below for each viewport, I'd hope that this change doesn't break anything.

[MichaelBuessemeyer]

I think this won't be a problem, as the waitForElementWithId works with promises. The onliest thing I see is that if one of the viewports is never rendered, that there will be endless calls of window.requestAnimationFrame as long as wk is opened in a browser tab.
I think this is not optimal. Maybe we can add a timeout to this function and retrigger adding the mouse controllers for all inputcatchers that were newly rendered when the layout is changed.
But this would be a thing for another PR.

Note:

export function waitForElementWithId(elementId: string): Promise<*> {
  const tryToResolve = resolve => {
    const el = document.getElementById(elementId);
    if (el) {
      resolve(el);
    } else {
      window.requestAnimationFrame(() => tryToResolve(resolve));
    }
  };
  return new Promise(tryToResolve);
}

[philippotto]

Good point! I cross-referenced this comment in the corresponding issue: #3475

I agree that this is out-of-scope for this PR.

[MichaelBuessemeyer]

The current workaround does not work for me when I stack all ortho views together and have the XY-viewport open.

Lets investigate why 🔍

[MichaelBuessemeyer]

I think this won't be a problem, as the waitForElementWithId works with promises. The onliest thing I see is that if one of the viewports is never rendered, that there will be endless calls of window.requestAnimationFrame as long as wk is opened in a browser tab.
I think this is not optimal. Maybe we can add a timeout to this function and retrigger adding the mouse controllers for all inputcatchers that were newly rendered when the layout is changed.
But this would be a thing for another PR.

Note:

export function waitForElementWithId(elementId: string): Promise<*> {
  const tryToResolve = resolve => {
    const el = document.getElementById(elementId);
    if (el) {
      resolve(el);
    } else {
      window.requestAnimationFrame(() => tryToResolve(resolve));
    }
  };
  return new Promise(tryToResolve);
}

[MichaelBuessemeyer]

My 🔍 results:
The root cause is that the cameras for the planes are not updated until the 3d viewport is rendered.
This bug only appeared now, because afaik gl always rendered each viewport and not only the viewports that were already visible like FlexLayout does it.

Then why are the viewport cameras updated once the 3d viewport is rendered?
This is because only the TdController uses a CameraController. A CameraController automatically updates the cameras of all viewports once they are changed in the store. See here. Therefore upon rendering the 3D viewport, a TdController is rendered and thus a CameraController that updates the viewports cameras 🎉.
Note: The TdController is only rendered when the 3d viewport exists due to this guard.

How to solve this (imo):
I think this is a software architectural problem, that all the viewports rely on the CameraController that is rendered by the 3d viewport for updating their cameras. Imo each viewport should have its own "CameraController" that is responsible for updating its own camera and not the camera of the other viewports.

It looks like implementing this change is quite some work, but think it is necessary and that a quick fix would not be sufficient / too dirty and might result in related bugs later on.
@philippotto Do you have enough time for the change I suggested?

Note: The changes in input_catcher and default_state are unnecessary.

[MichaelBuessemeyer]

You can verify my thesis by commenting out the loop starting at this line and

• then opening a tracing view with only the XY viewport active,
• then opening the 3d viewport
• and then opening the XY viewport again should no longer fix the issue.

If you uncomment the lines again, then the 3rd step should work and data should be visible in the xy viewport

[philippotto]

By postponing this null-check to the exact points where it's necessary (i.e., in setTargetAndFixPosition), we can eagerly instantiate the CameraController which should fix the issue.

[philippotto]

My results:
The root cause is that the cameras for the planes are not updated until the 3d viewport is rendered.
This bug only appeared now, because afaik gl always rendered each viewport and not only the viewports that were already visible like FlexLayout does it.

Then why are the viewport cameras updated once the 3d viewport is rendered?
This is because only the TdController uses a CameraController. A CameraController automatically updates the cameras of all viewports once they are changed in the store. See here. Therefore upon rendering the 3D viewport, a TdController is rendered and thus a CameraController that updates the viewports cameras .
Note: The TdController is only rendered when the 3d viewport exists due to this guard.

Thanks for the detailed investigation and write-up 👍 👍 From your last sentence, I could derive a relatively simple solution. I simply removed the guard and limited its scope so that it doesn't affect the CameraController.

How to solve this (imo):
I think this is a software architectural problem, that all the viewports rely on the CameraController that is rendered by the 3d viewport for updating their cameras. Imo each viewport should have its own "CameraController" that is responsible for updating its own camera and not the camera of the other viewports.

It looks like implementing this change is quite some work, but think it is necessary and that a quick fix would not be sufficient / too dirty and might result in related bugs later on.
@philippotto Do you have enough time for the change I suggested?

Yes, I agree, that an architectural change would be even better. However, as a quick fix, I hope that my above workaround is adequate.

Note: The changes in input_catcher and default_state are unnecessary.

See my other comments. The changes in input_catcher seem to be necessary (and I also think that they make the viewport-rect state more consistent).

[MichaelBuessemeyer]

I was finally able to test the fix and it works 🚀 🧻

[MichaelBuessemeyer]

Yes, I agree, that an architectural change would be even better. However, as a quick fix, I hope that my above workaround is adequate.

I just created an issue for this: #5436