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

Entity primitive continuously re-drawn when position is SampledProperty #6631

Open
thw0rted opened this issue May 25, 2018 · 15 comments
Open

Comments

@thw0rted
Copy link
Contributor

Example in Sandcastle, based on the Interpolation demo.

Note that the debug frame counter shows continuous frames being drawn even though we're in requestRenderMode = true and the animation is paused. I found this because my app is rendering with a poor framerate in some cases, and I tracked it to a single rogue entity that has a SampledPositionProperty and an ellipsoid graphic. I dug in the debugger a little and figured out that for some reason, the associated primitive collection is being updated just about every frame, which didn't make sense to me as I wasn't modifying anything.

This doesn't happen in 3D scene mode, and it also doesn't happen with other 2D primitives (billboard, label, etc). I haven't tried with other solids but I'd be surprised if the issue was limited to ellipsoid.

@hpinkos
Copy link
Contributor

hpinkos commented May 25, 2018

Hmm, that's very weird. Thanks for reporting this @thw0rted!

@ggetz any ideas here?

@ggetz
Copy link
Contributor

ggetz commented May 25, 2018

Looks like setReady is being called by the primitive every frame in 2D mode, but not 3D mode. (This add a function that triggers after the frame is rendered, which signals another frame to be rendered in requestRenderMode).

@mramato
Copy link
Member

mramato commented May 29, 2018

I assume this is a trivial fix, but @bagnell can you confirm that there's no reason we should be calling setReady every frame?

@OmarShehata
Copy link
Contributor

This might be related, trying to get the Clipping Planes Sandcastle working with requestRenderMode doesn't really work because the entity has a PlaneGraphics with a Plane which is a CallbackProperty, which forces it to re-render every frame.

Is this expected that CallbackProperties essentially break requestRenderMode (even if they're not returning different values?) If so, I think this could be worked around by not using a CallbackProperty and manually updating the property's value. @ggetz

Original forum post.

@hpinkos
Copy link
Contributor

hpinkos commented Feb 8, 2019

@OmarShehata this is how entities are designed to work when they're dynamic. The Entity API is really good at drawing things that are static, and things that are constantly changing, but it unfortunately doesn't handle updating things that change intermittently in the best way because it will always update every frame if any of the properties are not constant.

@thw0rted
Copy link
Contributor Author

thw0rted commented Feb 8, 2019

I haven't looked at this since it was first filed. The last response before Omar's was Matt asking if the diagnosis was correct. Has the issue been fixed in the meantime? If not, can it get some attention?

@OmarShehata
Copy link
Contributor

I understand that this is by design and it works in most cases. I do feel like for this situation it would be nice if as an application developer I could do something like entity.markDirty() or someway to force to update only when needed, since in requestRenderMode you're also responsible for calling the scene.requestRender when needed.

Right now to get the Sandcastle to work I had to clone the Entity's position and re-set it to force it to update, but even then it doesn't trigger an update in the same frame.

The last response before Omar's was Matt asking if the diagnosis was correct. Has the issue been fixed in the meantime? If not, can it get some attention?

Just to bump that question, @bagnell do you know if calling setReady on the Primitive every frame is intentional? #6631 (comment)

@jpespartero
Copy link

I have seen in the @thw0rted that this behavior doesn't happen in 3D scene, but in the example provided in #7943 the requestRenderMode is not working in the 3D scene when a CallbackProperty is used to update the entity vertices

Example on v1.59

Why the Callback function is not only called when the scene.requestRender is called?

@cmcleese
Copy link

cmcleese commented Apr 3, 2020

I see this has been dormant for awhile.

Perhaps it would possible to switch to the isConstant flag dynamically based on the currentTime.
Basically if the clock has changed time you can trigger the callback event by comparison of the timestamps.
Maybe this sounds hacky.
Is there no way to prevent scene rendering when time is paused and using a callbackProperty or sampledProperty on an entity?

@jhndrx
Copy link

jhndrx commented Oct 21, 2020

My team recently encountered this issue when using request render mode with entities whose position is a sampledPositionProperty. The goal of request render mode for us is to reduce the computer resources required when the clock is paused. However, this issue seems to cause the screen to continually update.

Has any progress been made towards this?

@dhyams
Copy link

dhyams commented Apr 5, 2021

We have run into this too; is anyone looking at this issue and/or is there a workaround available?

@adi64
Copy link

adi64 commented Jan 14, 2022

We're also having this problem. I'll dump what my understanding is so far. I am not 100% sure about this, so if anything is wrong, please correct me :)

