Deck gl poc map fc #1283
Draft
Deck gl poc map fc #1283
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Working layers using geojson data, events and custom shader Co-authored-by: Wassim27 <mw-guellati@outlook.fr> Co-authored-by: Gabriel Bruno <gabriel.bruno@polymtl.ca> Co-authored-by: Florence Lauer <florence_lauer@outlook.com> Co-authored-by: MohamedAli-M <mohamed-ali.messedi@polymtl.ca> Co-authored-by: mahdiguermache <mahdi.guermache@polymtl.ca> Co-authored-by: nik498 <54679682+nik498@users.noreply.github.com>
Current state: no background, simple geometries are displayed. All enabled layers are displayed, with proper color if that color is in the properties, otherwise, it is a default color. There is no interaction with the layers and the circles are very small when zoomed out. The selected shaders do not work. The original layer descriptions in the layers.config.ts file have not been changed or updated yet. The information they contain may be useful for the Deck.gl data, but we'll need to take each field and see how to define/type it for best Deck.gl support
This removes the need to have a mapbox access token, the .env do not need to add those values anymore and it is removed from documentation.
Add the xyzTileLayer to the main map, using the CUSTOM_RASTER_* environment variables.
For now, only three styles are available: OSM, positron and dark matter from CartoDB. Eventually, we can extend to also use MapTiler's tiles, but his requires an API key.
The type is named `MapLayer` and another type `LayerConfiguration` is used to configure the layer by the application. The configuration is meant to be fixed by the application and does not change, except through Preferences, if necessary.
Performance could be improved, but it works sufficiently well for now
TripsLayer is for animating a fading path, but we don't need that for general transit lines.
* Change the API of the event handlers * Drop support of mapbox types in event handling * Change the events to use the new API * mouseDown/Move/Up are now onDrag and onDragEnd events, which happen only with a specific feature, these are easier * There are simple tooltip events, which simplify popup management when only a simple text is required * The `click` event is separate in `leftClick` and `rightClick` TODO: The `path.hoverNode` and `path.unhoverNode` are not yet functional, as they programatically need to create a popup or menu at a specific location. TODO2: Aesthetic changes are not taken into account, as zoom events (and other layout events) are not yet properly handled TODO3: Not all layers are refreshed, even though the collection they display has elements that have changed. For example, saving a path does not show the updated path. Deck.GL does a shallow comparison to determine if the layer needs to be refreshed and probably the shallow comparison does not show any change if the coordinates changed.
Deck gl does a shallow comparison of the data to determine if the layer needs to be updated. So the data is not refreshed by default if one of the features in the collection changed position. We could add a `dataComparator` function, but this will be executed for every re-render of the map. We instead use an updateCount to save the number of times a layer has been updated throught the `map.updateLayer[s]` event. This will cause the data to be refreshed whenever the count is incremented.
Bring back all mapbox styles for circle-type layers. Type the layer description for circle layers, with fields being either a number/color, a function receiving the feature in parameter, or a property getter to retrieve the value from a geojson property.
Bring back most of mapbox styles for line-type layers. The highlight uses autoHighlight instead of changing the line width. The 'animatedArrowPath' also share a good part of the configuration and re-use the same logic for the PathLayer's data.
Bring back the mapbox styles for the polygon layers. With the lineColor and fillColor, it is not necessary to have both a polygon and a stroke layer, one layer does them all.
Deck.gl is now implemented in mainline Transition and the poc is not necessary anymore.
Add the `featureMinZoom` property, which can be a function that takes the feature in parameter and return the minimum zoom at which this feature should be displayed. Use this property for the transitPaths layer to recover the functionality previously handled by the defaultFilter of mapbox.
Add the `canFilter` layer property. Because we need to know from the start the number of filters to enable for a layer, as it is sent to the shader and not updatable, even with the updateTrigger, this property allows to set a placeholder filter for all features when no other filter is set. See if this can be prevented somehow. The PathMapLayerManager is renamed to TransitPathFilterManager because it does not involve layers, but manages the filters. This class emits the filter update events. The event is caught by the map, which updates the layer accordingly.
What remains that still depended on mapbox are the popup manager, which tracks which popup are currently opened and where, and the polygon draw tool, which is a little known feature of transition and only used for node selection. Some code was commented, other removed, but those features are expected to come back soon, tuned for deck.gl.
Defaults to `true`, but can be set in the user preferences. Layers can then use the `map.enableMapAnimations` preferences value to disable animations.
Add a prop to disable animation on the `AnimatedArrowPathLayer`. This deactivates the animated frame request and arrow path time step, thus saving the GPU from calculations. The previous speed divider of 0 also had the effect of making the path not move, but the layer was still refreshed for every frame, impacting GPU performances.
fixes chairemobilite#974 The `maxRadiusPixels` and `minRadiusPixels` can be set in the layer description to prevent the points from getting too big/small when zooming in/out. If the property is a number, the lineWidth min/max pixels are also set to 1/3 of the corresponding radius.
fixes chairemobilite#966 This moves all map related preferences to a separate section in the preferences edit form. Add a preference called `mapTileLayerOpacity`, which is a value between 0 and 1 representing the percentage opacity of the xyz tile layer. Extract the tile layer getter in the `TransitionMainMap` so it can be reset when the layer opacity is changed. If the opacity is 0 (invisible), the layer is not present.
From the commit title "Deck.gl proof of work" to this one, the deck.gl migration branch may have contained incomplete code with some blocks temporarily commented out. This commit fixes all remaining lint errors after the migration.
fixes chairemobilite#115 Add TextLayer for text on map
* deck.gl: 8.9.36 => 9.0.40 We use the ~9.0.40 to make sure it does not automatically upgrade to 9.1.0, as currently, the path animation does not work with the 9.1.x version of deck.gl
The `transitNodesSelectedErrors` and `transitPathWaypointsErrors` layers are removed. Instead the `transitNodesSelected` and the `transitPathWaypoints` layers' features have an additional boolean property that will be used to specify the strokeColor. This simplifies those layer managements and ensure all waypoints and nodes actions work identically whether the object is in error or not.
The `TransitionMapController` will replace the default controller, so that it is possible to handle events that are not exposed by Deck's default API (for example the `pointermove` event, which will be useful for the polygon selection tool). The controller receives the active event manager, as well as map callbacks to access some methods from the map to pick the objects under the pointer or translate pixel coordinates to geometric coordinates.
When a event handler has successfully run and handled the event, it should return `true` and the caller needs to set the `handled` property of the event to `true` to avoid other handlers running. All event handlers now need to return a boolean value. This prevents the map controller's event handler to run when the event has already been handled by a layer. The propagation is not stopped for dragStart/dragEnd as otherwise the Deck.GL map does not properly handle the dragging state.
This adds the `onLeftDblClick`, `onRightDblClick` and `onPointerMove` events to the map and map select events.
The draw tool is one of the edit tools. It draws a polygon as the user clicks on the map, double-clicking will close the polygon and trigger the `map.draw.polygon` event that receives an object with a `polygons` field containing polygon feature collection. It appears in the main map only when the 'nodes' section is active, for now, as it was this way before.
When a polygon is drawn the nodes in that polygon are selected. The functions that were previously in the `TransitionMainMap` are now moved to the `TransitNodePanel`, closer to where they should be applied. Context menu callbacks are updated accordingly. A new layer has been added in the node section: the `transitNodesSelectedPolygon`, which contain the polygon that was drawn by the user using the DrawPolygon tool.
add config in preferences for both xyz raster tiles and vector tiles + update naming for both config fixes chairemobilite#978
- Fix node selection in TransitNodePanel - Fix path selection in PathSectionMapEvents - Fix map click handling in NodeSectionMapEvents - Fix map click handling in PathSectionMapEvents
In an attempt to reduce the slowness of rendering when panning on some machines, the layer components are updated only when a state it depends on is updated, instead of on every render.
We max at one setState update per 100ms to have a smoother pan experience
When set to `false`, the preferences update will not trigger an event emission on the UI. This can be used for preferences that do not have side-effect in the UI, like the center coordinates of the map.
This function is registered as an event listener when the component is mounted, but its value can change, so we need to keep it in a reference and use the current ref instead.
|
Pour pouvoir être utilisé comme event listener sans problème, il faut utiliser un callback qui ne changera pas et qui utilise des variables/états qui en changerons pas non plus |
There was a problem hiding this comment.
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (2)
packages/transition-frontend/src/services/map/ContextMenuManager.tsx:52
- Ensure th A3D4 at keys provided for React list items are guaranteed to be unique; using the title may lead to duplicate keys if not properly enforced. Consider using a unique identifier for each ContextMenuItem.
key={element.key ? element.key : element.title}
packages/chaire-lib-common/src/config/Preferences.ts:221
- [nitpick] Consider adding inline documentation for the new 'emitChangeEvent' parameter in the update method to clearly describe its purpose and usage, ensuring future maintainability.
if (emitChangeEvent) {
1d34cd4 to
e32b3b4
Compare
|
replaced with other branche |
5b7970d to
9576dc7
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@kaligrafy La conversion au function component ne fonctionne pas correctement. Voir le dernier commit de la chaîne qui montre ce qui doit être fait pour faire fonctionner correctement le fitBounds.
Lorsqu'un
useCallbackouuseMemoest appelé, il "gèle" l'état actuel, donc le viewState est tel qu'il était lorsque ça a été appelé. Lorsqu'on met des fonctions dans les event listener dans le mount du composant, c'est pareil, il gèle l'appel à la fonction qui existait au moment duuseEffect, ce qui fait que dès qu'une de ces fonctions voit sa références modifiées (que ce soit parce qu'elle est recréée à chaque render ou lors d'un recalcul duuseCallback), cette nouvelle fonction ne sera pas celle appelée, mais bien l'originale.Le dernier commit montre comment on peut fixer ça pour le cas du fitBounds, mais une approche similaire devra être faite pour tous les callbacks changeants. Cela me semble un workaround douteux, et sachant cela, l'ensemble semble parsemé de bugs potentiels non encore découverts car on a pas testé de façon exhaustive la carte (genre le flicker du polygone est peut-être dû à qqc du genre).
Bref, tout ceci me laisse penser que notre composant est encore trop complexe pour être une function component, il va falloir le simplifier de plusieurs façons. En attendant, à moins d'avoir une excellente raison de passer à un function component, je proposerais de rester avec le class component et le revoir tranquillement par la suite.