-
Notifications
You must be signed in to change notification settings - Fork 16
Migrate the main map to Deck.gl #773
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
Conversation
|
Will fix #552 @davidmurray If you want to work on some tasks related to this issue, let me know and you can do PR on https://github.com/tahini/transition/tree/deck-gl-pow branch to get it in this PR |
|
@davidmurray Je suis à faire fonctionner les événements pour créer un nouveau path, et on dirait que ça construit le path à l'envers. Est-moi de mon côté ou serait-il possible que les flèches du path animé soient inversées? Car l'array se fait dans la direction à laquelle je m'attends (nouveau noeud en arrière), mais l'animation me montre l'autre direction. |
|
@davidmurray Pas besoin de fixer le problème, juste me dire si tu es certain de la direction des flèches, sinon, je vais regarder de mon côté. |
|
@tahini Ça se peut, je ne me souviens plus si c'était dans la bonne direction. Quand tu prends le même path (pré deck.gl et post deck.gl), l'animation change de sens? Le code que j'ai push provient à 80% de https://github.com/visgl/deck.gl/tree/master/modules/extensions/src. Ça me surprendrait qu'ils aient fait une erreur. Mais si c'est le cas on pourrait peut-être juste ajouter un |
fc128bc to
7c27395
Compare
62ad89b to
fda3073
Compare
|
@kaligrafy @greenscientist ready for review, see the #552 issue for what was done and what would be left to do as separate and later issues. I think, the minimial viable product for deck.gl support is there. Please test it if you can to make sure I didn't miss anything important. |
| "MapStyle": "Fond de carte", | ||
| "DefaultColor": "Couleur par défaut", | ||
| "DefaultWalkingSpeedKph": "Vitesse de marche par défaut (km/h)", | ||
| "DefaultWalkingSpeedKphHelp": "La vitesse de marche varie selon l'âge et le genre entre 3 km/h (personnes âges et jeunes enfants) et 7 km/h (personnes jeunes et/ou très actives). Les hommes sont en moyenne un peu plus rapides que les femmes et de manière générale, la vitesse de marche diminue avec l'âge. La vitesse d'une personne en chaise roulante manuelle varie entre 2 et 3 km/h selon la force physique.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
personnes âges -> personnes âgées
greenscientist
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't have much comments, a few small things about commented code.
I imagine we have tested the custom tile that we are using in prod?
|
|
||
| onUpdateLayers = () => { | ||
| const layersVisibility = {}; | ||
| /* const layersVisibility = {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Want to keep this commented code?? If so it would need a comment.
| newTransitNode.startEditing(); | ||
| serviceLocator.selectedObjectsManager.select('node', newTransitNode); | ||
| serviceLocator.eventManager.emit('selected.updateAutocompleteNameChoices.node', getRoadLabelAround(map, e)); | ||
| // FIXME Migration to DeckGL: reimplement this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is those FIXME/TODO documented in issues?
|
Retour en draft. J'ai un léger doute sur les performances, pour des grandes agences, je veux faire qq tests avant. |
|
Alors j'ai ajouté [enfin] une option de config pour désactiver les animations de carte. ça va rendre certains utilisateurs heureux et permettre aux gens comme moi avec des GPU moins performants de ne pas être trop demandant. |
e7352a9 to
a68c5fe
Compare
|
@tahini So we don't forget, can you comment here on that is the current blocker for this PR |
fixes chairemobilite#975 fixes chairemobilite#1233 When a path is selected, the fitBounds function creates a mercator viewport to get the proper view state with latitude/longitude and zoom for the requested bounding box.
This ensure that the visibility of the layers is correctly managed and only visible layers are displayed. The `enabledLayers` map state is replaced by a `visibleLayers` state, as it is used only to trigger re-rendering when the layers and layer visibilities are updated.
`isochronePolygons`, `transitStations` and `transitStationSelected` layers are not used anymore, so they are removed from the layers configuration file, as well as from the default preferences. Also, the path stroke layers have been removed and, if necessary, can be replaced with some of deck.gl's feature directly in the main corresponding layer.
* Deck.gl and other packages: ~9.0.40 -> ^9.1.0 * Mjolnir.js: 2.7.3 -> 3.0.0 Fix the passing of the uniform values in the `AnimatedArrowPathLayer` class to set the props instead and use Deck.gl's new API for uniform buffers.
It is not used anymore and the `MapPolygonService` will be rewritten from scratch.
The migration to deck.gl changed the format of the colors to the more universal hexadecimal rgba representation (starting with #). Previous color preferences were in the format of a rgba function (for example `rgba(255,0,0,0.5)`). This old format is automatically converted to hex string when we get a preferences that has the name color and the rgba format. This commit can be reverted after a while, when all current deck.gl users have changed their preferences.
The accessibility map and routing colors can now be defined in the user preferences in the frontend.
This allows to add this feature to any layer type, like a geojson one, instead of only path layers.
Add a `stroked` property to the `BaseLineLayerConfiguration` type. Defaults to `false`. Change the return type of the `getLayer` to an array, as the outline will be an additional layer. If the `stroked` is true, it adds an additional line layer that is 2 pixels wider than the main layer and of white color.
The measure tool will display a crosshair cursor, dragging an element will show a 'grabbing' cursor, hovering will show a 'pointer' and the default is the 'grab' hand. Also move the pickable to a common property of layers.
This class is created in the map constructor and handles the events by type and layer. The map can call the event manager to execute the events at different times. Extract the `condition` field of the events in a Base event handler type, to avoid repeating this field.
Add a `MapEditFeature` interface that any tool that interacts with the map, but not with the transit objects can implement to provide layers and events independently, in a standalone way. Let the measure tool implement this interface instead.
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.
7bfa359 to
b00dd38
Compare
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 60 out of 67 changed files in this pull request and generated 2 comments.
Files not reviewed (7)
- .env.docker: Language not supported
- .env.example: Language not supported
- locales/en/main.json: Language not supported
- locales/en/transit.json: Language not supported
- locales/fr/main.json: Language not supported
- locales/fr/transit.json: Language not supported
- packages/chaire-lib-frontend/package.json: Language not supported
Comments suppressed due to low confidence (2)
packages/chaire-lib-frontend/src/services/map/MapLayerManager.ts:113
- The updateLayer method no longer supports a function as the geojson parameter as it did originally. Confirm whether backward compatibility is needed or update the type and calling code accordingly.
if (this._layersByName[layerName] !== undefined) {
packages/chaire-lib-frontend/src/services/map/MapLayerManager.ts:144
- [nitpick] Filtering enabled layers solely based on non-empty features may inadvertently hide layers that are meant to be visible but are temporarily empty. Verify that this behavior is intended.
return this._enabledLayers.filter((layerName) => this._layersByName[layerName].layerData.features.length > 0).map((layerName) => this._layersByName[layerName]);
| // Convert rgba to hex | ||
| const rgba = value.split(','); | ||
| const r = parseInt(rgba[0].split('(')[1], 10); | ||
| const g = parseInt(rgba[1], 10); | ||
| const b = parseInt(rgba[2], 10); | ||
| const a = parseFloat(rgba[3].split(')')[0]); | ||
| const hex = `#${((1 << 24) + (r << 16) + (g << 8) + b).toString(16).slice(1)}`; | ||
| const alphaHex = Math.round(a * 255) | ||
| .toString(16) | ||
| .padStart(2, '0'); | ||
| return `${hex}${alphaHex}`; |
Copilot
AI
Apr 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The rgba-to-hex conversion assumes a very specific string format. Consider making the parsing logic more robust or validating the format before conversion to prevent potential parsing errors.
| // Convert rgba to hex | |
| const rgba = value.split(','); | |
| const r = parseInt(rgba[0].split('(')[1], 10); | |
| const g = parseInt(rgba[1], 10); | |
| const b = parseInt(rgba[2], 10); | |
| const a = parseFloat(rgba[3].split(')')[0]); | |
| const hex = `#${((1 << 24) + (r << 16) + (g << 8) + b).toString(16).slice(1)}`; | |
| const alphaHex = Math.round(a * 255) | |
| .toString(16) | |
| .padStart(2, '0'); | |
| return `${hex}${alphaHex}`; | |
| // Validate and extract rgba components using regex | |
| const rgbaMatch = value.match(/^rgba\(\s*(\d{1,3})\s*,\s*(\d{1,3})\s*,\s*(\d{1,3})\s*,\s*(\d*\.?\d+)\s*\)$/); | |
| if (rgbaMatch) { | |
| const r = parseInt(rgbaMatch[1], 10); | |
| const g = parseInt(rgbaMatch[2], 10); | |
| const b = parseInt(rgbaMatch[3], 10); | |
| const a = parseFloat(rgbaMatch[4]); | |
| // Ensure values are within valid ranges | |
| if (r >= 0 && r <= 255 && g >= 0 && g <= 255 && b >= 0 && b <= 255 && a >= 0 && a <= 1) { | |
| const hex = `#${((1 << 24) + (r << 16) + (g << 8) + b).toString(16).slice(1)}`; | |
| const alphaHex = Math.round(a * 255) | |
| .toString(16) | |
| .padStart(2, '0'); | |
| return `${hex}${alphaHex}`; | |
| } | |
| } | |
| // If validation fails, return the original value | |
| console.warn(`Invalid rgba format: ${value}`); |
| */ | ||
| const globalEventDescriptors: MapEventHandlerDescription[] = [ | ||
| { type: 'map', eventName: 'mouseout', handler: onMouseOut }, | ||
| /* { type: 'map', eventName: 'mouseout', handler: onMouseOut }, |
Copilot
AI
Apr 10, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The commented-out event handlers may lead to confusion. If these handlers are no longer required in the Deck.gl migration, it is recommended to remove them entirely to keep the codebase clean.
|
replaced with #1334 |
fixes chairemobilite#773 This adds the `_interviewer_count` and `_interviewer_created` columns to the exported interviews' csv file.
Step by step migration to the Deck.gl library for maps.