Skip to content

Conversation

@tahini
Copy link
Collaborator

@tahini tahini commented Nov 14, 2023

Step by step migration to the Deck.gl library for maps.

@tahini
Copy link
Collaborator Author

tahini commented Nov 14, 2023

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

@tahini
Copy link
Collaborator Author

tahini commented Nov 30, 2023

@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.

@tahini
Copy link
Collaborator Author

tahini commented Nov 30, 2023

@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é.

@davidmurray
Copy link
Collaborator

@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 *-1 ici https://github.com/tahini/transition/blob/deck-gl-pow/packages/transition-frontend/src/components/map/layers/AnimatedArrowPathLayer.tsx#L122

@tahini
Copy link
Collaborator Author

tahini commented Dec 1, 2023

Je confirme que le path est à l'envers (et que les nodes et waypoints ne sont pas encore avec le bon look :p )

Capture d’écran du 2023-12-01 10-35-41

Capture d’écran du 2023-12-01 10-44-06

@tahini tahini force-pushed the deck-gl-pow branch 2 times, most recently from fc128bc to 7c27395 Compare December 22, 2023 17:11
@tahini tahini force-pushed the deck-gl-pow branch 3 times, most recently from 62ad89b to fda3073 Compare January 23, 2024 21:09
@tahini tahini marked this pull request as ready for review February 6, 2024 19:31
@tahini
Copy link
Collaborator Author

tahini commented Feb 6, 2024

@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.",
Copy link
Collaborator

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

Copy link
Collaborator

@greenscientist greenscientist left a 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 = {};
Copy link
Collaborator

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
Copy link
Collaborator

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?

@tahini tahini marked this pull request as draft February 20, 2024 22:29
@tahini
Copy link
Collaborator Author

tahini commented Feb 20, 2024

Retour en draft. J'ai un léger doute sur les performances, pour des grandes agences, je veux faire qq tests avant.

@kaligrafy kaligrafy mentioned this pull request Feb 22, 2024
@tahini
Copy link
Collaborator Author

tahini commented Mar 11, 2024

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.

@tahini tahini marked this pull request as ready for review March 13, 2024 15:02
@tahini tahini force-pushed the deck-gl-pow branch 2 times, most recently from e7352a9 to a68c5fe Compare March 20, 2024 12:30
@tahini tahini mentioned this pull request Mar 22, 2024
@greenscientist
Copy link
Collaborator

@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.
@kaligrafy kaligrafy force-pushed the deck-gl-pow branch 2 times, most recently from 7bfa359 to b00dd38 Compare February 28, 2025 20:12
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.
tahini and others added 4 commits March 1, 2025 14:37
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.
@tahini tahini requested a review from Copilot April 10, 2025 19:45
Copy link
Contributor

Copilot AI left a 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]);

Comment on lines +331 to +341
// 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}`;
Copy link

Copilot AI Apr 10, 2025

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.

Suggested change
// 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}`);

Copilot uses AI. Check for mistakes.
*/
const globalEventDescriptors: MapEventHandlerDescription[] = [
{ type: 'map', eventName: 'mouseout', handler: onMouseOut },
/* { type: 'map', eventName: 'mouseout', handler: onMouseOut },
Copy link

Copilot AI Apr 10, 2025

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.

Copilot uses AI. Check for mistakes.
@tahini
Copy link
Collaborator Author

tahini commented Jul 15, 2025

replaced with #1334

@tahini tahini closed this Jul 15, 2025
GabrielBruno24 pushed a commit to GabrielBruno24/transition that referenced this pull request Dec 10, 2025
fixes chairemobilite#773

This adds the `_interviewer_count` and `_interviewer_created` columns to
the exported interviews' csv file.
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.

5 participants