[RFC] Spatial Filter about carto-react HOT 11 OPEN

Hi @aaranadev, thx for the proposal.

@bbecquet did a first review and we checked some aspects of it together, to see technical feasibility. We think expanding the Spatial Filter options make sense, at least partially, but we still have some questions to ask, to clarify RFC.

First some technical comments, then questions.

Technical feasibility

• State:

  • the current storage model should be easy to adapt. The mask geometry is GeoJSON. It could be turned into a Multipolygon or a full FeatureCollection if needed
  • things to add: actions to add/remove/edit by id a specific polygon

• deck.gl:

  • SolidPolygonLayer (used by MaskLayer) should work out-of-the-box with multiple polygons
  • deck.gl MaskExtension should be able to manage masking by arbitrary number of geometries from the MaskLayer (confirmed with @felixpalmer)

• UX/UI

  • Adaptation of FeatureSelectionWidgetUI to support multiple areas. Potentially the most complex piece.
  • “The widget must allow the show of various geometries together and if the union between them is inclusive or exclusive.” Here it's where we start having doubts about scope & complexity... see our questions in the next block, and let's call this "multiple polygons mode"

Questions

1. How core is the "multiple polygons mode" in the proposal? Is it useful to implement only one mode to calculate mask?

2. We would like to have a better definition of 'inclusive' vs 'exclusive' modes. Let's assume user draws 3 polygons, polygon A, polygon B and polygon C. So that means...(please confirm / elaborate more here)
   2.a. INCLUSIVE: The spatial filter would be an OR, and the filter would include the points inside polygon A or polygon B or polygon C
   2.b. EXCLUSIVE: The spatial filter would include points inside polygon A, and not polygon B, nor polygon C... ??

3. If we understood this correctly, we think that implementing only INCLUSIVE mode feels more clear, and less risky technically, and it would fit in (we would approve that, to go ahead with implementation plan). But it looks like EXCLUSIVE mode would require more elaboration and explanations before moving on, and we're not sure about it (probably it would include tricky cases, like holes, overlaps, self-crossing features...).

cc/ @moimart

from carto-react.

Hi @VictorVelarde , thx for the answer.

With my POC, I've been able to check the technical questions and State and deck.gl are working ok. About UX/UI, I think that is the more complex piece too.

Questions

1. I think both modes are important.

2. So that means... you can only use one mode simultaneously in a simple mode.
   2.a. INCLUSIVE is an AND, the idea is that you can generate a multipolygon with all the geometries and cross them all.
   2.b. EXCLUSIVE is an OR, the idea is that you can generate an array of polygons and source intercept them all separately, generating an array of values.

3. We need both modes.

If the inclusive/exclusive switch is too complex to add in the UX/UI of FeatureSelectionWidgetUI, we can only allow INCLUSIVE mode on the widget for now, but the exclusive mode has to be in the spatial filter logic.

cc: @efernandezleon

from carto-react.

Hi all,

thanks for the proposal, this looks like a great feature. Some thoughts below:

Performance

The more complex the mask gets the slower widgets will run. For points the performance is generally OK, but for lines it is slower and for polygons even slower. As a rough rule of thumb, the runtime will scale linearly with the number of vertices the mask polygon has. This performance hit comes from C4R not deck.gl.

A way to improve this would be to not store circular masks as polygons, but rather as points with a radius. Rendering this with deck is straightforward via a GeoJSONLayer.

Separately, from a UX point of view, it may be worth adding a limit on the mask complexity - or at least a warning.

exclusive/inclusive modes

Regarding exclusive/inclusive: the naming seems unclear to me. Instead I think of it as:

• Normal mode: each shape in the mask is equal and any point that is contained in any shape passes the filter
• Intersect mode: each shape in the mask is separate, and the final mask polygon is created by the region in which ALL of the shapes overlap

From a deck.gl perspective, the Intersect mode is not supported and the way to implement it would be to compute the overlapping region in C4R prior to passing this off to deck.gl. Agree with @VictorVelarde that this mode is more complex & risky to implement

from carto-react.

Ok, now I think I understand better your proposal @aaranadev. I think you want a couple of quite different things…
Let me rephrase those to validate we’re all on the same page:

• (a) SINGLE MASK: A way to draw one multipolygon for a single mask. Exactly the same behaviour as current one but with a more complex geometry (I’m considering just one multipolygon, and explicitly excluding here drawing holes). vs

• (b) MULTIPLE MASKS: A way to draw several spatial masks, so you have a collection of independent filters, probably also with an associated ID. I think here we all misunderstood it at the beginning and we automatically started to think about “different ways to draw a complex geometry” (but in all cases keeping the idea of a SINGLE spatial filter). This has many consequences, not only in the UI side.