If you define a dynamic property of an Entity (e.g. CallbackProperty, but probably also SampledPositionProperty), it is marked as dynamic. A dynamic entity is updated every frame, because Cesium has no way of knowing if property values for the new frame will change. That's all fair.
If properties related to the geometry are affected, the DynamicGeometryUpdater will be invoked. In DynamicGeometryUpdater.prototype.update, the underlying Primitive is destroyed:

primitives.removeAndDestroy(this._primitive);

and then re-created.

If I've tracked the code correctly, this will cause the Primitive to re-initialize, which is probably the point where it calls setReady.
setReady then causes a new frame to be requested, which again leads to an update of the dynamic Entity, which will destroy and re-create its Primitive, which will call setReady again.

At this point, I could see a potential fix in two directions:

  • Comparing the newly returned with the old value of dynamic properties, and only updating / re-creating the Primitive if something actually changed. This could potentially be costly, because it requires a copy of the old state.
  • Find a way to update Primitives without destroying and re-creating them.

@rropp5
Copy link
Contributor

rropp5 commented Feb 23, 2024

Hey @ggetz I have a hunch that this is resolved if we change setReady() in Primitive.js to this:

function setReady(primitive, frameState, state, error) {
  primitive._error = error;
  primitive._state = state;
  primitive._ready =
    primitive._state === PrimitiveState.COMPLETE ||
    primitive._state === PrimitiveState.FAILED;
}

instead of this:

function setReady(primitive, frameState, state, error) {
  primitive._error = error;
  primitive._state = state;
  frameState.afterRender.push(function () {
    primitive._ready =
      primitive._state === PrimitiveState.COMPLETE ||
      primitive._state === PrimitiveState.FAILED;
    if (!defined(error)) {
      return true;
    }
  });
}

Essentially, the primitive follows this lifecycle on a single frame/clock tick:

  1. The clock ticks which causes DynamicGeometryUpdater to destroy and re-create the dynamic Primitives.
  2. Immediately following this, the Scene determines if it should render the next frame.
  3. Through the course of rendering the next frame, updateAndRenderPrimitives(scene) is invoked.
  4. updateAndRenderPrimitives(scene) ultimately calls update on the newly created dynamic Primitives
  5. When the Primitives are updated, setReady is invoked
  6. As the code stands today, primitive._ready is not being set until after the render completes since it's done in the afterRender function.

By setting primitive._ready immediately, there's no need to request another render which solves the infinite rendering problem reported on this issue. Do you see any reason why this solution would cause problems?

@rropp5
Copy link
Contributor

rropp5 commented Feb 23, 2024

I built a few more Sandcastles to test different cases.

In all of these sandcastles, the expectation is that the Ellipse should only render when a renderRequest (button click) is explicitly made or the amount of time specified by maximumRenderTimeChange has elapsed. Instead, the scene constantly renders in all of them.

Case 1: animation is paused, Infinity maximumRenderTimeChange

Ground Primitive : Expected behavior is that the ellipse only changes size when the "Request Render" button is clicked

Primitive : Expected behavior is that the ellipse only changes size when the "Request Render" button is clicked

Case 2: animation is active, 2 second maximumRenderTimeChange

Ground Primitive : Expected behavior is that the ellipse changes size every 2 seconds

Primitive : Expected behavior is that the ellipse changes size every 2 seconds

rropp5 added a commit to rropp5/cesium that referenced this issue Feb 23, 2024
Update Primitive _ready flag immediately instead of on the next render. This resolves the infinite rendering loop caused by dynamic Primitives which was reported under CesiumGS#6631.
@rropp5
Copy link
Contributor

rropp5 commented Feb 24, 2024

This original Sandcastle from this bug report is missing one important detail but once this line is added, my commit fixes the constant rendering issue:

// Only render when explicitly requested
viewer.scene.maximumRenderTimeChange = Infinity;

The default maximum render time change is 0.0 so when debugging this Sandcastle with my fix (#11855) I noticed it continued to render constantly. However, I figured out this was because the time difference always greater than 0: Scene.js:L4030. Once I added the line above, all worked as expected. Maybe a better default value for viewer.scene.maximumRenderTimeChange would be Infinity?

Updated original Sandcastle

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

No branches or pull requests