Add anchorLineColor to Cesium3DTileStyle #5295
Conversation
Source/Scene/Cesium3DTileFeature.js
Outdated
| * @default {@link Material.ColorType} | ||
| */ | ||
| material : { | ||
| //var polyline = this._polylineCollection.get(this._batchId); |
Source/Scene/Cesium3DTileFeature.js
Outdated
| return polyline.material; | ||
| } | ||
|
|
||
| if (!defined(this._material)) { |
There was a problem hiding this comment.
If there is no polyline, just return undefined.
There was a problem hiding this comment.
And also remove _material above.
Source/Scene/Cesium3DTileFeature.js
Outdated
| * | ||
| * @default {@link Material.ColorType} | ||
| */ | ||
| material : { |
There was a problem hiding this comment.
Instead of material, why not anchorLineColor? Polylines have a color material by default. You can get/set polyline.material.uniforms.color. If a polyline does not exist, return undefined or do nothing.
Source/Scene/Cesium3DTileStyle.js
Outdated
| return this._anchorLineColor; | ||
| }, | ||
| set : function(value) { | ||
| this._backgroundColor = value; |
There was a problem hiding this comment.
This should be setting this._anchorLineColor.
| @@ -136,6 +138,10 @@ define([ | |||
| feature.backgroundYPadding = style.backgroundYPadding.evaluate(frameState, feature); | |||
| feature.backgroundEnabled = style.backgroundEnabled.evaluate(frameState, feature); | |||
|
|
|||
| // Create a color material with fromType: | |||
| feature.material = Material.fromType('Color'); | |||
There was a problem hiding this comment.
With the anchorLineColor property suggestion above, this would become:
if (defined(feature.anchorLineColor)) {
feature.anchorLineColor = style.anchorLineColor.evaluateColor(frameState, feature);
}
|
Is anchor line the most common term for these types of lines? I've also heard drop lines and extruded lines. |
|
@oterral or @bagnell please also update the spec in CesiumGS/3d-tiles#124 when this is finalized. |
|
@oterral can you please add yourself to CONTRIBUTORS.md and send in a Contributor License Agreement (CLA) if we don't already have one? |
|
Ah right, I missed the CLA. OK, please discuss with @lilleyse; I might not be available tomorrow. |
|
Thx for the review. I will make the changes according to comments. |
|
@bagnell do you have a preference for the name ? I can change to dropLineColor if you prefer. |
|
I don't have a preference. I was just wondering if there was a commonly used name. If not, stick with anchorLine. |
oterral
force-pushed
the
vector-tiles-labels-translu
branch
2 times, most recently
from
3b39922 to
ccfce64
Compare
|
PR updated . I'm not sure if the build failure is related to this PR. |
Allow to modify the color of the anchor line via the style.