(a) SINGLE MASK. I think that both from [deck.gl](http://deck.gl) & c4r, we see this first piece as a feasible change, not requiring too big changes in the architecture of spatialFilter.

(b) MULTIPLE MASKS. In this case, the impact is potentially quite bigger and we need more info. Let me add some comments / questions:

• b1) ui: I think this would require an input in the UI to add proper ID to each filter (or maybe we can safely assume ‘region A’, region B, region C for each polygon…?)
• b2) state: we should manage a collection of different / independent spatial filters, with an id + corresponding actions (probably we should assume single-polygon mode for each filter, for simplicity reasons). Probably creating a new set of keys & actions would be better, as overloading current spatialFilter would be complex and probably dirty. This would require several changes in c4r, especially around widgets & also worker calculations.
• b3) deck.gl: first we would need to decide how to deal with masks… but probably we would need to manage a collection of different MaskLayer, with custom ID, from the c4r side, so probably no limitations on deck.gl (any limit on the number of masks, due to perf. reasons @felixpalmer ?)
• b4) widgets. Main question HERE! ❓❓. Right now the spatialFilter just ‘merges’ with the rest of the filters, without changing dimensions of the feature groups. But if I understand you well, what you suggest here is basically having the possibility of splitting the features into different groups (group A, group B, group C), and that would need to be displayed in the widgets…
  • Is that correct? Are you expecting that automatic behavior in the context of this RFC? I’m not 100% sure about it. You said you had a PoC, would you share it too?
  • If that’s the case, this needs more discussion about those effects in widgets… if not please elaborate further...

Perf note: from Felix’s feedback… adding complex geometries would have a clear impact over perf, so yes I also think displaying a warning over XXX vertices would be required (and then we might consider the complexity of using custom structure for Circles in a second step).

cc/ @bbecquet

from carto-react.

Great!

I think that the (a): SINGLE MASK, all agree, but I don't understand

and explicitly excluding here drawing holes

Can you explain me?

On another side (b) MULTIPLE MASK

Yes, the impact is bigger because you need to do an intersection by every position of the array.

• b1): The name maybe be autogenerated, region A, B, C or Mask A, B, C, is the same to me.
• b2): Alright we need to generate a unique id to be able to associate a geometry. About c4r, I've done the changes and they aren't so hard.
• b3): The MaskLayer accepts an array of geometries and works very fast, you don't need to change it.
• b4): About the widgets... the models aren't changed only the UI

I have a POC but is not accessible because I've copied several parts of c4r and I have done the hack for h3-js to be allowed to work in workers if you want I can show you the results in a video, or if you want to see the code, I can create a draft with the changes in the repository.

from carto-react.

any limit on the number of masks, due to perf. reasons @felixpalmer ?

There can be max four independent masks. Think of each of these as a single color bitmap that contains a rendered deck.gl layer. This layer can be a composite layer, which contains collection of sublayers, so in practice you can render as many layers as you want into a mask. You will have to pay the cost of them being rendered any time the mask changes, which can often be every frame.

Standard (non-mask) layers can specify one (and only one) mask for masking. It is not possible to read two masks and apply the result of both to a layer.

The MaskLayer accepts an array of geometries and works very fast, you don't need to change it.

@aaranadev it would be good to see your PoC, to see how you've implemented the Intersect mode

from carto-react.

@felixpalmer @VictorVelarde, I've uploaded the code to public repo.

https://github.com/aaranadev/c4r-compare-mode

You can see the code and test it.

When npm install is done, you need to remove the error of h3-js library, more info: https://github.com/CartoDB/carto-react/blob/master/patches/h3-js%2B3.7.2.patch

from carto-react.

@aaranadev this code https://github.com/aaranadev/c4r-compare-mode/blob/main/src/components/layers/MaskLayer.ts#L16 in your example is not doing an intersect. I mean, the SolidPolygonLayer is receiving a list of polygons so each feature inside the polygons is going to be rendered, right?

from carto-react.

Yes, it's the same code that C4R has but changing an array of one element by n elements

from carto-react.

Before entering in the technical discussion I'd go a step back and getting more info why do you need this feature. I'm asking because it looks like it's related to comparison mode and I think there are approaches far simpler creating a new widget.

We need the functionality to add more than one geometry to allow filtering or drawing on the map.

Looks like a good feature for CARTO for React. Agree on this

Feature Selection Widget: The widget must allow the show of various geometries together and if the union between them is inclusive or exclusive. The spatial filter feature must allow saving multiple geometries and show them in FeatureSelectionLayer and MaskLayer

Not agree with this approach. There are simpler ways of implementing it. Like creating a new widget that can ask a source to get the data filtered by a custom geom.

I think using the FeatureSelection Widget for this is making it far more complicated

I'll propose a solution early next week

from carto-react.

I've setup a meeting next Monday to discuss it and we'll post the conclusion here.

We've proposed this feature because we want to have a comparison mode in site selection. This proposal works but requires a huge effort that it's not required to accomplish the comparison mode in site selection.

• The proposed solution is complex and requires lots of non-trivial work. To Give support to multi geometry in the spatial filter, in mask extension, modify the way widgets return data...
• It adds complexity for an external user. It's not easy to understand that there is a mode that returns the data per each geometry in the filter.

I think it could be addressed with a simpler approach.

1. We can modify the workers to return the data based on a custom geometry. So a worker will take the raw data, filter by a custom geometry, and then return the data for the desired widget (formula, histogram, category...).
2. The comparison mode doesn't need to interact with layers, nor filter other widgets, it just need to show the data, so we can just use the widget ui for that purpose implementing a comparison mode only on the widget ui.

from carto-react.