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

GroundPolylinePrimitive and Ground Primitive materials render poorly on newer mobile devices #6735

Closed
chris-cooper opened this issue Jun 27, 2018 · 27 comments · Fixed by #9064

Comments

@chris-cooper
Copy link
Contributor

image

These lines dance as the camera moves. This is on an MP2F2LL/A.

@likangning93
Copy link
Contributor

There seems to be a problem with materials on ground polygons too... maybe this is depth buffer related?

@likangning93
Copy link
Contributor

At least part of this is weird depth buffer stuff:

2018-06-27

Sandcastle to reproduce.
Need to zoom and pan a bit for this weirdness to happen, then it usually goes away when the view becomes static. It's weird that ground polygons with materials won't draw at all, though, even when the camera goes static.
Maybe something odd about vertex attributes...

@likangning93
Copy link
Contributor

Part 2 of the problem (aka the part that's probably my fault):
materials on ground polygons work roughly correctly (jitter problem when moving from above) when using approximate inverse trig, but not when using planar methods!

Sandcastle.
Tap on the screen to place a ground polyline with a texture and zoom to it. This works roughly correctly in 3D but not in 2D/CV, or if the offset parameter is lowered to below one degree.
Interestingly, the setting the offset really low "almost" works for the first rectangle, placed at lat/long (0,0), but then doesn't work at all after that.

I'll keep updating with notes.

@likangning93
Copy link
Contributor

Actually, to reduce noise I'm going to move tracking for this into a separate issue. I'm pretty confident it's a similar problem under the hood.

@likangning93
Copy link
Contributor

likangning93 commented Jun 27, 2018

Another problem:

Sandcastle on a deployed branch

var viewer = new Cesium.Viewer('cesiumContainer', {
    terrainProvider : Cesium.createWorldTerrain()
});
var scene = viewer.scene;

var loopPositions = Cesium.Cartesian3.fromDegreesArray([
    -111.94500779274114, 36.27638678884143,
    -111.90983004392696, 36.07985366173454
]);

var instance = new Cesium.GeometryInstance({
    geometry : new Cesium.GroundPolylineGeometry({
        positions : loopPositions,
        width : 16.0
    }),
    attributes : {
        show : new Cesium.ShowGeometryInstanceAttribute(),
        color : Cesium.ColorGeometryInstanceAttribute.fromColor(Cesium.Color.fromCssColorString('#67ADDF').withAlpha(0.7))
    }
});

var polylineOnTerrainPrimitive = new Cesium.GroundPolylinePrimitive({
    geometryInstances : instance,
    debugShowShadowVolume : true
});
polylineOnTerrainPrimitive.appearance = new Cesium.PolylineColorAppearance();

scene.groundPrimitives.add(polylineOnTerrainPrimitive);

viewer.camera.lookAt(loopPositions[1], new Cesium.Cartesian3(5000.0, 5000.0, 5000.0));
viewer.camera.lookAtTransform(Cesium.Matrix4.IDENTITY);

On the iPad, going to a view like this seems to sometimes ignore that depth testing is supposed to be disabled:
6cdb2068-bb55-498c-98fe-71ce1c66ed6e

The depth test also seems to be against the "weird" depth buffer observed above.

@likangning93
Copy link
Contributor

@bagnell and @mramato pointed out that logarithmic depth isn't supported on iPad/Safari, so the "looks like this is depth testing" problem is actually the far "cap" of the volume being beyond the far plane. This particular problem is reproducible on desktop by disabling log depth.

@likangning93
Copy link
Contributor

This particular problem is reproducible on desktop by disabling log depth.

So that's fixable, but depth texture problems are... still a problem.

@likangning93
Copy link
Contributor

@likangning93
Copy link
Contributor

likangning93 commented Jun 28, 2018

Some additional reading:

Maybe the problem is that we're packing the depth (czm_packDepth) with an assumption of some source precision, and that's causing problems. I might try transferring the depth to a float texture to see what happens. [EDIT] This is maybe what the fix was in that Unity forum post. I talked about float texture problems on iOS in another issue, but maybe Apple optimized for [0.0, 1.0] range.

[EDIT] This is what czm_packDepth looks like:

/**
 * Packs a depth value into a vec3 that can be represented by unsigned bytes.
 *
 * @name czm_packDepth
 * @glslFunction
 *
 * @param {float} depth The floating-point depth.
 * @returns {vec3} The packed depth.
 */
vec4 czm_packDepth(float depth)
{
    // See Aras Pranckevičius' post Encoding Floats to RGBA
    // http://aras-p.info/blog/2009/07/30/encoding-floats-to-rgba-the-final/
    vec4 enc = vec4(1.0, 255.0, 65025.0, 16581375.0) * depth;
    enc = fract(enc);
    enc -= enc.yzww * vec4(1.0 / 255.0, 1.0 / 255.0, 1.0 / 255.0, 0.0);
    return enc;
}

That 16581375.0 looks an awful lot like 2^24 = 16777216, with some nudging around that I'd probably need to dive down the rabbit hole to figure out. But yeah, this smells like it's been optimized for 24 bit precision depth.

@likangning93
Copy link
Contributor

@chris-cooper in the meantime, you might be able to adjust the near/far ratio in your scene to get better results for the specific use case.

var viewer = new Cesium.Viewer('cesiumContainer', {
    terrainProvider : Cesium.createWorldTerrain()
});
var scene = viewer.scene;
scene.farToNearRatio = 50.0;

var loopPositions = Cesium.Cartesian3.fromDegreesArray([
    -111.94500779274114, 36.27638678884143,
    -111.90983004392696, 36.07985366173454
]);

var instance = new Cesium.GeometryInstance({
    geometry : new Cesium.GroundPolylineGeometry({
        positions : loopPositions,
        width : 4.0
    }),
    attributes : {
        show : new Cesium.ShowGeometryInstanceAttribute(),
        color : Cesium.ColorGeometryInstanceAttribute.fromColor(Cesium.Color.fromCssColorString('#67ADDF').withAlpha(0.7))
    }
});

var polylineOnTerrainPrimitive = new Cesium.GroundPolylinePrimitive({
    geometryInstances : instance
});
polylineOnTerrainPrimitive.appearance = new Cesium.PolylineColorAppearance();

scene.groundPrimitives.add(polylineOnTerrainPrimitive);

viewer.camera.lookAt(loopPositions[1], new Cesium.Cartesian3(5000.0, 5000.0, 5000.0));
viewer.camera.lookAtTransform(Cesium.Matrix4.IDENTITY);

I'm seeing that a near/far ratio of 50 seems ok for viewing lines at distances of centimeters to a couple meters. For further distances it seems to depend on the view angle. It might also depend on the terrain complexity at the particular view location.

But having to tweak far/near ratios for a specific platform really isn't ideal, I'll dig deeper on if we're unintentionally causing precision issues.

@likangning93
Copy link
Contributor

I might try transferring the depth to a float texture to see what happens. [EDIT] This is maybe what the fix was in that Unity forum post. I talked about float texture problems on iOS in another issue, but maybe Apple optimized for [0.0, 1.0] range.

Now I have learned two things:

  • on Desktop, you can't render to a single-channel float buffer, which in retrospect makes sense
  • on iOS you can't render to a float buffer at all, which in retrospect also kind of makes sense if their under-the-hood implementation for reading from float textures is actually an approximation

So copying globe depth to a float texture instead of the current packing isn't going to work. @bagnell also proved out that the packing isn't at fault here by providing the globe depth stencil texture directly to this on an experimental branch, we still had artifact problems.

@likangning93
Copy link
Contributor

likangning93 commented Jun 29, 2018

So I guess the conclusion is, depth texture precision on modern iOS devices just isn't sufficient.
Going back to that Unity post, their bug might also have been different since they use near/far 1/60 and I don't know if they bothered to test more view distances than the tens-of-meters in their images.

One more idea that @bagnell suggested is re-rendering all the terrain (and eventually 3D Tiles) to produce a depth texture instead of relying on the depth texture feature. If it worked this would also be an awesome addon for enabling support on platforms like Internet Explorer and lower-end mobile devices that don't have depth textures builtin, and it would also impact a lot of other Cesium features that depend on the depth texture.
But the big caveat is that it could be slow, and that's also still "if" we can get it working - note to people like future me, this isn't as straightforward as just packing gl_FragCoord.z into gl_FragColor in the fragment shader because the gl_FragCoord.z I think is part of the problem...

Could also be a lot of work to implement, so unfortunately I don't know how soon I'll be able to evaluate this.

@likangning93
Copy link
Contributor

likangning93 commented Jul 4, 2018

@chris-cooper another development: polyline-on-terrain problems don't seem to be as severe on the iPad 4. We've been testing (and reproducing the problem) on a first-generation iPad Pro, which supposedly has a similar SoC [EDIT] similar to the MP2F2LL/A iPad 5 mentioned above.

If you have older hardware available for your use case, that might also be an option.

@likangning93
Copy link
Contributor

Relevant issue on three.js: mrdoob/three.js#9092

@chris-cooper
Copy link
Contributor Author

Thanks for the extra info @likangning93

@likangning93 likangning93 changed the title GroundPolylinePrimitive renders poorly on ipad GroundPolylinePrimitive renders poorly on newer mobile devices Dec 5, 2018
@hpinkos
Copy link
Contributor

hpinkos commented Dec 7, 2018

Also reported by @Spec1alFx in #7377

@likangning93
Copy link
Contributor

Just since it hasn't been written down here yet, fallback possibility: use the regular multi-pass algorithm with the polyline geometry + vertex shader, disallow materials beyond color.

@likangning93 likangning93 changed the title GroundPolylinePrimitive renders poorly on newer mobile devices GroundPolylinePrimitive and Ground Primitive materials render poorly on newer mobile devices Sep 24, 2019
@likangning93
Copy link
Contributor

likangning93 commented Oct 12, 2019

Some notes from the couple odd times I got to play around with this:

  • it looks like depth interpolation on these devices is inaccurate as opposed to being just a depth buffer problem
  • using a dedicated varying for depth seems to work fine though

On Sandcastle, with a custom shader on a primitive (Updated 2020-07-28)

weasel_shit

The above Sandcastle computes UVs using depth from just drawing a Rectangle geometry, this is NOT going through our depth packing code or even using a depth texture. The shader splits the display to show computed UVs on the bottom and a diff between depth-by-varying and depth using gl_FragCoord.z. As you can see, in the iOS example there's always some discrepancy in the diff, which occasionally leads to mangled texture coordinates, which is easier to catch when the camera is moving. Granted, the diff has been multipled by 1000 to be visible, but on other platforms there's no difference shown even with that multiplier. Here it is on my Adreno 308 (Qualcomm 425) smartphone:

Screenshot_20191011-221337 (1)

@likangning93
Copy link
Contributor

depth interpolation on these devices is inaccurate

I put together a small example of this here: https://likangning93.github.io/webgl-examples/tests/depthPrecision/, code at https://github.com/likangning93/webgl-examples/blob/gh-pages/tests/depthPrecision/webgl-demo.js.

Similar to the above, the right third of the webgl display here is a diff between depth-by-varying and gl_FragCoord.z multipled by 1000 for visibility. Here's what I see on our first-gen iPad Pro:

Image from iOS

The red bands indicate a discrepancy here across the depth range.

For comparison, here it is on my Qualcomm 425/Adreno 308 Android device:

Screenshot_20191011-224641

@likangning93
Copy link
Contributor

Posted to the webgl dev list: https://groups.google.com/forum/#!topic/webgl-dev-list/O34GBqZBfmk

@xinaesthete
Copy link

Really encouraging to see some activity on this, good luck @likangning93

@lilleyse
Copy link
Contributor

lilleyse commented Jul 28, 2020

I'm testing #9064 this on a Lenovo IdeaPad Duet Chromebook. Good results so far. There's subtler discontinuities on frustum switches but it's so much better than before. I tested every Sandcastle in this issue (@likangning93 whew -- lotta work went into this github issue).

Note: I don't see the banding artifacts in #6735 (comment) so this device must have good enough depth texture precision. Need to test on the iPad pro next.

before
polygon-bad
after
polygon-good

before
flicker2
after
flicker

before
2018-06-27

after
ezgif com-optimize

@dennisadams
Copy link
Contributor

@lilleyse I hope the polylines gifs got mixed up.

There's subtler discontinuities on frustum switches

I've also noticed some frustum artifacts in entities and in classification.

@dennisadams
Copy link
Contributor

Before:
Peek 2020-07-29 16-27

After:
a

Certainly better, but still has a problem on the frustum split . Notice the discontinuity on the polyline too.

@lilleyse
Copy link
Contributor

@dennisadams that's a different issue that @IanLilleyT started looking into: #8953.

@lilleyse
Copy link
Contributor

Better view of the frustum discontinuity on an ipad 4:

developmentGround Polyline Material - Cesium Sandcastle

@dennisadams
Copy link
Contributor

@dennisadams that's a different issue that @IanLilleyT started looking into: #8953.

Actually this bug appears to predate #8953 as described there.
Also, I don't see any artifacts in that sandcastle (3D Tiles Photogrammetry Classification.html) which is classified by a 3D tileset, compared to the one I mentioned (in Classification Types.html) which is classified by a polygon.
I'll try to do some more debugging if I have time.

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

Successfully merging a pull request may close this issue.

6 participants