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

Request render mode doesn't always work with 3D Tiles or Models #6898

Closed
mramato opened this issue Aug 8, 2018 · 27 comments
Closed

Request render mode doesn't always work with 3D Tiles or Models #6898

mramato opened this issue Aug 8, 2018 · 27 comments

Comments

@mramato
Copy link
Contributor

mramato commented Aug 8, 2018

If I fly from home view to a tileset in request render mode (and the tileset isn't cached), the flight happens but nothing ever gets rendered until I manually force one.

Does Cesium automatically trigger a render on flight complete? Shouldn't it?

CC @ggetz

@mramato
Copy link
Contributor Author

mramato commented Aug 8, 2018

I can make this happen all of the time in ion because the preview window has nothing else going on (no world imagery or terrain) so a tileset is the only thing that needs to load and Cesium is missing an internal render request somewhere.

@mramato
Copy link
Contributor Author

mramato commented Aug 8, 2018

As a workaround, I ended up adding a setTimeout in the flyTo complete callback I was using, (just a straight up requestRender didn't work, it had to be on several hundred ms delay)

@ggetz
Copy link
Contributor

ggetz commented Aug 8, 2018

@mramato It doesn't trigger one by default. It could.

I believe nothing is rendered because the tileset is not selected for rendering, therefore nothing begins loading. So there may a more direct way to make sure the tileset begins loading in request render mode than the flyTo completing. I'll take a look soon.

@mramato
Copy link
Contributor Author

mramato commented Aug 8, 2018

Do we already trigger a render every time a ajax request completes? If not, could this be something where it's fetching content but doesn't render the content once it's fetched?

Otherwise, I'll leave it to your expertise. Thanks.

@mramato
Copy link
Contributor Author

mramato commented Aug 23, 2018

We should really try to fix this for the next release. requestRender bugs on our end will make it hard for end users to use the feature and inserting random render calls in user code to work around it kind of negates the point.

@hpinkos
Copy link
Contributor

hpinkos commented Aug 27, 2018

I ran into another issue where the tileset doesn't clear from the globe on scene.primitives.remove. calling scene.requestRender after remove didn't work either, I had to call scene.forceRender.

@mramato
Copy link
Contributor Author

mramato commented Nov 7, 2018

I'm running into similar problems with 3D model loading.

@mramato mramato changed the title Request render mode doesn't always work with 3D Tiles Request render mode doesn't always work with 3D Tiles or Models Nov 8, 2018
@mramato
Copy link
Contributor Author

mramato commented Nov 8, 2018

Here's a trivial way to reproduce this with Sandcastle direct link:

@ebogo1 edit: updated Sandcastle link

var viewer = new Cesium.Viewer('cesiumContainer', {
    requestRenderMode: true
});

var model = Cesium.Model.fromGltf({
    url: '../../../../Apps/SampleData/models/CesiumMilkTruck/CesiumMilkTruck-kmc.glb',
    scene: viewer.scene,
    modelMatrix: Cesium.Transforms.eastNorthUpToFixedFrame(Cesium.Cartesian3.fromDegrees(1, 2, 10000))
});

setTimeout(function () {
    viewer.scene.primitives.add(model);
    viewer.scene.requestRender();

    model.readyPromise.then(function () {
        var r = model.boundingSphere.radius;

        var center = Cesium.Matrix4.multiplyByPoint(model.modelMatrix, model.boundingSphere.center, new Cesium.Cartesian3());
        var heading = Cesium.Math.toRadians(-130.0);
        var pitch = Cesium.Math.toRadians(-25.0);
        var range = r * 3.5;
        viewer.scene.camera.lookAt(center, new Cesium.HeadingPitchRange(heading, pitch, range));
    });
}, 1000);

It doesn't happen every time (because race condition), but most of the time the model.readyPromise will never resolve until the user interfaces with the window or resizes the screen (even though we call requestRender after adding the primitive)

@ggetz does this help narrow down the issue?

@mramato
Copy link
Contributor Author

mramato commented Jan 18, 2019

Also reported in #7489

@KermMartian
Copy link

Apologies for bumping this (via my #7489). I wanted to check if there was a target release for resolving this, or if there was anything I could do to help with its resolution. Thanks!

@hpinkos
Copy link
Contributor

hpinkos commented Feb 12, 2019

@KermMartian we don't have a specific release target, but we're hoping to look into this soon. There are a few other things in-progress we need to finish first though.

If you're interested in looking into a fix, we would be happy to review a pull request! @ggetz should be able to give some suggestions for where to look for the problem

@mramato
Copy link
Contributor Author

mramato commented Aug 16, 2019

@likangning93 @lilleyse @ggetz this is the issue I was mentioning yesterday.

@KermMartian
Copy link

I'm enthused if this is going to be addressed soon. :) I'm not pleased with our current solution, which is to not use the request render mode.

@mramato
Copy link
Contributor Author

mramato commented Sep 3, 2019

A lot has changed since this issue was written. Can someone verify that there are still actual problems here?

@KermMartian one workaround I've used successfully is to just request a render at least once a second with the below code:

function requestRenderBug() {
    if (!viewer.isDestroyed()) {
        viewer.scene.requestRender();
        setTimeout(requestRenderBug, 1000);
    }
}
setTimeout(requestRenderBug, 1000);

Gets you most of the benefit while avoiding the bug.

@mramato
Copy link
Contributor Author

mramato commented Feb 10, 2021

I just ran into this using the latest Cesium, so definitely still a problem.

@ebogo1 ebogo1 moved this to Next priority in CesiumJS Issue/PR backlog Oct 26, 2021
@ebogo1 ebogo1 moved this from Next priority to Notable backlog items in CesiumJS Issue/PR backlog Oct 26, 2021
@lilleyse
Copy link
Contributor

lilleyse commented Dec 10, 2021

The problem is that Model requires at least one update before it becomes ready (and possibly more if asynchronous is true since the JobScheduler may take a few frames before it becomes ready) but update is never called until a user action occurs that triggers requestRender.

One approach is for scene to keep a list of primitives that still need updating and request render until the list is empty. Another is for scene to listen to model load events like it does for RequestScheduler, TaskProcessor, and Globe. Neither approach is super well defined at the moment.

@ebogo1
Copy link
Contributor

ebogo1 commented Dec 10, 2021

A lot has changed since this issue was written. Can someone verify that there are still actual problems here?

I can still reproduce the bug with model.readyPromise not resolving until a render is forced by something like resizing the browser window - updated Sandcastle. This is happening in sandcastle.cesium.com on CesiumJS 1.88.

But, I cannot reproduce this issue locally, even on older versions of CesiumJS, both built and unbuilt. Even if I host a .glb to load on a local server as opposed to linking to it with a relative filepath, I still cannot reproduce locally. Has this ever been an issue in local development?

@ggetz ggetz removed their assignment Jan 27, 2022
@ephjos
Copy link

ephjos commented Feb 17, 2022

I just ran into this issue (or something very similar) while developing locally. In my case a tileset I was adding to the scene was not showing up until I interacted with the viewer. I was able use what @mramato did above and resolve the issue. I noticed that the issue was only happening on Chrome (Brave and Chromium), but Firefox updates immediately without issue. I am able to reproduce this behavior with the sandcastle.

  1. Open @ebogo1's updated Sandcastle
  2. Allow the page to load
  3. Click Run in the top-left
  4. The viewer should reload

On Chrome the zoomed out earth view should be shown:
sel-220217-1432-32
If the user changes the size of the window or otherwise interacts with the viewer, the milk truck is shown.

On Firefox, the milk truck is shown immediately.

Versions

  • Brave Browser 97.1.34.81
  • Chromium 97.0.4692.99 Arch Linux
  • Mozilla Firefox 96.0.2
  • Linux 5.16.2-arch1-1 x86_64 GNU/Linux

@ggetz
Copy link
Contributor

ggetz commented Jun 9, 2022

After recent updates, I can no longer reproduce the issue as described in @ephjos or @ebogo1's sandcastle example.

@lilleyse Is there still an issue here besides what those examples demonstrate? Or can this be marked as fixed?

@ephjos
Copy link

ephjos commented Jun 9, 2022

@ggetz I am still able to reproduce this issue using the above Sandcastle with version 1.94. It actually appears to be happening on Firefox now as well.

I used the steps in #6898 (comment). The issue does not always happen on the first Run/Refresh, but I can reliably get it to happen on all three browsers.

Versions

  • Brave Browser 102.1.39.111
  • Chromium 102.0.5005.61 Arch Linux
  • Mozilla Firefox 101.0
  • Linux 5.18.1-arch1-1 x86_64 GNU/Linux

@lilleyse
Copy link
Contributor

lilleyse commented Jun 9, 2022

#6898 (comment) describes why it's happening. We haven't done anything to fix it yet.

@mramato
Copy link
Contributor Author

mramato commented Dec 27, 2022

@ptrgags @lilleyse Did the glTF/3D Tiles next refactor happen to address this issue?

@lilleyse
Copy link
Contributor

No, it was not addressed as part of the refactor.

I don't think it's too hard to implement though:

  • Call scene.requestRender() when a tile is moved to the processing queue to ensure at least one update is called
  • Call scene.requestRender() in processTiles if length is greater than 0

Additionally so that individual models work:

One problem is that scene is not really accessible to either of these. So some sort of requestRender function might need to be added to FrameState.

@Aviian314
Copy link

Aviian314 commented Feb 21, 2023

Additionally so that individual models work:

One problem is that scene is not really accessible to either of these. So some sort of requestRender function might need to be added to FrameState.

Request render mode has been causing me issues with models not loading until the user performs an action that forces a render. Like mentioned earlier in this thread, it can take multiple render calls before a model is ready and shows. In the processLoader method, I added the lambda frameState.afterRender.push(() => true); and it appears to make models load correctly. Of course, its rendering at its full rate until the model is ready but after they show, the rendering stops as desired.

Edit: I'm working with CesiumJS version 1.101

@ggetz
Copy link
Contributor

ggetz commented Apr 12, 2023

After #11059 and adding requestRender to the loaders, I can no longer replicate. I did update the sandcastle example to use the newer APIs.

Closing this as fixed.

@ggetz ggetz closed this as completed Apr 12, 2023
@ggetz ggetz moved this from Notable backlog items to Issue/PR closed in CesiumJS Issue/PR backlog Apr 12, 2023
@mramato
Copy link
Contributor Author

mramato commented Apr 12, 2023

@ggetz I'm confused by the new example. The doc for request render mode includes the following:

If true, rendering a frame will only occur when needed as determined by changes within the scene.

I could be wrong, but historically adding a primitive to the scene was considered an internally detected "Change within the scene", but in your example it seems you have to call requestRender after adding the primitive. Did something change? At the very least, the documentation probably need to be a little clearer hear around when a render needs to be specifically requested.

@ggetz
Copy link
Contributor

ggetz commented Apr 25, 2023

I could be wrong, but historically adding a primitive to the scene was considered an internally detected

I don't think adding the primitive itself ever triggered a requestRender explicitly. That does raise the question of whether it should, but that is documented in #6734. The example here replicated the problematic cases above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Issue/PR closed
Development

No branches or pull requests

9 participants