Protobuf changes #472
Protobuf changes #472
Conversation
|
|
| "@deck.gl/layers": "^8.1.5", | ||
| "@deck.gl/mesh-layers": "^8.1.5", | ||
| "@deck.gl/react": "^8.1.5", | ||
| "@deck.gl/core": "8.1.8", |
There was a problem hiding this comment.
We should not be using fixed versions. Why is this necessary. Is there a patch version that is required? Otherwise ^ should cover this already.
There was a problem hiding this comment.
I partially agree with you, unfortunately dependencies among the whole viz.gl packages are complex and having fuzzy versions leads to tons of conflicts. Some of them can be solved with resolutions, but a lot of them cannot.
There was a problem hiding this comment.
we've made it this far? Do we have a concise statement of the problem? This does not seem to be a streetscape.gl issue, but an application issue. So the solution should be placed there, not in streetscape.gl
| const XVIZ_TO_LAYER_TYPE = { | ||
| // V1 | ||
| points2d: 'scatterplot', |
There was a problem hiding this comment.
Lets not change the existing style unless there is some reason. When in rome ...
| const {objectStates} = this.props; | ||
| const {layerProps} = this.state; | ||
| const {updateTriggers} = layerProps; | ||
| const { objectStates } = this.props; |
|
|
||
| if (changeFlags.dataChanged) { | ||
| // Pre-process data | ||
| let data = props.data; | ||
| const dataType = this._getLayerType(data); | ||
| type = XVIZ_TO_LAYER_TYPE[dataType]; | ||
|
|
||
| if (type === 'scatterplot' && data[0].vertices && Array.isArray(data[0].vertices[0])) { | ||
| if ( | ||
| !data.vertexBuffer && |
There was a problem hiding this comment.
What is this vertexBuffer? it's related other work but I don't think it exists here yet correct?
| @@ -331,7 +382,10 @@ export default class XVIZLayer extends CompositeLayer { | |||
| length: data[0].points.length / 3, | |||
| attributes: { | |||
| getPosition: data[0].points, | |||
| getColor: data[0].colors | |||
| getColor: { | |||
There was a problem hiding this comment.
This is fixing the difference between rgb and rgba? We should make sure we have a test for this
| @@ -378,6 +432,7 @@ export default class XVIZLayer extends CompositeLayer { | |||
| data, | |||
| lightSettings, | |||
| wireframe: layerProps.stroked, | |||
| extruded: layerProps.stroked, | |||
There was a problem hiding this comment.
This doesn't make sense to me. Do we know where this comes from?
There was a problem hiding this comment.
this could better check if we have a height instead, I will make the change
| // TODO (mauricio): adjust this once we can send mime type from the backend | ||
| new Blob([data[0].data], { type: "image/png" }) | ||
| ), | ||
| bounds: [-1, -1, 1, 1] |
There was a problem hiding this comment.
add comment on this. We need to get the actual width changes added to xviz.
| @@ -400,6 +455,22 @@ export default class XVIZLayer extends CompositeLayer { | |||
| }) | |||
| ); | |||
|
|
|||
| case XVIZ_PRIMITIVE_TYPES.image: | |||
| // TODO (mauricio): images stream is being filtered out, figure out why | |||
| // TODO (mauricio): also figure out wht a box appears on top of the stadium | |||
There was a problem hiding this comment.
Should this comment be next to the stadium case?
| @@ -13395,7 +13417,7 @@ run-queue@^1.0.0, run-queue@^1.0.3: | |||
|
|
|||
| rw@^1.3.3: | |||
| version "1.3.3" | |||
| resolved "https://registry.yarnpkg.com/rw/-/rw-1.3.3.tgz#3f862dfa91ab766b14885ef4d01124bfda074fb4" | |||
| resolved "https://unpm.uberinternal.com/rw/-/rw-1.3.3.tgz#3f862dfa91ab766b14885ef4d01124bfda074fb4" | |||
There was a problem hiding this comment.
Looks like you forgot to change your npmrc. Can't use uber internal repo
| @@ -400,6 +448,21 @@ export default class XVIZLayer extends CompositeLayer { | |||
| }) | |||
| ); | |||
|
|
|||
| case XVIZ_PRIMITIVE_TYPES.image: | |||
There was a problem hiding this comment.
What does this constant buy us? The rest of the code is just using a string. Just curious if i'm missing something.
Mainly this creates a dissonance in the code and prompts the question "why"
| layerProps, | ||
| this.getSubLayerProps({ | ||
| id: XVIZ_PRIMITIVE_TYPES.image, | ||
| image: URL.createObjectURL( |
Adding some changes to add compatibility with protobuf