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.

Sign up for GitHub

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

Enable improved depth testing for all billboards #6802

Closed
wants to merge 8 commits into from
Closed

Enable improved depth testing for all billboards #6802

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
00d94d9
enable better depth testing for all billboards
Jul 13, 2018
54f6994
bump up range
Jul 13, 2018
b4baa2a
Merge branch 'master' into enable-billboard-depth
Jul 19, 2018
c2a35f4
wiggle room
Jul 19, 2018
b218f4a
fix context
Jul 19, 2018
912e14b
Merge branch 'master' into enable-billboard-depth
Jul 20, 2018
3293844
default disableDepthTestDistance to undefined
Jul 23, 2018
534c7b8
remove depth test distance ifdef
Jul 23, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions Source/DataSources/BillboardVisualizer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ define([
var defaultHorizontalOrigin = HorizontalOrigin.CENTER;
var defaultVerticalOrigin = VerticalOrigin.CENTER;
var defaultSizeInMeters = false;
var defaultDisableDepthTestDistance = 0.0;

var positionScratch = new Cartesian3();
var colorScratch = new Color();
Expand Down Expand Up @@ -156,7 +155,7 @@ define([
billboard.pixelOffsetScaleByDistance = Property.getValueOrUndefined(billboardGraphics._pixelOffsetScaleByDistance, time, pixelOffsetScaleByDistanceScratch);
billboard.sizeInMeters = Property.getValueOrDefault(billboardGraphics._sizeInMeters, time, defaultSizeInMeters);
billboard.distanceDisplayCondition = Property.getValueOrUndefined(billboardGraphics._distanceDisplayCondition, time, distanceDisplayConditionScratch);
billboard.disableDepthTestDistance = Property.getValueOrDefault(billboardGraphics._disableDepthTestDistance, time, defaultDisableDepthTestDistance);
billboard.disableDepthTestDistance = Property.getValueOrUndefined(billboardGraphics._disableDepthTestDistance, time);

var subRegion = Property.getValueOrUndefined(billboardGraphics._imageSubRegion, time, boundingRectangleScratch);
if (defined(subRegion)) {
Expand Down
3 changes: 1 addition & 2 deletions Source/DataSources/LabelVisualizer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ define([
var defaultHeightReference = HeightReference.NONE;
var defaultHorizontalOrigin = HorizontalOrigin.CENTER;
var defaultVerticalOrigin = VerticalOrigin.CENTER;
var defaultDisableDepthTestDistance = 0.0;

var positionScratch = new Cartesian3();
var fillColorScratch = new Color();
Expand Down Expand Up @@ -164,7 +163,7 @@ define([
label.pixelOffsetScaleByDistance = Property.getValueOrUndefined(labelGraphics._pixelOffsetScaleByDistance, time, pixelOffsetScaleByDistanceScratch);
label.scaleByDistance = Property.getValueOrUndefined(labelGraphics._scaleByDistance, time, scaleByDistanceScratch);
label.distanceDisplayCondition = Property.getValueOrUndefined(labelGraphics._distanceDisplayCondition, time, distanceDisplayConditionScratch);
label.disableDepthTestDistance = Property.getValueOrDefault(labelGraphics._disableDepthTestDistance, time, defaultDisableDepthTestDistance);
label.disableDepthTestDistance = Property.getValueOrUndefined(labelGraphics._disableDepthTestDistance, time);
}
return true;
};
Expand Down
5 changes: 2 additions & 3 deletions Source/Scene/Billboard.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ define([
this._pixelOffsetScaleByDistance = pixelOffsetScaleByDistance;
this._sizeInMeters = defaultValue(options.sizeInMeters, false);
this._distanceDisplayCondition = distanceDisplayCondition;
this._disableDepthTestDistance = defaultValue(options.disableDepthTestDistance, 0.0);
this._disableDepthTestDistance = options.disableDepthTestDistance;
this._id = options.id;
this._collection = defaultValue(options.collection, billboardCollection);

Expand Down Expand Up @@ -789,7 +789,6 @@ define([
* When set to zero, the depth test is always applied. When set to Number.POSITIVE_INFINITY, the depth test is never applied.
* @memberof Billboard.prototype
* @type {Number}
* @default 0.0
*/
disableDepthTestDistance : {
get : function() {
Expand All @@ -798,7 +797,7 @@ define([
set : function(value) {
if (this._disableDepthTestDistance !== value) {
//>>includeStart('debug', pragmas.debug);
if (!defined(value) || value < 0.0) {
if (defined(value) && value < 0.0) {
throw new DeveloperError('disableDepthTestDistance must be greater than or equal to 0.0.');
}
//>>includeEnd('debug');
Expand Down
12 changes: 5 additions & 7 deletions Source/Scene/BillboardCollection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1191,16 +1191,14 @@ define([
}

var disableDepthTestDistance = billboard.disableDepthTestDistance;
if (disableDepthTestDistance === 0.0 && context.depthTexture) {
disableDepthTestDistance = 4000.0;
if (!defined(disableDepthTestDistance)) {
disableDepthTestDistance = context.depthTexture ? 5000.0 : 0.0;
}

disableDepthTestDistance *= disableDepthTestDistance;
if (disableDepthTestDistance > 0.0) {
billboardCollection._shaderDisableDepthDistance = true;
if (disableDepthTestDistance === Number.POSITIVE_INFINITY) {
disableDepthTestDistance = -1.0;
}
billboardCollection._shaderDisableDepthDistance = true;
if (disableDepthTestDistance === Number.POSITIVE_INFINITY) {
disableDepthTestDistance = -1.0;
}

var imageHeight;
Expand Down
4 changes: 2 additions & 2 deletions Source/Scene/Label.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ define([
this._scaleByDistance = scaleByDistance;
this._heightReference = defaultValue(options.heightReference, HeightReference.NONE);
this._distanceDisplayCondition = distanceDisplayCondition;
this._disableDepthTestDistance = defaultValue(options.disableDepthTestDistance, 0.0);
this._disableDepthTestDistance = options.disableDepthTestDistance;

this._labelCollection = labelCollection;
this._glyphs = [];
Expand Down Expand Up @@ -938,7 +938,7 @@ define([
set : function(value) {
if (this._disableDepthTestDistance !== value) {
//>>includeStart('debug', pragmas.debug);
if (!defined(value) || value < 0.0) {
if (defined(value) && value < 0.0) {
throw new DeveloperError('disableDepthTestDistance must be greater than 0.0.');
}
//>>includeEnd('debug');
Expand Down
17 changes: 9 additions & 8 deletions Source/Shaders/BillboardCollectionVS.glsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,14 +196,6 @@ void main()

depthOrigin = vec2(1.0) - (depthOrigin * 0.5);
#endif
#if defined(VERTEX_DEPTH_CHECK) || defined(FRAGMENT_DEPTH_CHECK)
temp = compressedAttribute3.w;
temp = temp * SHIFT_RIGHT12;

vec2 dimensions;
dimensions.y = (temp - floor(temp)) * SHIFT_LEFT12;
dimensions.x = floor(temp);
#endif

#ifdef EYE_DISTANCE_TRANSLUCENCY
vec4 translucencyByDistance;
Expand All @@ -216,6 +208,15 @@ void main()
translucencyByDistance.w = ((temp - floor(temp)) * SHIFT_LEFT8) / 255.0;
#endif

#if defined(VERTEX_DEPTH_CHECK) || defined(FRAGMENT_DEPTH_CHECK)
temp = compressedAttribute3.w;
temp = temp * SHIFT_RIGHT12;

vec2 dimensions;
dimensions.y = (temp - floor(temp)) * SHIFT_LEFT12;
dimensions.x = floor(temp);
#endif

#ifdef ALIGNED_AXIS
vec3 alignedAxis = czm_octDecode(floor(compressedAttribute1.y * SHIFT_RIGHT8));
temp = compressedAttribute2.z * SHIFT_RIGHT5;
Expand Down
52 changes: 30 additions & 22 deletions Specs/Scene/BillboardCollectionSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,23 +113,23 @@ defineSuite([
expect(b.horizontalOrigin).toEqual(HorizontalOrigin.CENTER);
expect(b.verticalOrigin).toEqual(VerticalOrigin.CENTER);
expect(b.scale).toEqual(1.0);
expect(b.image).not.toBeDefined();
expect(b.image).toBeUndefined();
expect(b.color.red).toEqual(1.0);
expect(b.color.green).toEqual(1.0);
expect(b.color.blue).toEqual(1.0);
expect(b.color.alpha).toEqual(1.0);
expect(b.rotation).toEqual(0.0);
expect(b.alignedAxis).toEqual(Cartesian3.ZERO);
expect(b.scaleByDistance).not.toBeDefined();
expect(b.translucencyByDistance).not.toBeDefined();
expect(b.pixelOffsetScaleByDistance).not.toBeDefined();
expect(b.width).not.toBeDefined();
expect(b.height).not.toBeDefined();
expect(b.id).not.toBeDefined();
expect(b.scaleByDistance).toBeUndefined();
expect(b.translucencyByDistance).toBeUndefined();
expect(b.pixelOffsetScaleByDistance).toBeUndefined();
expect(b.width).toBeUndefined();
expect(b.height).toBeUndefined();
expect(b.id).toBeUndefined();
expect(b.heightReference).toEqual(HeightReference.NONE);
expect(b.sizeInMeters).toEqual(false);
expect(b.distanceDisplayCondition).not.toBeDefined();
expect(b.disableDepthTestDistance).toEqual(0.0);
expect(b.distanceDisplayCondition).toBeUndefined();
expect(b.disableDepthTestDistance).toBeUndefined();
});

it('can add and remove before first update.', function() {
Expand Down Expand Up @@ -287,23 +287,23 @@ defineSuite([
scaleByDistance : new NearFarScalar(1.0, 3.0, 1.0e6, 0.0)
});
b.scaleByDistance = undefined;
expect(b.scaleByDistance).not.toBeDefined();
expect(b.scaleByDistance).toBeUndefined();
});

it('disables billboard translucencyByDistance', function() {
var b = billboards.add({
translucencyByDistance : new NearFarScalar(1.0, 1.0, 1.0e6, 0.0)
});
b.translucencyByDistance = undefined;
expect(b.translucencyByDistance).not.toBeDefined();
expect(b.translucencyByDistance).toBeUndefined();
});

it('disables billboard pixelOffsetScaleByDistance', function() {
var b = billboards.add({
pixelOffsetScaleByDistance : new NearFarScalar(1.0, 1.0, 1.0e6, 0.0)
});
b.pixelOffsetScaleByDistance = undefined;
expect(b.pixelOffsetScaleByDistance).not.toBeDefined();
expect(b.pixelOffsetScaleByDistance).toBeUndefined();
});

it('renders billboard with scaleByDistance', function() {
Expand Down Expand Up @@ -461,11 +461,13 @@ defineSuite([
it('renders with disableDepthTestDistance', function() {
var b = billboards.add({
position : new Cartesian3(-1.0, 0.0, 0.0),
image : greenImage
image : greenImage,
disableDepthTestDistance : 0.0
});
billboards.add({
position : Cartesian3.ZERO,
image : blueImage
image : blueImage,
disableDepthTestDistance : 0.0
});

expect(scene).toRender([0, 0, 255, 255]);
Expand Down Expand Up @@ -620,7 +622,7 @@ defineSuite([
});

it('sets and gets a texture atlas', function() {
expect(billboards.textureAtlas).not.toBeDefined();
expect(billboards.textureAtlas).toBeUndefined();

var atlas = new TextureAtlas({ context : scene.context });
billboards.textureAtlas = atlas;
Expand Down Expand Up @@ -682,14 +684,16 @@ defineSuite([
it('adds and renders a billboard', function() {
billboards.add({
position : Cartesian3.ZERO,
image : greenImage
image : greenImage,
disableDepthTestDistance : 0.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you have to explicitly set disableDepthTestDistance for this to pass?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't think we have to, but when they're both set to the default 5000 the wrong one renders in front. Should I open an issue? I'll see if I can reproduce it for a normal case. Would it be weird for this since there's no globe and it's at 0,0,0?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bagnell yeah it looks like it's a bug with disableDepthTestDistance. I can reproduce it pretty easily, I'll open a new issue

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's because the disableDepthTestDistance is greater than the distance from the camera to the billboard

});

expect(scene).toRender([0, 255, 0, 255]);

billboards.add({
position : new Cartesian3(1.0, 0.0, 0.0), // Closer to camera
image : largeBlueImage
image : largeBlueImage,
disableDepthTestDistance : 0.0
});

expect(scene).toRender([0, 0, 255, 255]);
Expand All @@ -698,11 +702,13 @@ defineSuite([
it('removes and renders a billboard', function() {
billboards.add({
position : Cartesian3.ZERO,
image : greenImage
image : greenImage,
disableDepthTestDistance : 0.0
});
var blueBillboard = billboards.add({
position : new Cartesian3(1.0, 0.0, 0.0), // Closer to camera
image : largeBlueImage
image : largeBlueImage,
disableDepthTestDistance : 0.0
});

expect(scene).toRender([0, 0, 255, 255]);
Expand Down Expand Up @@ -915,13 +921,15 @@ defineSuite([

var b1 = billboards.add({
position : Cartesian3.ZERO,
image : greenImage
image : greenImage,
disableDepthTestDistance : 0.0
});
expect(scene).toRender([0, 255, 0, 255]);

var b2 = billboards.add({
position : new Cartesian3(1.0, 0.0, 0.0), // Closer to camera
image : largeBlueImage
image : largeBlueImage,
disableDepthTestDistance : 0.0
});
expect(scene).toRender([0, 0, 255, 255]);

Expand Down Expand Up @@ -1864,7 +1872,7 @@ defineSuite([
scene.globe.removedCallback = false;
b.heightReference = HeightReference.NONE;
expect(scene.globe.removedCallback).toEqual(true);
expect(scene.globe.callback).not.toBeDefined();
expect(scene.globe.callback).toBeUndefined();
});

it('changing the position updates the callback', function() {
Expand Down
28 changes: 16 additions & 12 deletions Specs/Scene/LabelCollectionSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,12 @@ defineSuite([
expect(label.horizontalOrigin).toEqual(HorizontalOrigin.LEFT);
expect(label.verticalOrigin).toEqual(VerticalOrigin.BASELINE);
expect(label.scale).toEqual(1.0);
expect(label.id).not.toBeDefined();
expect(label.translucencyByDistance).not.toBeDefined();
expect(label.pixelOffsetScaleByDistance).not.toBeDefined();
expect(label.scaleByDistance).not.toBeDefined();
expect(label.distanceDisplayCondition).not.toBeDefined();
expect(label.disableDepthTestDistance).toEqual(0.0);
expect(label.id).toBeUndefined();
expect(label.translucencyByDistance).toBeUndefined();
expect(label.pixelOffsetScaleByDistance).toBeUndefined();
expect(label.scaleByDistance).toBeUndefined();
expect(label.distanceDisplayCondition).toBeUndefined();
expect(label.disableDepthTestDistance).toBeUndefined();
});

it('can add a label with specified values', function() {
Expand Down Expand Up @@ -385,7 +385,8 @@ defineSuite([
position : Cartesian3.ZERO,
text : solidBox,
horizontalOrigin : HorizontalOrigin.CENTER,
verticalOrigin : VerticalOrigin.CENTER
verticalOrigin : VerticalOrigin.CENTER,
disableDepthTestDistance : 0.0
});

expect(scene).toRenderAndCall(function(rgba) {
Expand All @@ -404,7 +405,8 @@ defineSuite([
alpha : 1.0
},
horizontalOrigin : HorizontalOrigin.CENTER,
verticalOrigin : VerticalOrigin.CENTER
verticalOrigin : VerticalOrigin.CENTER,
disableDepthTestDistance : 0.0
});

expect(scene).toRenderAndCall(function(rgba) {
Expand Down Expand Up @@ -689,14 +691,16 @@ defineSuite([
text : solidBox,
fillColor : Color.LIME,
horizontalOrigin : HorizontalOrigin.CENTER,
verticalOrigin : VerticalOrigin.CENTER
verticalOrigin : VerticalOrigin.CENTER,
disableDepthTestDistance : 0.0
});
labels.add({
position : Cartesian3.ZERO,
text : solidBox,
fillColor : Color.BLUE,
horizontalOrigin : HorizontalOrigin.CENTER,
verticalOrigin : VerticalOrigin.CENTER
verticalOrigin : VerticalOrigin.CENTER,
disableDepthTestDistance : 0.0
});

expect(scene).toRender([0, 0, 255, 255]);
Expand Down Expand Up @@ -2321,7 +2325,7 @@ defineSuite([
scene.globe.removedCallback = false;
l.heightReference = HeightReference.NONE;
expect(scene.globe.removedCallback).toEqual(true);
expect(scene.globe.callback).not.toBeDefined();
expect(scene.globe.callback).toBeUndefined();
});

it('changing the position updates the callback', function() {
Expand Down Expand Up @@ -2384,7 +2388,7 @@ defineSuite([
var spy = spyOn(billboard, '_removeCallbackFunc');
labelsWithHeight.remove(l);
expect(spy).toHaveBeenCalled();
expect(labelsWithHeight._spareBillboards[0]._removeCallbackFunc).not.toBeDefined();
expect(labelsWithHeight._spareBillboards[0]._removeCallbackFunc).toBeUndefined();
});
});

Expand Down