Skip to content
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

Fix #9208 Timeline map sync does not update for time ranges visualization #9209

Merged
merged 2 commits into from
Jun 5, 2023

Conversation

allyoucanmap
Copy link
Contributor

@allyoucanmap allyoucanmap commented Jun 1, 2023

Description

This PR improved the get time items selector to use the updated range data if provided

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x", remove the others)

  • Bugfix

Issue

What is the current behavior?

#9208

What is the new behavior?

Breaking change

Does this PR introduce a breaking change? (check one with "x", remove the other)

  • No

Other useful information

@tdipisa
It seems there is a problem with the spatial filter of domains, here an example:

The following example shows a DescribeDomains request where we expect to get information of a single feature but instead we get two:

request: https://development.demo.geonode.org/geoserver/gwc/service/wmts?service=WMTS&REQUEST=DescribeDomains&version=1.0.0&layer=geonode:test_time&tileMatrixSet=EPSG:4326&bbox=829991.4716345865,5755211.389189923,1039581.8031925397,5899065.876422625,EPSG:3857&expandLimit=20&time=2022-06-01T00:00:00.000Z/2024-01-09T11:07:53.218Z

response:

<Domains xmlns="http://demo.geo-solutions.it/share/wmts-multidim/wmts_multi_dimensional.xsd" xmlns:ows="http://www.opengis.net/ows/1.1" version="1.2">
<SpaceDomain>
<BoundingBox CRS="EPSG:4326" minx="2.021484375000004" miny="40.871987756697415" maxx="30.849609375000014" maxy="50.645977340713586"/>
</SpaceDomain>
<DimensionDomain>
<ows:Identifier>time</ows:Identifier>
<Domain>2023-01-01T00:00:00.000Z/2023-06-01T00:00:00.000Z,2023-01-01T00:00:00.000Z/2023-12-31T00:00:00.000Z</Domain>
<Size>2</Size>
</DimensionDomain>
</Domains>

Visual representation of bbox param (red) and returned space domain (orange)
image

Note: We are also applying the bbox param to GetDomainValues and GetHistogram so they could have the same problem

@allyoucanmap allyoucanmap added bug BackportNeeded Commits provided for an issue need to be backported to the milestone's stable branch C044-VLAANDEREN-2023-SUPPORT labels Jun 1, 2023
@allyoucanmap allyoucanmap added this to the 2023.01.02 milestone Jun 1, 2023
@allyoucanmap allyoucanmap self-assigned this Jun 1, 2023
@allyoucanmap allyoucanmap linked an issue Jun 1, 2023 that may be closed by this pull request
1 task
@tdipisa tdipisa requested review from offtherailz and MV88 and removed request for offtherailz and MV88 June 1, 2023 10:27
Copy link
Member

@offtherailz offtherailz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should

  • move expandLimitSelector default internally if possible
  • Check and debug the single point use case to indentify why it works

@allyoucanmap
Copy link
Contributor Author

@offtherailz below test done with master without applying fix from this PR:

The layer in this map does not have data.values and the domain contains -- for this reason it always pass to the else if

image

in this case the rangeData is populated from this app initialization because of the updateRangeDataOnRangeChange epic that update the value for all domain that returns true for isTimeDomainInterval

image

check-getTimeItems.mp4

The check || data && data.domain && !isTimeDomainInterval(data.domain) is still used by domain that are not interval because they have a rangeData equal to undefined at initialization

image

@offtherailz offtherailz self-requested a review June 1, 2023 13:11
@tdipisa tdipisa merged commit ecd4e2d into geosolutions-it:master Jun 5, 2023
6 checks passed
@tdipisa
Copy link
Member

tdipisa commented Jun 5, 2023

@offtherailz you forgot to merge this :)

@ElenaGallo please test in DEV and let us know if we can backport.

Let's spend some more time on what you have reported for the GS issue you have experienced, maybe checking it with someone from the GS team.

@aaime
Copy link
Member

aaime commented Jun 6, 2023

The issue with the bbox capturing two features is due to the WMTS multidimensional using a "BBOX" filter against the database, which in turn is configured for loose bbox filtering:

image

With loose bbox, queries are very fast, but not precise, as a "bbox vs bbox" filter is applied: the second line does not intersects the red search area, but the bbox of the line does, that's why it's returned.

How to fix it:

  • Disable loose bbox at the store level. That will slow down all GetMap/GetFeature requests (I don't recall how much) to make all the filters more precise.
  • Change the WMTS multidimensional code to use a "intersect polygon" filter instead, which is always precise, regardless of that setting.

@tdipisa
Copy link
Member

tdipisa commented Jun 6, 2023

Thank you so much for reporting the above @aaime

@ElenaGallo
Copy link
Contributor

Test passed, @allyoucanmap please backport to the stable branch. Thanks

allyoucanmap added a commit to allyoucanmap/MapStore2 that referenced this pull request Jun 7, 2023
…anges visualization (geosolutions-it#9209)

* Fix geosolutions-it#9208 Timeline map sync does not update for time ranges visualization

* move the default expand limit value to selector
@allyoucanmap allyoucanmap removed the BackportNeeded Commits provided for an issue need to be backported to the milestone's stable branch label Jun 7, 2023
tdipisa pushed a commit that referenced this pull request Jun 7, 2023
…tion (#9209) (#9211)

* Fix #9208 Timeline map sync does not update for time ranges visualization

* move the default expand limit value to selector
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Timeline map sync does not update for time ranges visualization
5 participants