Coder Social home page Coder Social logo

Declarative styling about 3d-tiles HOT 94 OPEN

cesiumgs avatar cesiumgs commented on May 19, 2024
Declarative styling

from 3d-tiles.

Comments (94)

lilleyse avatar lilleyse commented on May 19, 2024 1

@lilleyse is it reasonable for you to make this change (code and spec) as part of your current point cloud declarative styling work?

Yes it should be easy to change. For the spec I'll open a separate PR for this fix. For the code I'll make the change in my styling PR.

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

The immediate need for declarative 3D Tiles styling is to be able to write a simple expression that maps a feature's property (or properties) value to its appearance (initially, show/color), e.g.,

  • "Show all buildings greater than 10 meters tall"
  • "Use a color ramp to color buildings based on the number of floors"

There's early implementation work in Cesium's 3d-tiles-style branch.

The styling format is JSON, and the schema is not at all final. Here's a few examples:

// Show all buildings greater than 10 meters tall, e.g., `${Height} > 10`
{
    "show" : {
        "leftOperand" : "${Height}",
        "operator" : ">",
        "rightOperand" : 10
    }
}
// Create a color ramp  with the provided palette based on the number of floors in a building
{
    "color" : {
        "expression" : {  // Will make this less verbose soon, just ${Floors}
            "leftOperand" : "${Floors}",
            "operator" : "+",
            "rightOperand" : 0
        },
        "intervals" : [
            1,  // [1, 10)
            10, // [10, 19)
            20  // [20, infinity)
        ],
        "colors" : [
            [237, 248, 251],
            [158, 188, 218],
            [129, 15, 124]
        ]
    }
}
// Map a feature's ID to a color
{
    "color" : {
        "propertyName" : "id",
        "map" : {
            "1" : [255, 0, 0],
            "2" : [0, 255, 0]
        },
        "default" : [255, 255, 255]
    }
}

In Cesium, a style can be applied to a Cesium3DTileset object, e.g.,

tileset.style = new Cesium.Cesium3DTileStyle(tileset, styleJson);

Creating the Cesium3DTileStyle object basically "complies" the style, and assigning it to tileset.style tells the tileset to apply the style (efficiently based on visibility and if the style or feature properties have changed).

After the style is compiled, it can be changed, e.g.,

tileset.style.show.operator = '===';

Feature properties that may impact how the style is evaluated can also be changed, e.g.,

feature.setProperty('id', 1);

In addition to declarative styling, the lower-level API can be used to override the appearance of individual features, e.g.,

var feature = scene.pick(movement.endPosition);
if (Cesium.defined(feature)) {
    feature.color = Cesium.Color.YELLOW;
}

Outdated, but still useful:
img_0350

Schema Ideas

  • Shorthand schema for literal boolean expression for show
  • Use CSS color formats throughout
  • Add RegEx pattern to color ramp
  • Resolve an expression (including reference to a property name) to a color (using optional regex)
  • Replace expression JSON with real expressions, e.g., ${Height} * 2 > (${Width} + 1) / 3. The JSON becomes the AST, which is (initially if not always) not part of the spec, but rather a Cesium implementation detail.

Related-ish

from 3d-tiles.

pierotofy avatar pierotofy commented on May 19, 2024

I'd much rather use a concise syntax (e.g. "{Height} > 100") than individual properties, which would get really messy for complex rules.

A simple parser can transform the concise syntax into the equivalent structure in a precompilation step.

Something that I could see being used is also the ability to specify styling programmatically, so maybe define an abstract "Cesium3DTileStyleFormatter" interface in Cesium that can be implemented by the developer to specify the styling at runtime, e.g.:

var sf = new MyStyleFormatter();
// ...
tileset.style = new Cesium.Cesium3DTileStyle(tileset, sf);

Where MyStyleFormatter implements:

show: function(tile){
    return tile.Height > 100;
}

It could be useful to do things like:

color: function(tile){
   return new StaticColor(Math.random() * 255, Math.random() * 255, Math.random() * 255);
}

To color buildings at randoms (and other possibilities).

In fact a JSON style could be a special case of programmatic styling:

var sf = new JsonStyleFormatter(styleJson);
// ...
tileset.style = new Cesium.Cesium3DTileStyle(tileset, sf);

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

Thanks for the input, @pierotofy. We definitely want to support custom functions for expressions, and, I agree, will most likely go with the concise syntax. Thanks for the code snippets!

from 3d-tiles.

mramato avatar mramato commented on May 19, 2024

