SSE Independent from Resolution Density & Cesium Renders at Screen Res

[ProjectBarks]

Please see: #8082

Changes

• SSE is independent from resolution density - Created a resolution constant within Scene and FrameState that is used to either upscale or downscale SSE values. I took the approach of dividing the SSE value by the constant opposed to scaling up the max sse value.
• Resolution scaled by the devices pixel ratio - Just scales the resolutionScale by the window device pixel ratio.
• Removed Force Update in Viewer - There used to be a _forceResize attribute in Viewer that was used to make the canvas resize when the window size changed. However, this was only called when the resolutionScale function was called which calls the the resolutionScale function of the cesiumWidget. Basically resolutionScale is a proxy to a class that already has a force update function and so that logic should be handled solely in the class it proxies the call.

[cesium-concierge]

Thanks for the pull request @ProjectBarks!

• ✔️ Signed CLA found.
• ❔ CHANGES.md was not updated.
  • If this change updates the public API in any way, please add a bullet point to CHANGES.md.
• ❔ Unit tests were not updated.
  • Make sure you've updated tests to reflect your changes, added tests for any new code, and ran the code coverage tool.

Reviewers, don't forget to make sure that:

• Cesium Viewer works.
• Works in 2D/CV.
• Works (or fails gracefully) in IE11.

[lilleyse]

Besides the comment about the globe, the approach here seems like the correct one - leaving resolutionScale and maximumScreenSpaceError alone but still having 3D Tiles / Globe render the same content regardless of pixel density. @mramato and @emackey can you review too?

[ProjectBarks]

https://www.diffchecker.com/qUyoflwq
It looks like (while zooming in gives different results) it ends up with the same results for the quad tree.

Here are the sandcastles I used to test:

• 1.61 - Local
• 1.60 - Build

[mramato]

I think I've convinced myself that all of these changes are good and will produce the behavior we desire.

The only gotcha here is that maximumScreenSpaceError is now computed in CSS pixels while Cesium renders in device pixels. So if you are on a 4K screen with devicePixelRatio of 2, your maximumScreenSpaceError is double what you would get on a 4K screen with devicePixelRatio of `1. I liked @lilleyse's original idea of having a separate property for controlling this behavior so this aspect wasn't completely hidden from the user, but I guess @lilleyse decided that wasn't worth it?

If I wanted to render Cesium at full resolution, with full tiles. I would essentially have to add code so that maximumScreenSpaceError for tilesets and globe get devided by devicePixelRatio, correct? That seems doable, but a little odd.

[lilleyse]

I liked @lilleyse's original idea of having a separate property for controlling this behavior so this aspect wasn't completely hidden from the user, but I guess @lilleyse decided that wasn't worth it?

If I wanted to render Cesium at full resolution, with full tiles. I would essentially have to add code so that maximumScreenSpaceError for tilesets and globe get devided by devicePixelRatio, correct? That seems doable, but a little odd.

Yeah I'm on the fence about the option. But it's really easy to add and might be useful to some people, even if most won't touch it. It could be worth adding back. And I like your summary

The only gotcha here is that maximumScreenSpaceError is now computed in CSS pixels while Cesium renders in device pixels.

which could inform both the name and description of that variable.

[ProjectBarks]

The only property I would consider adding is something similar to:

• renderFixedResolutionwhich allows someone to render a fixed resolution 1080p, 2K, 4K`

but the whole SSE to css pixel seems like a logical progression.

A CSS pixel, on the other hand, is designed to be roughly equivalent across devices. If you load the same website on side-by-side devices with a similar physical dimensions, but different pixel ratios, the website will appear to be roughly the same visual size.

[ProjectBarks]

@lilleyse @mramato is this good to merge?

[mramato]

@lilleyse I'm okay with punting on the option for this PR and adding it later if needed. Feel free to merge if you are otherwise happy with this PR. I'd like it in master at least a week before we ship.

[lilleyse]

I'm seeing some test failures locally on Linux:

Widgets/CesiumWidget/CesiumWidget resizing triggers a render in requestRender mode

Many Widgets/Viewer tests are failing from this error: RuntimeError: The browser supports WebGL, but initialization failed.

[lilleyse]

@mramato this looks good to me. Can you check out the changes to customizeJasmine and spec-main to see if that's ok?

[mramato]

@lilleyse that was my suggested placement of the fix, so I'm good with them. We really need to clean-up and revisit our jasmine config (since we have two of them right now) could be a nice sprint issue. I'm doing a little light testing now and will merge assuming no surprises.

[mramato]

Thanks again @ProjectBarks! Everything working as advertised.

[OmarShehata]

@mramato the change in this PR prevents the tests from running in IE11:

Assignment to read-only properties is not allowed in strict mode
spec-main.js (19,5)

The issue is that window.devicePixelRatio is supposed to be a read-only property (but only IE11 complains about this).

[mramato]

IE11 can't really run the WebGL tests anyway, so in practice it might not matter. However, since the spec says devicePixelRatio is read-only, feel free to revert the change and write up a separate issue to ensure our tests all pass on non-1 devicePixelRatio machines.

[DanielLeone]

Just a heads up for anyone that wanted to revert this behaviour.
viewer.resolutionScale = 1.0 / window.devicePixelRatio isn't quite enough
because window.devicePixelRatio can change; so this will actually cause problems when you zoom the browser in and out.
I'm currently using this code instead which seems to have the desired effect.

let resolutionScale = -1, lastResolutionScale = -1;
viewer.cesiumWidget.resize = new Proxy(viewer.cesiumWidget.resize, {
    apply(target, thisArg, argArray) {
        // calculate the reciprocal of devicePixelRatio so cesium calculates the scene.pixelRatio as 1.
        resolutionScale = 1 / window.devicePixelRatio;
        if (resolutionScale !== lastResolutionScale) {
            // be careful not to set resolution scale more often than necessary
            //  because it triggers a forced resize in the accessor
            lastResolutionScale = resolutionScale;
            viewer.cesiumWidget.resolutionScale = resolutionScale;
        }
        // call our original resize method as normal
        return target.apply(thisArg, argArray);
    }
});

Would we consider some alternative changes inside of Cesium to avoid having to do this? I'm happy to help.

• passing some more things into the CesiumWidget constructor?
• allow setting Scene.pixelRatio in some fashion?

[kring]

We struggled with this as well. This was our solution, which seems ok so far:
https://github.com/TerriaJS/terriajs/pull/3690/files

Using native resolution sounds good in theory, but we saw terrible performance on e.g. Macs with 4K displays and lame GPUs.

[lilleyse]

@kring @DanielLeone you can now replace any workaround code with viewer.useBrowserRecommendedResolution = true. It will ship with Cesium 1.62. #8222