[FE-8035] Upgrade Mapbox #373
Conversation
mapbox isn't meant to be in a node environment, and therefore fails when we run unit tests
|
We're going to need to produce un-premultiplied images from the BE, otherwise immerse will be limited to opaque colors only, and there can certainly be scenarios where that is unwanted. Here's the BE ticket: https://jira.omnisci.com/browse/BE-3956 |
| @@ -2,7 +2,6 @@ import d3 from "d3" | |||
| import * as _ from "lodash" | |||
| import { redrawAllAsync, resetRedrawStack } from "../core/core-async" | |||
| import { utils } from "../utils/utils" | |||
| import { mapDrawMixin } from "./map-draw-mixin" | |||
There was a problem hiding this comment.
Thanks for removing this.
|
If light-v9 is the only option that is comparable to light-v8, we can go with it. Seems like all functionalities are working fine based on my testing. Slight opacity difference in general but assuming we are ok with that. Only thing needed to get fixed is the slider value should reflect the current opacity for the multilayers. |
|
One more thing @CaitW, is your immerse build is up to date? Does it include the raster resize fix that REMOVES |
| @@ -760,6 +760,9 @@ function genLayeredVega(chart) { | |||
| const vegaSpec = { | |||
| width: Math.round(width), | |||
| height: Math.round(height), | |||
| viewRenderOptions: { | |||
There was a problem hiding this comment.
Is this the trick BE needs?
|
Looks like there is outstanding PR in mapbox.gl mapbox/mapbox-gl-js#8693 to add originalEvent for the NavigationControl buttons. We can probably use that to call UpdateDraw on the +/- zoom button to immediately update the shapes. |
Immerse PR
Mapbox Upgrade
Initial investigation notes + reasoning for the upgrade
TLDR: we forked Mapbox a long time ago and never updated it. We'd like to use the current version of Mapbox and not rely on a fork.
This affects all map charts and scatterplot.
Major Changes
Adjusting for premultiplied alpha
Rendering returns premultiplied images.
As an intermediate solution, I changed things so that we request a fully-opaque image from core and then apply the transparency client-side. This seems to create a chart that most closely resembles the given color scale.I'm don't know enough about this subject to know whether this is the best solution. @vastcharade suggested potentially requesting non-premultiplied images from core. See relevant BE IssueBackend pushed a fix for this, so all I did was revert the changes I noted in
strikethroughabove and add this to thegenLayedVegacall:Additional changes:
mapDrawMixinbecause it doesn't appear to be usedmapbox-ported-functions.jsArbitrary decisions up for debate:
light-v8tolight-v9(alters the basemap)axiosfor getting an image