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

Add point cloud styling section #138

Merged
merged 3 commits into from
Jan 10, 2017
Merged

Add point cloud styling section #138

merged 3 commits into from
Jan 10, 2017

Conversation

lilleyse
Copy link
Contributor

@lilleyse lilleyse commented Oct 21, 2016

Added pointSize, ${POSITION}, ${COLOR}, ${NORMAL}

@pjcozzi I'm in the process of writing a section for shader styling. Did we decide on a final solution for the current GLSL styling limitations? I have the current issues in CesiumGS/cesium#3241 under the Point Cloud Shader Styling section:

  • Point Cloud Shader Styling
    • Unsupported:
      • isNan, isFinite(supported in later versions of GLSL with isnan and isinf)
      • Strings, not supported in the GLSL styling engine or the batch table binary
      • Regular expressions
      • Arrays that aren't length 2, 3, or 4.
      • null and undefined - use a default value like 0?
      • type mismatching - comparing floats to bools to vecn will result in shader compilation errors

and some notes here: CesiumGS/cesium#4336 (comment)

I am also going to write a section for the new ColorBlendMode types in CesiumGS/cesium#4451. Technically this is not part of the declarative styling language though, so should it go in the Styling section or the main README ?

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 21, 2016

Did we decide on a final solution for the current GLSL styling limitations?

No. The plan is to continue to work on the implementation and see how impactful these limitations are, then we will be in a better position to decide.

Can you submit an issue to this repo labeled "Draft 1.0" with the above info and then link to it from the Cesium roadmap issue?

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 21, 2016

Technically this is not part of the declarative styling language though, so should it go in the Styling section or the main README ?

The spec is about the spec, not the Cesium implementation. I think the spec should say that the computed color from the style can be applied in an application-specific way, e.g., replace, modulate, mix. We just have to carefully word so that a valid implementation can't just set the color to black and say it conforms.

@@ -469,6 +470,37 @@ ${temperatures.values[0]} === 70
${temperatures['values'][0]} === 70 // Same as (temperatures[values])[0] and temperatures.values[0]
```

### Point Cloud

In addition to evaluating a point's `color` and `show` properties, a point cloud style may evaluate `pointSize`, or the size of each point in pixels. The default `pointSize` is `1.0`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Be more explicit here about what a point cloud is; mention the title type and link to it.

}
```

Implementations may clamp the evaluated `pointSize` to the system's supported point size range. For example, WebGL renderers may query `ALIASED_POINT_SIZE_RANGE` to get the system limits when rendering with `POINTS`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps say that a pointSize of 1.0 must be supported.

Implementations may clamp the evaluated `pointSize` to the system's supported point size range. For example, WebGL renderers may query `ALIASED_POINT_SIZE_RANGE` to get the system limits when rendering with `POINTS`.

Point cloud styles may also reference per-point semantics including position, color, and normal to allow for more flexible styling of the source data.
* `${POSITION}` is an array of three values representing the xyz coordinates of the point before the `RTC_CENTER` and tile transform are applied. When the positions are quantized, `${POSITION}` refers to the position after the `QUANTIZED_VOLUME_SCALE` is applied, but before `QUANTIZED_VOLUME_OFFSET` is applied.
Copy link
Contributor

Choose a reason for hiding this comment

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

Precisely say what coordinate system that is using the same terms in the point cloud spec.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also precisely say the type of each component, e.g., float.


Point cloud styles may also reference per-point semantics including position, color, and normal to allow for more flexible styling of the source data.
* `${POSITION}` is an array of three values representing the xyz coordinates of the point before the `RTC_CENTER` and tile transform are applied. When the positions are quantized, `${POSITION}` refers to the position after the `QUANTIZED_VOLUME_SCALE` is applied, but before `QUANTIZED_VOLUME_OFFSET` is applied.
* `${COLOR}` evaluates to a Color type, where each of the rgba color components are in the range `0.0` to `1.0`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Precisely state component type.

Point cloud styles may also reference per-point semantics including position, color, and normal to allow for more flexible styling of the source data.
* `${POSITION}` is an array of three values representing the xyz coordinates of the point before the `RTC_CENTER` and tile transform are applied. When the positions are quantized, `${POSITION}` refers to the position after the `QUANTIZED_VOLUME_SCALE` is applied, but before `QUANTIZED_VOLUME_OFFSET` is applied.
* `${COLOR}` evaluates to a Color type, where each of the rgba color components are in the range `0.0` to `1.0`.
* `${NORMAL}` is an array of three values representing the normal of the point before the tile transform is applied. When normals are oct-encoded `${NORMAL}` refers to the decoded normal.
Copy link
Contributor

Choose a reason for hiding this comment

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

Precisely state coordinate system and component type.


Implementations may clamp the evaluated `pointSize` to the system's supported point size range. For example, WebGL renderers may query `ALIASED_POINT_SIZE_RANGE` to get the system limits when rendering with `POINTS`.

Point cloud styles may also reference per-point semantics including position, color, and normal to allow for more flexible styling of the source data.
Copy link
Contributor

Choose a reason for hiding this comment

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

"semantics from the Feature Table"

```json
{
"color" : "${COLOR} * color('red')'",
"show" : "${POSITION}[0] > 0.5",
Copy link
Contributor

Choose a reason for hiding this comment

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

Should these be arrays or .xyz and .rgba? I get that the later is more work since we have to introduce a vec3 type to the styling language, but it is more natural.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah vec3/vec4 is more natural and is how a styling shader would implements it, as well as makes more sense in terms of math operations.

@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 23, 2016

Bump. @lilleyse let's not forget about this one.

@lilleyse lilleyse mentioned this pull request Jan 9, 2017
1 task
@lilleyse lilleyse changed the base branch from master to vector January 9, 2017 18:49
@lilleyse lilleyse changed the base branch from vector to master January 9, 2017 18:51
@lilleyse
Copy link
Contributor Author

lilleyse commented Jan 9, 2017

Updated with reference to #166

@pjcozzi
Copy link
Contributor

pjcozzi commented Jan 10, 2017

@lilleyse what else is need here after #166?

@lilleyse
Copy link
Contributor Author

That should be it, unless you see any areas to change.

@lilleyse
Copy link
Contributor Author

The Point Cloud Shader Styling section is still a TODO because we haven't fully fleshed out the rules for typechecking and other things.

@pjcozzi
Copy link
Contributor

pjcozzi commented Jan 10, 2017

The Point Cloud Shader Styling section is still a TODO because we haven't fully fleshed out the rules for typechecking and other things.

Please submit an issue for this labeled 1.0 and styling by flushing out the tasklist and Cesium PR linked to at the top here.

In the meantime, it is OK to merge this with the TODO.

@pjcozzi pjcozzi merged commit 434bd71 into master Jan 10, 2017
@pjcozzi pjcozzi deleted the pnts-styling branch January 10, 2017 17:51
@lilleyse
Copy link
Contributor Author

This issue should have all the current GLSL limitations: #140

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

Successfully merging this pull request may close these issues.

2 participants