Mapstate update #217
Mapstate update #217
Conversation
|
@lucas-angermann thanks for all the work! I have a few comments on this. |
CHANGELOG.md
Outdated
| @@ -1,3 +1,14 @@ | |||
| # [12.x.x](https://github.com/dlr-eoc/ukis-frontend-libraries/tree/v12.0.0) (2024-02-07) (services-map-state, map-ol, map-cesium and map-maplibre) | |||
There was a problem hiding this comment.
You can actually omit the version, I always add it when we create a new version
| } | ||
| resetViewAngle() { | ||
| /** set map rotation with the MapStateService */ | ||
| this.mapStateSvc.setViewAngle(0.01); |
There was a problem hiding this comment.
What are the low values "0.0.1" good for?
There was a problem hiding this comment.
When setting the value to 0, there seems to be an rounding issue with the conversions. So it is more stable to use very small numbers instead of 0. Internally, small numbers close to 0 are then snapped to zero (https://openlayers.org/en/latest/apidoc/module-ol_View-View.html):
The rotation constraint snaps to specific angles. It is determined by the following options: enableRotation and constrainRotation. By default rotation is allowed and its value is snapped to zero when approaching the horizontal.
There was a problem hiding this comment.
Then I would handle that internally, but leave the interface as a user would expect.
e.g.
let radAngle;
if(viewAngle === 0){
radAngle = viewAngle;
}else{
radAngle = CesiumMath.toRadians(viewAngle-90)
}
- https://github.com/CesiumGS/cesium/blob/1.114/packages/engine/Source/Core/Math.js#L441
- https://github.com/CesiumGS/cesium/blob/1.114/packages/engine/Source/Core/defined.js
and OpenLayers
| @@ -224,38 +224,41 @@ export class MapCesiumService { | |||
| } | |||
|
|
|||
| // Set initial oblique view, see https://cesium.com/learn/cesiumjs/ref-doc/Camera.html | |||
| //subtract 90°, to get the same behavior as in openlayers | |||
| public setViewAngle(viewAngle: number) { | |||
There was a problem hiding this comment.
Here, I would allow the flyTo options to be passed and merged. I implemented this in one of our projects so that users don't have to set a timeout and can listen to flyTo.complete instead.
/**
* Set initial oblique view, see https://cesium.com/learn/cesiumjs/ref-doc/Camera.html
* subtract 90°, to get the same behavior as in openlayers
* options of viewer.camera.flyTo`
*/
public setViewAngle(viewAngle: number, options?: any`) {
const flyToOptions = Object.assign({
destination: this.viewer.camera.position,
orientation: {
heading: this.viewer.camera.heading,
pitch: CesiumMath.toRadians(viewAngle-90),
roll: this.viewer.camera.roll,
});
}, options || {});
this.viewer.camera.flyTo(flyToOptions)
}
| return CesiumMath.toDegrees(this.viewer.camera.pitch) + 90; | ||
| } | ||
| // subtract rotation degree from 360° to get the same behavior as in openlayers | ||
| public setRotation(rotation: number) { |
There was a problem hiding this comment.
Here I would also allow the user to listen to flyTo.complete.
| @@ -59,6 +59,28 @@ export class MapStateService { | |||
| this.setMapState(state); | |||
| } | |||
|
|
|||
| public setViewAngle(angle: number, notifier: IMapState['options']['notifier'] = 'user') { | |||
| if (!angle) { | |||
There was a problem hiding this comment.
In here, I would allow to pass 0 as an angle. Otherwise, users must use small values to reset the ViewAngle.
| } | ||
|
|
||
| public setRotation(rotation: number, notifier: IMapState['options']['notifier'] = 'user') { | ||
| if (!rotation) { |
There was a problem hiding this comment.
In here, I would allow to pass 0 as rotation. Otherwise, users must use small values.
…ernals - change before release
|
@lucas-angermann I tried to resolve my comments, maybe you can check if this is all correct? |
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
For 3D applications it would be useful to add view rotation and view angle to the mapState. This would enable us to set initial values and change them according to user input.
Issue Number: #216
What is the new behavior?
The mapState now contains the values rotation and viewAngle. To use this functionality, map-ol, map-cesium and map-maplibre were connected to the new mapState values. In demo-maps some examples were added.
Does this PR introduce a breaking change?
Other information
Is it part of one/more packages and which?