Skip to content

Commit

Permalink
Update Viewport RFCs (visgl#1301)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ibgreen
ibgreen authored Jan 8, 2018
1 parent 29fb3e1 commit ce3fbcf
Show file tree
Hide file tree
Showing 11 changed files with 513 additions and 298 deletions.
46 changes: 27 additions & 19 deletions dev-docs/RFCs/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ Implementation of non-trivial new deck.gl features should typically be started o
| **Pre-Approved** | No major initial objections, draft pre-approved for prototyping |
| **Review** | Ready for formal review |
| **Approved** | Approved, ready for implementation |
| **Implemented** | Approved and implemented |
| **Experimental** | Approved, implemented as experimental API |
| **Implemented** | Approved and implemented (as officially supported API) |
| **Deferred** | Review uncovered reasons not to proceed at this time |
| **Rejected** | Review uncovered reasons not to proceed |

Expand All @@ -31,27 +32,36 @@ These are early ideas not yet associated with any release
| [**Projection Mode Improvements**](vNext/projection-mode-improvements-rfc.md) | @ibgreen @pessimistress | **Draft** | Improvements to projection system |
| [**Composite Layer Customization**](vNext/composite-layer-customization-rfc.md) | @ibgreen | **Draft** | Improvements to customizing composite layers |
| [**Shader Module Injection**](vNext/shader-module-injection-rfc.md) | @ibgreen | **Draft** | |
| | | | |
| **Effects** | | | |
| **[Effects Manager](v6.0/effects-manager-rfc.md)** | @1chandu @ibgreen | Draft | Official support for effects (shadows, reflections, better lighting, postprocessing, framebuffer composition etc). |
| **[Render Layer to Texture](v6.0/render-layer-to-texture-rfc.md)** | TBD | Proposed | Allow layers to render to texture and then use texture in subsequent layers. |


## v6.0 RFCs

Current direction for [deck.gl v6.0](https://github.com/uber/deck.gl/projects/3) is to focus on **animation** and **visual effects**, so we want to prioritize related RFCs. In particular, the topic of animation is big, and it has been broken down into a number of separete RFCs that should all move us in the right direction.
Current direction for [deck.gl v6.0](https://github.com/uber/deck.gl/projects/3) is to focus on **animation** and TBA...

So we want to prioritize related RFCs. In particular, the topic of animation is big, and it has been broken down into a number of separete RFCs that should all move us in the right direction.

Also see luma.gl RFCs

| RFC | Author | Status | Description |
| --- | --- | --- | --- |
| **Animation** | | | |
| [**PropTypes**](v6.0/prop-types-rfc.md) | ? | Draft | Validate e.g ranges for numeric attributes, support animation/auto-interpolation. |
| [**Property Animation**](v6.0/property-animation-rfc.md) | @ibgreen | Draft | Allow Layer props and GL parameters to accept functions in addition to values and call these on every render to update values |
| [**Attribute Animation**](v6.0/attribute-animation-rfc.md) | @pessimistress | Proposed | Automatically interpolate between two copies of a vertex attributes |
| [**PropTypes**](v6.0/prop-types-rfc.md) | ? | Draft | Validate e.g ranges for numeric attributes, support animation/auto-interpolation. |
| [**Expose Layer AttributeManager**](v6.0/expose-attribute-manager.md) | @ibgreen | **Review** | simplifies pregenerating attributes in apps for fast animation. |
| | | | |
| **Effects** | | | |
| **[Effects Manager](v6.0/effects-manager-rfc.md)** | @1chandu @ibgreen | Draft | Official support for effects (shadows, reflections, better lighting, postprocessing, framebuffer composition etc). |
| **[Render Layer to Texture](v6.0/render-layer-to-texture-rfc.md)** | TBD | Proposed | Allow layers to render to texture and then use texture in subsequent layers. |
| | | | |
| **Ease-of-Use** | | | |
| [**dataUrl**](v6.0/data-url-rfc.md) | @pessimistress & @ibgreen | Draft | **Ease-of-Use** Allow deck.gl layers to specify a URL and asynchronously download the resulting data |
| | | | |
| **Finalize Multi-Viewport API** | | | |
| [**New View Class**](v6.0/view-class-rfc.md) | @ibgreen | **Draft** | Proposed Official API for multi-view(port) support, replacing the v5.0 experimental API |
| [**Per-View Controllers**](v6.0/per-view-controllers-rfc.md) | @ibgreen | **Draft** | Support one controller per view in multi-view apps |
| [**Unified ViewState**](v6.0/view-state-rfc.md) | @ibgreen | **Draft** | Highly controversial proposal for an even more Unified View/Controller Architecture. Will likely be deferred. Review again after other related RFCs have been approved/implemented |
| | | | |
| **Internals** | | | |
| [**Reduce Distribution Size**](v6.0/reduce-distribution-size-rfc.md) | @ibgreen | **Review** | **Hygiene** Reduce size of distribution and the bundle size of apps consuming deck.gl |
| [**Reduce Repository Size**](v6.0/reduce-repo-size-rfc.md) | @ibgreen | **Draft** | **Hygiene** Reduce size of deck.gl github repository |
Expand All @@ -62,39 +72,37 @@ Possible other animation related RFCs:

## v5.0 RFCs

These RFCs are being implemented (fully or partially) in v5.0.
These RFCs were implemented in v5.0. Also see luma.gl RFCs.

| RFC | Author | Status | Description |
| --- | --- | --- | --- |
| **Animation** | | | |
| [**Auto Highlighting**](v5.0/auto-highlighting-rfc.md) | @ibgreen @1chandu | **Implemented** | Auto highlight hovered object via `picking` module |
| [**Viewport interpolation**](v5.0/viewport-animation-rfc.md) | @1chandu | Proposed | This should build on the new Viewport system in the First Person RFC. Also needs to consider react-map-gl. |
| [**Viewport interpolation**](v5.0/viewport-animation-rfc.md) | @1chandu | **Experimental** | This should build on the new Viewport system in the First Person RFC. Also needs to consider react-map-gl. |
| | | | |
| **Viewports and Controllers** | | | |
| [**First Person Viewport**](v5.0/first-person-mercator-viewport-rfc.md) | @ibgreen | **Pre-Approved** | Geospatially enable all viewports |
| [**Multiple Viewports**](v5.0/multi-viewport-rfc.md) | @ibgreen | **Pre-Approved** | Supoort for multiple viewports, synchronized or unsynchronized |
| [**Controller Architecture**](v5.0/controller-architecture-rfc.md) | @ibgreen | **Draft** | Generalize and Freeze experimental Controller Architecture from v4.1 |
| [**First Person Geospatial Viewport**](v5.0/first-person-mercator-viewport-rfc.md) | @ibgreen | **Experimental** | Geospatially enable all viewports, add FirstPersonViewport for hybrid apps. |
| [**Multiple Viewports**](v5.0/multi-viewport-rfc.md) | @ibgreen | **Experimental** | Support for multiple viewports |
| | | | |
| **General** | | | |
| [**Break out EventManager**](v5.0/break-out-event-manager-rfc.md) | @ibgreen | **Draft** | **Hygiene** Break out shared event manager code |
| [**Break out Math Module**](v5.0/break-out-math-module-rfc.md) | @ibgreen | **Draft** | **Hygiene** Break out luma.gl math module |
| [**Break out EventManager**](v5.0/break-out-event-manager-rfc.md) | @ibgreen | **Implemented** | **Hygiene** Break out event manager module |


## v4.1 RFCs

These RFCs that have been implemented (fully or partially) in v4.1.
These RFCs were implemented in v4.1.

| RFC | Author | Status | Description |
| --- | --- | --- | --- |
| [**Picking Improvements**](v4.1/picking-improvements-rfc.md) | @shaojingli | "Direction" RFC | Outlines a number of improvements to picking |
| [**Event Handling**](v4.1/event-handling-rfc.md) | Many authors | **Approved** & Implemented | Attempt to define enduring event handling architecture |
| [**Event Handling**](v4.1/event-handling-rfc.md) | Many authors | **Implemented** | Attempt to define enduring event handling architecture |


## v4.0 RFCs

These RFCs that have been implemented (fully or partially) in v4.0.
These RFCs were implemented in v4.0.

| RFC | Author | Status | Description |
| --- | --- | --- | --- |
| [**Non-Geospatial Viewports**](v4.0/non-geospatial-viewports-rfc.md) | @ibgreen @gnavvy | **Approved** & Implemented | Support for non-geospatial viewports |
| [**Composite Layer Prop Forwarding**](v4.0/composite-layer-prop-forwarding-rfc.md) | @shaojingli | **Approved** & Implemented | Conventions for prop forwarding |
| [**Non-Geospatial Viewports**](v4.0/non-geospatial-viewports-rfc.md) | @ibgreen @gnavvy | **Implemented** | Support for non-geospatial viewports |
| [**Composite Layer Prop Forwarding**](v4.0/composite-layer-prop-forwarding-rfc.md) | @shaojingli | **Implemented** | Conventions for prop forwarding |
2 changes: 1 addition & 1 deletion dev-docs/RFCs/v5.0/auto-highlighting-rfc.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

* **Authors**: Ravi Akkenapally and Ib Green
* **Date**: August 9, 2017
* **Status**: **Approved and Implementd**
* **Status**: **Implemented**

Notes:
* Please add comments as reviews to the [PR](https://github.com/uber/deck.gl/pull/839)
Expand Down
4 changes: 2 additions & 2 deletions dev-docs/RFCs/v5.0/break-out-event-manager-rfc.md
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
# RFC: Break out Event Manager

* **Author**: Ib Green & ?
* **Author**: Ib Green & Xiaoji Chen
* **Date**: Aug, 2017
* **Status**: Draft
* **Status**: **Imlemented**


## Motivation
Expand Down
31 changes: 0 additions & 31 deletions dev-docs/RFCs/v5.0/break-out-math-module-rfc.md
View file

This file was deleted.

49 changes: 27 additions & 22 deletions ...5.0/first-person-mercator-viewport-rfc.md → ...0/first-person-geospatial-viewport-rfc.md
View file
Original file line number Diff line number Diff line change
@@ -1,15 +1,22 @@
# RFC - First Person Viewport
# RFC - First Person Geospatial Viewport

* **Author**: Ib Green
* **Date**: August 10, 2017
* **Status**: **Pre-Approved**

Notes:
* Please add comments as reviews to the [PR](https://github.com/uber/deck.gl/pull/838)
* **Status**: **Implemented**

References:
* Controller Changes [Controller Architecture RFC](./controller-architecture-rfc.md)
* The initial [Infovis Viewport RFC](../v4.0/non-geospatial-viewports-rfc.md) for deck.gl v4.
* Controller Changes [Controller Architecture RFC](./controller-architecture-rfc.md)
* Initial review in [PR](https://github.com/uber/deck.gl/pull/838)


## Overview

This RFC proposes:
* An "extended" Viewport hierarchy that adds subclasses based on "differences in view matrix" in addition to differences in projection matrix.
* The new hierarchy includes a "FirstPersonViewport" class that allows the application to specify eye position directly.
* All Viewports are "geospatially enabled", i.e. they can take a lng/lat anchor in addition to their normal coordinates.
* Viewports support the same options as layers, including model matrices so that apps can use their data coordinates to place FirstPersonViewports.


## Motivation
Expand All @@ -36,19 +43,20 @@ However, to synchronize our 3D data with external perspective-enabled map system
The map system typically locks in FoV (Field of view), viewing angle (always pitched downward) and altitude (camera height over the “ground”).
When rendering 3D environments on top of pre-rendered video (e.g. overlaying perception data on top of vehicle log cameras)

Note: There are subtleties around the positioning of the camera which is handled in the next proposal.
## Proposed Features


## Proposal Part 1: Geospatially Enable all Viewports
### Proposed: Geospatially Enable all Viewports

In deck.gl 4.1 only the WebMercatorViewport can handle layers with geospatial coordinates: it is the only viewport that can produce the uniforms required by the project module. But moving some properties from the `WebMercatorViewport` class into the base viewport, it is possible to extend cartographic projection to all viewpott.

The key insight is that through the addition of the "meters mode", we have already addedthe general capability of overlaying an arbitrary linear coordinate system (which is what all non-geospatial viewports use) on top of a geospatial coordinate system using an anchor point.

While METER_OFFSETS mode was initially introduced to solve a narrow use-case, the technique is actually very general, and there is no reason why we would not support geospatial anchoring and linear coordinates for all viewports (in addition to layers), including first person and orbit (i.e. third person) viewports.

Note: There are subtleties around the positioning of the camera which is handled in the next proposal.


## Proposal Part 2: An Alternative Viewport Hierarchy
### Proposed: An Alternative Viewport Hierarchy

In deck.gl v4.0, a viewport class hierarchy was introduced to support non-geospatial viewports. It separates between `Perspective` and `Orthographic` cameras (inspired by common WebGL frameworks).

Expand All @@ -59,12 +67,11 @@ As an alternative to the `Perspective`/`Orthographic` camera class, this RFC rec
Also, since the majority viewports will be used with a perspective projection matrix, if the `projectionMatrix` prop is not supplied, `Viewport` will try to create a perspective projection matrix using new `fov`, `near` and `far` props (note that `aspect` is automatically calculated from `width` and `height`).


## Summary of Proposed Changes
## Proposed API Changes

### Viewport
### New `Viewport` Properties and Methods

New properties:

- `longitude` - (optional) anchor
- `latitude` - (optional) anchor
- `zoom` scale - This will be hardcoded to meter = unity scale by `FirstPersonViewport`
Expand All @@ -76,27 +83,25 @@ New properties:
- `far`

New Methods:

* `Viewport.isGeospatial()` can be called to check if a viewport is geospatially "enabled". If longitude, latitude and zoom are supplied to a viewport, then that viewport is considered geospatial.
* `Viewport.isMapSynched()` offers an easy way for the app to determine if a base map can be displayed under the viewport.
* `Viewport.getMercatorParameters()` offers a way to get map props that include offsets etc.


Remarks:
* `modelMatrix` - A convenience to make the viewport API more similar to the `Layer` props. When positioning the camera in a scene with `Layer`s using a certain `modelMatrix` it is nice to be able to use the same coordinates and the same `modelMatrix` with the viewport.
* `projectionMatrix` - When supplied, the projectionMatrix parameter allows for complete application control of all **projection** matrix parameters - including field-of-view, near/far clipping planes, aspect ratios etc, e.g. using `new Matrix4().projection(...)` or `new Matrix4().ortho(...)` etc..


### FirstPersonViewport
### Proposed: New `FirstPersonViewport` Class

Extends `Viewport`. Creates a `Viewport` with a view matrix that is placed in the player's position, with a controllable direction and orientation:
- `direction` (`Vector3`) - player direction
- `up` (`Vector3`, `[0, 0, 1]`): specifies the camera up direction


### ThirdPersonViewport
### Proposed: New `ThirdPersonViewport` Class

This prototyping behind this proposal has so far not focused on `ThirdPersonViewport`. It is expected to extend `Viewport`, possibly with props such as:
`ThirdPersonViewport` is expected to extend `Viewport`, possibly with props such as:
- player `direction`
- camera `direction`, relative to player direction (additive to player `direction`)
- camera `distance,` from player
Expand All @@ -106,7 +111,7 @@ The idea here with two directions being that one might want a third person camer
See comments under `OrbitViewport` below.


### WebMercatorViewport
### Proposed: `WebMercatorViewport` Changes

Creates a viewport with a special perspective projection matrix with a FOV that works with mapbox-gl, and a view matrix that follows mapbox-gl's undocumented internal bearing/pitch/altitude conventions.

Expand All @@ -122,9 +127,9 @@ inherits from `Viewport`, setting parameters as follows

`isMapSynched()` would return false for a `Viewport` (Note: not a `Layer`):
* If `position` is specified
* If `modelMatrix is specified
* If `altitude is !== 0.5
* If `pitch > 60 degrees
* If `modelMatrix` is specified
* If `altitude` is !== 0.5
* If `pitch` > 60 degrees

We could also check zoom levels etc, potentially moving all mapbox limit checks into the `WebMercatorViewport` viewport.

Expand Down
Loading

0 comments on commit ce3fbcf

Please sign in to comment.