Request render mode is ignored when a Billboard is added to the system

rropp5 commented 6 months ago

What happened?

Adding a Billboard to the system triggers a render when the Scene is configured to use requestRenderMode. This was not the case prior to version 1.110 and is a regression that was introduced by the fix for #11529 (committed under #11535). This makes it impossible to leverage the performance benefit of requestRenderMode when Billboards are added frequently.

The fix to this issue should also resolve the original bug (label backgrounds failing to render in requestRenderMode).

Problematic code in Billboard.js:

Billboard.prototype._loadImage = function () {
   ...

   const scene = that._billboardCollection._scene;
   if (!defined(scene)) {
      return;
   }
   // Request a new render in request render mode
   scene.frameState.afterRender.push(() => true);

   ...
}

My initial thought is the solution should be implemented within the LabelCollection instead of Billboard because this is a problem specific to a Label's background.

Reproduction steps

Enable requestRenderMode on the Scene
Set maximumRenderTimeChange to Infinity on the Scene
Periodically add Billboard entities to the Viewer.
Observe a render occurs each time a Billboard is added even when requestRender is not invoked.

Sandcastle: Add a billboard every 100ms in request render mode. Observe renders happen each time a billboard is added.

Sandcastle example

https://sandcastle.cesium.com/#c=fVNNj9MwEP0rQ04pTZ2k3Zb0U6CeVgItYhGnXFxnmlo4drGdfizqf8dxGlS6K3yJ5/nN85sZhylpLBw4HlHDEiQeYY2G1xX54bEwD5iP10payiXqPOjNc5nLOIZntKDxV42m+crCSVSqwFy2esQwlEiujG+e8MWdu3usrnHuNZ6kOHfJxx1KwNNecMath30mFneKFT3xqq5axe+8wvWOyrLRfZRbLrk9e4fM12Ys1farMv9Wt3ag21E5CgfZMBklGfmQJlk6eciyNILBeDgZTocZGQ2nWTIep9MIRg/TyWQ6JeMkdfRxkjV9EK4FTNXSOvnExQbto7SoD1SEYQ+WK/idS4DWCqu1/p+TRhBe44QJJTHsCok6nZZ+DcjJyXYUF/QhTdyC9629ft+TOy9KIBGqdNP9VBRclkC9pQ0XYqOoLmAwcDPiZYnauLPb+XDX0p2qRQFS2Xfta4DrCyIoLbccDaFFEfrKAfbKOEzJWWc1avG/l83gyrxdvKIlziAPCIn93sRbyrhwwyUl3+bBfcqlAS6Nm0vU1A69eRAFC2PPAlcd+SOv9kpbqLUInbDFai+oa3O8qdlPtISZa1cBFvFt6qLgB+DF8o3fAZigxriTbS3EM3/BPFgtYsd/lSoUbZr9dEAt6Lmh7dLV5xYkhCxiF76daZUSG6rvlP8A

Environment

Browser: Chrome, Edge, Firefox (likely all browsers exhibit this problem) CesiumJS Version: 1.114 Operating System: Windows 10

ggetz commented 6 months ago

Observe a render occurs each time a Billboard is added even when requestRender is not invoked.

While I'll acknowledge this is a change in behavior, I'm not sure I'd classify this is a bug. If a new Billboard is added, it seems to me like the expected behavior would be to render it. What are your thoughts?

rropp5 commented 6 months ago

@ggetz I believe it's very important in many use cases to be able to batch Billboard creation without forcing a render each time. Also, I feel that this is a significant deviation from the explanation of requestRenderMode on your blog post.

Starting with Cesium 1.42, a new opt-in explicit rendering mode allows the developer to precisely control the rendering in Cesium based on the needs of the application. When enabled, a new frame will be rendered only in the following scenarios:

The camera changes, either with user input or with the Cesium API.

The simulation time advances beyond a specified threshold.

Loading in terrain, imagery, 3D Tiles, or data sources, including each individual tile load. At a lower level, this is triggered when a web request is resolved via a URI or blob, or an asynchronous process returns from a web worker.

Globe imagery layers are added, removed, or changed.

Globe terrain providers change.

The scene mode morphs.

A frame is explicitly rendered with the Cesium API.

This covers a lot of the bread and butter functionality that Cesium is commonly used for, such as loading in terrain, imagery, and data like 3D Tiles. For more fine-tuned changes to the scene, instead of watching for every update that might require a render, this is left up to the application. If the app makes changes to the scene or its contents through the Cesium API in a way that is not covered by one of the above cases, for example, using the Entity or Primitive API, then explicitly request a new render frame.

Additionally, this is inconsistent with behavior of other graphics objects in the Entity API because other graphics objects on the Entity do not immediately force a render. Consider this slightly modified Sandcastle where a Point is created instead of a Billboard.

With this change in behavior it's no longer possible to achieve good performance in a heavyweight system that frequently creates Billboards. So, I do believe this should be considered a bug and the behavior should be reverted.

It seems to me there should be a less invasive way to fix #11529 so everyone is happy. I'm happy to discuss alternative solutions for #11529 but must admit the LabelCollection code is not something I've spent much time looking at.

CesiumGS / cesium