This is one area where I think Cesium's current Property system can be
leveraged to create some powerful capabilities. I'm not saying the
Property system exactly as it is today is a perfect fit, but ultimately I
personally feel it's where things will go. I've wanted to add additional
Property capabilities for a while and this may be a good reason to look
into that.

On Fri, Feb 5, 2016 at 10:16 AM, Patrick Cozzi [email protected]
wrote:

Thanks for the input, @pierotofy https://github.com/pierotofy. We
definitely want to support custom functions for expressions, and, I agree,
will most likely go with the concise syntax. Thanks for the code snippets!


Reply to this email directly or view it on GitHub
#2 (comment)
.

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

Update #2 (comment) to account for the latest implementation work and offline discussion with @mramato.

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

The styling spec will be in the 3d-tiles/spec branch (just a placeholder for now). I'll bootstrap the prose/schema writing as things start to solidify.

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

Notes on parsers

We'll start with jsep.

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

Notes on expressions

Now

  • Types
    • Number
    • Boolean
    • String
      • Might be a jsep bug with escaped quotes, e.g., "a \"b\" c"
    • Color (CSS color format)
    • Variable (feature property) - resolves to Number, Noolean, String, or Color (or null I suppose)
  • Short-circuit expressions
  • Member expressions
    • Spec
    • Implementation
  • Arrays
  • Boolean, Number, String constructors
    • Spec
    • Implementation
  • RegEx
    • Implementation
    • Spec
    • toString
  • Evaluate colors take non-literal arguments in constructor
  • Constructor function changes
  • Fast path for well-known expressions, e.g., ${Property} + literal
  • Remove debugging console.log statements
  • Examples / test cases (good start, but not complete):
"show" : 'a // Syntax error
"show" : true // Boolean literal
"show" : !false // Unary NOT
"show" : true && true // Logical AND
"show" : false && true // Logical AND, short-circuit
"show" : false || true // Logical OR
"show" : true || false // Logical OR, short-circuit
"show" : (false && false) || (true && true)
"show" : true ? true : false // Conditional, short-circuit-ish
"show" : false ? false : true // Conditional, short-circuit-ish
"show" : 2 > 1 // And similiar for <, <=, >=, ===, !===
"show" : 0 > -1 // Unary NEGATE
"show" : (1 + 1) > 1 // *, /, %
// For now, do not support:
//   * Unary: ~, +
//   * Binary : |, ^, &, ==, !=, <<, >>, >>>
//   * Expressions
//     * Array, e.g., [1, 2]
//     * Compound, e.g., 1; 2; 3
//     * Member, e.g., feature.name // might need this soon
//     * This, e.g., this
// See node types, operations, and literals in the annotated source: http://jsep.from.so/annotated_source/jsep.html
"show" : "${SomeFeatureProperty}" // Feature property is a boolean
"show" : "${ZipCode} === 19341" // Variable equals number
"show" : "${Temperature} > 100"
"show" : "(${Temperature} > 100) && ((${Weight} / ${Area}) > 2.0)"
"show" : "${County} === 'Chester'" // Property name equals string
"show" : "${County} === regExp('/^Chest/')" // String compare with RegEx. Open to other syntax
"show" : "${County} !== null" // I guess we should support null, what about undefined?
"show" : 1 // Convert number to boolean I suppose
// CSS colors
"color" : "#EAA56C"
"color" : "cyan"
"color" : "rgb(100, 255, 190)"
"color" : "hsl(250, 60%, 70%)"
"color" : "(${Temperature} === 'hot') ? 'red', '#ffffff'"
"color" : "rgb(255, 0, 0, (${Visible} ? 255 : 0))"
"color" : "rgb(${red}, ${green}, ${blue}, 255)" // Number properties
"color" : "rgb(${red} * 255, ${green} * 255, ${blue} * 255, 255)" // Convert 0..1 to 0..255

from 3d-tiles.

pierotofy avatar pierotofy commented on May 19, 2024

For RegEx syntax we could also use (borrowed from Perl/Ruby):

"show" : "${County} =~ /^Chest/" // Matches
"show" : "${County} !~ /^Chest/" // Does not match

null and undefined should probably be both supported (and enforce strong typing):

"show" : "${County} !== null && ${Country} !== undefined"
"show" : "${County} !== null" // Does not match undefined values
"show" : "${County} !== undefined" // Does not match null values

I would vote in favor of not allowing casts from numbers to bools, just to enforce better code practices, but it's not a big deal if they are allowed.

"show" : "1" // Error: "Expected bool, got number"
"show" : "${param} === 1" // OK

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

Thanks @pierotofy, @ggetz is starting the design/implementation now. We'll look at these cases; I'm not sure about introducing =~ yet though as we want to map to JavaScript as best as we can.

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

@ggetz check out the new Color section in the styling spec and let me know what you think about the TODOs (just open PRs for them) and please make sure our implementation and spec are in-sync and that we reasonably covered the edge cases.

from 3d-tiles.

ggetz avatar ggetz commented on May 19, 2024

I added my comments in #70

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

@ggetz there is a new section on Number. Let me know your thoughts and please bring our implementation in sync. Also note that isNaN() is true and isFinite() is false due to implicit conversion to Number.

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

@ggetz there is a new section on conversions. Please make sure to test all possible Color conversions carefully, e.g., Color() !== undefined, Color() !== null, !Color() === false, etc. We want the semantics to be the same as if we were using the Cesium Color object in JavaScript.

Note that we have to add a toString() function to Color for implicit conversion to string (or explicit via Color().toString()). We'll have to add a few more functions to make it walk and talk like a real JavaScript object. More info to follow.

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

@ggetz for strings, make sure we parse both ' and ".

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

@ggetz there is a new section on operators. Please make sure our implementation is in-sync (in particular I added the unary + and I don't know if we implemented ternary yet) and carefully tested.

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

@ggetz are we in-sync with this:

Color supports the following binary operators by performing component-wise operations: ===, !==, +, -, *, /, and %. For example Color() === Color() is true since the red, green, blue, and alpha components are equal.

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

Note that we have to add a toString() function to Color for implicit conversion to string (or explicit via Color().toString()). We'll have to add a few more functions to make it walk and talk like a real JavaScript object. More info to follow.

I looked more at this. Just toString() is fine for our current purposes. See Section 19 here if interested.

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

@ggetz do you want to design the regex support?

from 3d-tiles.

ggetz avatar ggetz commented on May 19, 2024

Color supports the following binary operators by performing component-wise operations: ===, !==, +, -, *, /, and %. For example Color() === Color() is true since the red, green, blue, and alpha components are equal.

Yes this is implemented

from 3d-tiles.

ggetz avatar ggetz commented on May 19, 2024

do you want to design the regex support?

Sure

from 3d-tiles.

ggetz avatar ggetz commented on May 19, 2024

There is a new section on Number. Let me know your thoughts and please bring our implementation in sync. Also note that isNaN() is true and isFinite() is false due to implicit conversion to Number.

Looks good, I will put this in the implementation

from 3d-tiles.

ggetz avatar ggetz commented on May 19, 2024

there is a new section on conversions. Please make sure to test all possible Color conversions carefully, e.g., Color() !== undefined, Color() !== null, !Color() === false, etc. We want the semantics to be the same as if we were using the Cesium Color object in JavaScript.

OK

there is a new section on operators. Please make sure our implementation is in-sync (in particular I added the unary + and I don't know if we implemented ternary yet) and carefully tested.

Yes, I'll have to add in unary + and ternary.

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

@ggetz there's a new section on variables. Please make sure to test all the different datatypes.

from 3d-tiles.

ggetz avatar ggetz commented on May 19, 2024

for strings, make sure we parse both ' and ".

I believe jsep handles this, I will add a test to confirm.

from 3d-tiles.

ggetz avatar ggetz commented on May 19, 2024

there's a new section on variables. Please make sure to test all the different datatypes.

For variables, do we support multilevel properties? For example,

feature : {
    property : {
        subproperty : 1
    }
}

can this be reached by ${property.subproperty} ?

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

For variables, do we support multilevel properties?

Probably, see #65, I just need to double check everything will work out throughout the pipeline.

Hold on this for the moment.

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

@ggetz what is the behavior of CSS colors for values out of range (e.g., red = 300). Please make sure we match it, e.g., throw error, clamp, etc.

from 3d-tiles.

mramato avatar mramato commented on May 19, 2024

Why does HTML think “chucknorris” is a color?

from 3d-tiles.

ggetz avatar ggetz commented on May 19, 2024

when CSS colors are out of range with the rgb(), hsl(), etc.. functions, they just clamp the value to the max and min.

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

OK, please make sure we do the same.

Also, what about a string that isn't hex or is too long?

from 3d-tiles.

ggetz avatar ggetz commented on May 19, 2024

That's covered in the the link @mramato posted.

make the string a length that is a multiple of 3 by adding 0s: chucknorris0
separate the string into 3 equal length strings: chuc knor ris0
truncate each string to 2 characters: ch kn ri
keep the hex values, and add 0's where necessary: C0 00 00

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

OK, our implementation should work the same.

from 3d-tiles.

ggetz avatar ggetz commented on May 19, 2024

Trying the behavior myself in chrome, the color is ignored if it is not of the form #fff or #ffffff or an accepted color keyword, and ignores the color if there are any non-hex charters. Should we go with the legacy behavior described in the link or the chrome behavior?

Also I've been using Color.fromCssColorString() to convert the css string. If we go with the former behavior, should I modify the fromCssColorString function?

from 3d-tiles.

mramato avatar mramato commented on May 19, 2024

We should follow the specification and if Color.fromCssColorString deviates from the spec, we should fix it. MDN has a summary and then links to the actual part of the spec that deals with color:

MDN: https://developer.mozilla.org/en-US/docs/Web/CSS/color
CSS3 Colors: https://drafts.csswg.org/css-color-3/#color

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

@mramato is right in general, but is @ggetz saying that Chrome implements colors in CSS differently than the spec?

from 3d-tiles.

ggetz avatar ggetz commented on May 19, 2024

The css3 color spec says:

The format of an RGB value in hexadecimal notation is a ‘#’ immediately followed by either three or six hexadecimal characters. The three-digit RGB notation (#rgb) is converted into six-digit form (#rrggbb) by replicating digits, not by adding zeros. For example, #fb0 expands to #ffbb00. This ensures that white (#ffffff) can be specified with the short notation (#fff) and removes any dependencies on the color depth of the display.

Which matches the behavior of Chrome (and Color.fromCssColorString)

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

So it sounds like everything is in-sync?

from 3d-tiles.

ggetz avatar ggetz commented on May 19, 2024

Yes.

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

👍

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

@ggetz can you bring the implementation in-sync with the latest on dot and bracket notation for member and array access? See

Make sure to test a complex nested use case, e.g., feature properties like:

{
    "aaa" : {
        "bbb" : "ccc",
        "values" : [{
            "ddd" : "eee",
            "ggg" : [1, 2, 3]
        }, {
            "ddd" : "fff",
            "ggg" : [4, 5, 6]
        }]
    }
}

(use better names)

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

@ggetz also let me know what you think about this case:

var foo = {
  "a.b" : "string",
  "a" : {
    "b" : "object"
  }
};

console.log(foo2.a.b); // object
console.log(foo2['a.b']); // string

With style expressions, what does ${a.b} evaluate to? Perhaps object, and then ${'a.b'} evaluates to string.

We could also require an explicit feature in this case (or always), e.g., ${feature.a.b} and ${feature['a.b']}. Then we need to handle the case when a feature has a feature property, e.g., perhaps ${feature} === ${feature.feature}

If this is really painful, we could explicitly disallow this for now, give precedence to object in this case, and make it a TODO.

from 3d-tiles.

ggetz avatar ggetz commented on May 19, 2024

${a.b} and ${'a.b'} seem more intuitive and cleaner to me. Requiring an explicit feature without always requiring it is inconsistent. Also by not requiring the explicit feature, we don't need to worry about the feature property case.

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

OK with me, see how the implementation feels.

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

@ggetz also please update the spec when you confirm this.

from 3d-tiles.

ggetz avatar ggetz commented on May 19, 2024

also please update the spec when you confirm this.

OK

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

@ggetz the spec currently says

Explicit Boolean, Number, and String constructor functions are not supported.

However, for example, if a tile stores YearBuilt as a string, an expression like

${YearBuilt} >= 1970

"works" because of implicit conversion, but

${YearBuilt} === 1970

is never true. Perhaps we should add Boolean, Number, and String for explicit conversion:

Number(${YearBuilt}) === 1970

If you agree, please update the spec and code.

from 3d-tiles.

ggetz avatar ggetz commented on May 19, 2024

The unary + operator I recently implemented will convert a string to a number. So in this case, you can use +${YearBuilt} === 1970 to check if this is true. Is that sufficient?

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

For this specific case, yes. Given that we have omitted == and !=, I still think we will need these for other cases, but we can wait until we have more experience with styling to see.

from 3d-tiles.

ggetz avatar ggetz commented on May 19, 2024

OK, I agree these should be added. I will add this to the roadmap.

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

I will add this to the roadmap.

Also, some useful info is here: http://stackoverflow.com/questions/4719320/new-number-vs-number

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

@ggetz here's some notes for when you are ready to integrate the new expressions into the style JSON schema.

  • All the various show options go away, and show is always a string with an expression that evaluates to Boolean.
  • color can be a string with an expression that evaluates to Color or an object for a color map or ramp (replacing the highlighted code here).

Here's a map:

"color" : {
    "key" : "RegEx('^1(\\d)$').exec(${id})",
    "map" : {
        "1" : "Color('#FF0000')",
        "2" : "Color('#00FF00')"
    },
    "defaultValue" : "Color('#FFFFFF')"
}

Note that the value in each pair in the map (e.g., "Color('#FF0000')") is an expression evaluating to a Color so it could include variables, etc.

Likewise, I'm pretty sure we will want the key in each pair to also be an expression (kinda like you had before), and we can optimize the case where all keys in the map are literals by building the hash lookup at compile time.

For "color ramps" or "intervals", I think it is best to start with a generic set of conditionals that we execute like a series of if...else statements, e.g.,

"color" : {
    "key" : "${Height} / ${Area}",
    "conditional" : {
        "(${KEY} >= 1.0)  && (${KEY} < 10.0)"  : "Color('#FF00FF')",
        "(${KEY} >= 10.0) && (${KEY} < 30.0)"  : "Color('#FF0000')",
        "(${KEY} >= 30.0) && (${KEY} < 50.0)"  : "Color('#FFFF00')",
        "(${KEY} >= 50.0) && (${KEY} < 70.0)"  : "Color('#00FF00')",
        "(${KEY} >= 70.0) && (${KEY} < 100.0)" : "Color('#00FFFF')",
        "(${KEY} >= 100.0)"                    : "Color('#0000FF')",
    }
}

key could be optional (or even not part of the schema, but it adds a nice readability and easy optimization for complete expressions). Each conditional property has an expression that evaluates to Boolean and an expression that evaluates to Color.

This approach allows a lot of flexibility and avoids a bunch of schema for closed/open intervals; however, it isn't well suited to a binary search for evaluation (it is basically a linear search). Optimizing it would require a bunch of analysis; later, if we need, we could just introduce an interval schema.

Note that these color maps and ramps could be implemented completely in an expression with the ternary operator, but it would be ugly, painful to generate, and harder to optimize at runtime.

from 3d-tiles.

ggetz avatar ggetz commented on May 19, 2024

For "color ramps" or "intervals", I think it is best to start with a generic set of conditionals that we execute like a series of if...else statements

Aren't a map and ramp pretty much the same thing? For example, you can express the "map" in "ramp" syntax pretty easily if we add "defaultValue":

"color" : {
    "key" : "RegEx('^1(\\d)$').exec(${id})",
    "conditional" : {
        "${key} === 1" : "Color('#FF0000')",
        "${key} === 2" : "Color('#00FF00')"
    },
    "defaultValue" : "Color('#FFFFFF')"
}

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

I actually had that at one point, but made them separate so it would be easy to optimize maps with all literals, e.g., build the hash (JavaScript object) at compile time. However, I think it is still pretty easy to do the same compile-time analysis with conditional (just look for === LITERAL for all properties). Give it a shot!

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

@ggetz I tightened up the spec in 20d8201. Can you please review?

A few questions:

  • Does our RegExp type have a toString() function like Color? It should.
  • We need to explain how operators work with RegExp (even if we just disable them for now). For example, colors say this:

Colors support the following binary operators by performing component-wise operations: ===, !==, +, -, *, /, and %. For example color() === color() is true since the red, green, blue, and alpha components are equal.

  • It's fine to say the following, but we also need to document the arguments like we do for Color, isNaN, etc.

The regExp function behaves like the JavaScript RegExp constructor and takes the same arguments.

  • Can you add a statement to the variables section like 'Variables can be used anywhere an expression can be used except...' or can they be used everywhere (in which case, we should make that explicit)?

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

@ggetz to organize things, I moved our task lists from #2 (comment) and #2 (comment) to the top comment: #2 (comment)

from 3d-tiles.

ggetz avatar ggetz commented on May 19, 2024

Does our RegExp type have a toString() function like Color? It should.

Not an explicit function, I will add one.

We need to explain how operators work with RegExp (even if we just disable them for now). For example, colors say this:

I would say operators do not work with RegExp at all for now.

I'll add the other items to the spec as well.

from 3d-tiles.

ggetz avatar ggetz commented on May 19, 2024

Also the changes in 20d8201 look good.

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

@ggetz I stepped through the code in Expression.js and made some minor tweaks in CesiumGS/cesium@10c8c9f

Here's a few questions/TODOs:

  • Check over all the try blocks and only do the part of the code that might throw instead the try block.
  • Does _evaluateLiteralString need to call replaceBackslashes? Can we do this at compile time instead?
  • Can we do the instanceof checks in _evaluatePlus at compile time, e.g., so we will end up with more _evaluate functions but they will be optimized for their particular type instead of doing type checking at runtime. If it helps make things clean in the compile step, we could even do a first pass over the AST to compute type info, then a second pass to generate the runtime AST. Not sure that the extra pass is necessary though.
  • All of the calls to new Color (like in _evaluatePlus) will be bad for performance. Can we make color.evaluate take a result parameter (see styleContent in Cesium3DTileStyleEngine.js) and use scratch variables?

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

@ggetz test coverage for Expression.js is pretty good at 98%. Can you please add tests for the DeveloperErrors that are not covered to bring coverage up to 100%?

from 3d-tiles.

ggetz avatar ggetz commented on May 19, 2024

Can we do the instanceof checks in _evaluatePlus at compile time, e.g., so we will end up with more _evaluate functions but they will be optimized for their particular type instead of doing type checking at runtime.

I'm guessing this would apply to all of the arithmetic _evaluate functions?

from 3d-tiles.

ggetz avatar ggetz commented on May 19, 2024

Also we could do some of the instanceof checks at compile time, but we would need to leave one evaluate function with the instanceof checks in at runtime because of expressions where the types are not clear until evaluation, for example ${myColor} + 3 * color('red')

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

I'm guessing this would apply to all of the arithmetic _evaluate functions?

Yes.

Also we could do some of the instanceof checks at compile time, but we would need to leave one...

OK.

from 3d-tiles.

ggetz avatar ggetz commented on May 19, 2024

Should we wait for the optimized evaluate functions? I believe they are currently on the roadmap under "Later".

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

Should we wait for the optimized evaluate functions?

Will it be significantly time consuming or significantly deteriorate the code quality? If not, I would do it now. If so, then we can evaluate later.

from 3d-tiles.

ggetz avatar ggetz commented on May 19, 2024

If it's targeted to the operators involving Colors, it shouldn't be too time consuming.

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

@ggetz did you add key to CesiumGS/cesium#3655 as suggested in #2 (comment)?

For example:

"color" : {
    "key" : "${Height} / ${Area}",
    "conditional" : {
        "(${KEY} >= 1.0)  && (${KEY} < 10.0)"  : "color('#FF00FF')",
        "(${KEY} >= 10.0) && (${KEY} < 30.0)"  : "color('#FF0000')",
        "(${KEY} >= 30.0) && (${KEY} < 50.0)"  : "color('#FFFF00')",
        "(${KEY} >= 50.0) && (${KEY} < 70.0)"  : "color('#00FF00')",
        "(${KEY} >= 70.0) && (${KEY} < 100.0)" : "color('#00FFFF')",
        "(${KEY} >= 100.0)"                    : "color('#0000FF')"
    }
}

If not, can you please add it and name it expression since key came from the map terminology?

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

Also, what do you think of renaming conditional in the schema to conditions, e.g.,

"color" : {
    "expression" : "RegEx('^1(\\d)$').exec(${id})",
    "conditional" : {
        "{$EXPRESSION} === 1" : "color('#FF0000')",
        "{$EXPRESSION} === 2" : "color('#00FF00')",
        "true" : "color('#FFFFFF')"
    }
}

becomes

"color" : {
    "expression" : "RegEx('^1(\\d)$').exec(${id})",
    "conditions" : {
        "{$EXPRESSION} === 1" : "color('#FF0000')",
        "{$EXPRESSION} === 2" : "color('#00FF00')",
        "true" : "color('#FFFFFF')"
    }
}

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

@ggetz did you add key to CesiumGS/cesium#3655 as suggested in #2 (comment)?

Also, later, we could consider expanding key/expression into an object that could hold multiple expressions for complex conditions, e.g.,

    "color" : {
        "expressions" : {
            "id" : "RegEx('^1(\\d)$').exec(${id})",
            "area" : "${length} * ${height}"
        },
        "conditions" : {
            "({$ID} !== 1) && (${AREA} > 0)" : "color('#FF0000')"
        }
    }

This is overkill for now, but let's consider it if experience shows it will be useful.

from 3d-tiles.

ggetz avatar ggetz commented on May 19, 2024

"Expression" and "conditions" sound good. For expression, I think ${expression} is more straight forward than {$EXPRESSION} because it looks like the other variables.

I do like the idea of multiple expressions as well.

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

I think ${expression} is more straight forward than {$EXPRESSION} because it looks like the other variables.

OK.

Also, this will override a property named expression so, in that case, they need to use feature.expression.

I do like the idea of multiple expressions as well.

OK, let's add it when we need it.

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

I do like the idea of multiple expressions as well.

Also, I already updated the roadmap for this.

from 3d-tiles.

ggetz avatar ggetz commented on May 19, 2024

Also, I already updated the roadmap for this.

OK thanks.

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

First cut of the spec is in #80.

from 3d-tiles.

ggetz avatar ggetz commented on May 19, 2024

First cut of the spec is in #80.

OK

from 3d-tiles.

ggetz avatar ggetz commented on May 19, 2024

Suggestions for new name:

  • 3D Tiles Ink
  • 3D Tiles Flux
  • 3D Tiles Chalk
  • 3D Tiles Pigment
  • 3D Tiles Embellish
  • 3D Tiles Flourish

from 3d-tiles.

twpayne avatar twpayne commented on May 19, 2024

tl;dr The current specification for conditions requires JSON object property order to be preserved, which is rarely true.

JavaScript's for ... in statement iterates over object properties in an arbitrary order. The popular Java library org.json defines a JSON object as "an unordered collection of name/value pairs". Iteration order over Golang maps (a common internal representation for JSON objects) is randomized.

Concrete example: Styling/README.md contains the example

{
    "show" : "${Area} > 0",
    "color" : {
        "conditions" : {
            "${Height} < 60" : "color('#13293D')",
            "${Height} < 120" : "color('#1B98E0')",
            "true" : "color('#E8F1F2', 0.5)"
        }
    }
}

A feature with property Height == 0 matches all three conditions, and therefore its computed color will depend entirely on the evaluation order of the conditions.

Generating conditions objects automatically will also be problematic, as some languages and libraries do not allow the order of properties in the serialized JSON output to be controlled.

Suggestion: either require that all conditions are mutually exclusive (tricky to enforce) or replace the conditions object with an ordered Array of [condition, value] pairs (uglier, but well defined).

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

Good eye, @twpayne! I'm surprised we haven't ran into this issue in practice already.

replace the conditions object with an ordered Array of [condition, value] pairs (uglier, but well defined).

+1 for this.

@lilleyse is it reasonable for you to make this change (code and spec) as part of your current point cloud declarative styling work?

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

Yes it should be easy to change. For the spec I'll open a separate PR for this fix. For the code I'll make the change in my styling PR.

In CesiumGS/cesium@5cb1088, part of CesiumGS/cesium#4336.

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

Labeling draft 1.0 just for the utility functions and finishing the vector types.

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

@ggetz is there anything here critical for 1.0?

from 3d-tiles.

ronanmcnulty avatar ronanmcnulty commented on May 19, 2024

Is there anyway to color based on whether the lat & lng are within a polygon?

from 3d-tiles.

ggetz avatar ggetz commented on May 19, 2024

@ronanmcnulty It sounds like classification might fit your use case better.

from 3d-tiles.

ThornM9 avatar ThornM9 commented on May 19, 2024

Thanks for the input, @pierotofy. We definitely want to support custom functions for expressions, and, I agree, will most likely go with the concise syntax. Thanks for the code snippets!

Were custom functions for expressions ever implemented @pjcozzi ? I'm attempting to color a point cloud by elevation but there's no height attribute for point clouds.

from 3d-tiles.

pjcozzi avatar pjcozzi commented on May 19, 2024

@ThornM9 no but there may be another way to achieve your use case, the best place to ask is the Cesium forum: https://groups.google.com/forum/?hl=en#!forum/cesium-dev

from 3d-tiles.

conradlempert avatar conradlempert commented on May 19, 2024

I noticed that atan2(y,x) is more inaccurate than atan(y/x). (While atan(y/x) only works for x > 0 of course). In my particular use case I used the atan2 function to compute the longitude of points in the tileset.

The resulting longitude from atan2(y, x) was off by about 0.00001 (ca. 10 meters), while it looked perfect with atan(y/x).

Unfortunately I don't know how to tell you the exact resulting numbers since they are only in the GPU I guess, I can only give this estimate from what it looks like.

My cesium version is 1.63, however this was reproducable in a sandcastle.

Sandcastle:
https://sandcastle.cesium.com/#c=lVX7TyM3EP5XRtFJbErq7CabB081B7SNRAGRcKeTIiFnd0JcHDu1vYSA+N873kcIHHdqfyCsxzPfN283m1daKAcnUmcpTNdwZfSMwRU6NDCQElUDTrTMFlPB4UaJBzRWuDVc66l2IrFwzqcMRglXSqg7DzCQ+DdXqdEwNjpJtBRAR/iLOxRwInTCjRTIJuqBG3gQuCKiI1C4ghO0IluwL7ksmNSS/HyileNCoZnUGvA8UQDkmiEJefogUjT7lWFikEi+aiPTcaES1BsT9VI/mKiCzgmJFt1bvuJf+3RcXAY5R2bkBneo1TVanZkE2czoxcCS2jANom7cijYMRSjMJqiQLY1YCEfZsoynaVDyFo4kWlkHKVonFHdCK3LnnXnCF2g4IxOfjMKjLYP9N+5z4+iLq3bg1QDiOIx6ccy6cRzFnXYUNwp5q9UJOxGLo7DX34uisNMqL+LOXiduhazdi3v9OG63vbieX2ojkNL/HemfyFOq95VwyfxaS1lSd4i022u3+2EnbO/1Or2S4deQtXrdbtiLOu1+e6/f75YXXdbqR+RLv9eJwzDsdyrqsmhl4hhVNl1TwRfCInNzVMEsU4l3a5PbojeK5C7znj6q6s2mOlPe39FyjobSSzGhyQmgbEFGIipYWa/nwjsgIOr2PPgcsvEqp9M+bPRymXhEORJPuA9R2Ni+oSHQr2164k/s+vLb4Pzz+c3ZG81UWD6VeIpLNx9TwU+FdVwlBHlBI0huXl2OhuPhl7Pb4cXvw4vh+Ftl/eI/8qwB+L9mE4Zwr/QK3Jw7+hEW7gUNop6B1OpOuCzFpqTS+g9ycbHMikIDaSrtgEZ9DUuDCeW8UULO9QpJDmudAc08WMQCXzigX4mcsi+1vreVJcw0zd0cX0nfcK2Em5MlV8G6+VhnJc1AWt3wmHauM0l7Ccntmd8uVE/rGcnS064wdyGvahW5H3RD7SlG/2TcYEp9UGb+TEqxtFqk7Osfo37MbrXCSwrnekv7oIJQ2iy4pHKmV2U37WxEwQMm7eDTc1GNy4vbwefR5fnN+OyFPcIvsAO7bzwg4S7sNLx8u9iw8yHC+iOE9f9BePoI4ckj1Os7B9t52hRlQCXwe6iW12JSI+X3CdilW7Zu/vjusT6pHXyI3NpAt36G3fgP2BW+dWuJBFuOYIozeiPs1khSG13jUvIE3/uS99zbyH0DgdNlPyPN4WxGIpq8Cm6jv/8ObjO/1Szt+1Ct+FkWnyiawuylNKdGX3nDT88b9Bc4hhwi2Nr0+s7w5Vwk+UO0Wf1Bvo/qbGNbz7dBnrBqB1YJ+8HLN/LXQa5ULZF8m9QatcNcelzF+ZtYLLVx/okMGGs6XFCWyY/mNEvuiSixtkAAOGxumx6m4gFEevTByw6J5NbSzSyT+Q6d1I4Pm6T/nanU+cvjx1bytVebR8fnhZAxdtik48eWTms55eYd8r8

from 3d-tiles.

lilleyse avatar lilleyse commented on May 19, 2024

See #179 for optimizing large number of conditional expressions.

from 3d-tiles.

lilleyse avatar lilleyse commented on May 19, 2024

See #239 for batching vector images into a texture atlas that's part of the payload

from 3d-tiles.

lilleyse avatar lilleyse commented on May 19, 2024

See #252 for mutables (like GL uniforms)

from 3d-tiles.

lilleyse avatar lilleyse commented on May 19, 2024

See #266 for styling alpha separately

from 3d-tiles.

lilleyse avatar lilleyse commented on May 19, 2024

See #268 for accessing feature ids in the styling language

from 3d-tiles.

lilleyse avatar lilleyse commented on May 19, 2024

See #292 for accessing instanced attributes in the styling language

from 3d-tiles.

lilleyse avatar lilleyse commented on May 19, 2024

More considerations for 3D Tiles Next:

  • Can styles access metadata at different granularities including tileset, tile, and group metadata. We support this in CesiumJS.
  • Do properties need to be fully qualified based on their class id? e.g. ${classId.propertyId}? Or only when there are collisions?
  • Can property textures be styled? Would styling need to happen on the GPU in this case?
  • Should geometry attributes like position, normal, etc be available as built-ins and not just for point clouds?
  • Do styles have access to statistics?
  • Do styles have access to class min/max properties values? Both are needed to do color ramps. Statistics are good for localized color ramps, class min/max is good for global color ramps (think a color ramp from the Mariana trench to Everest)
  • Do styles have access to enums? ${buildingType} === BuildingType.HOSPITAL

from 3d-tiles.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.