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.

Sign up for GitHub

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

Jump to bottom

Version 2024.3.2 #143

Merged
merged 22 commits into from
Jul 18, 2024
Merged

Version 2024.3.2 #143

merged 22 commits into from
Jul 18, 2024

Conversation

Waguramu
Copy link
Contributor

@Waguramu Waguramu commented Jul 8, 2024 •
edited

Goal: add marker clustering based on partial quadtrees.

Scope:

#124 #126

Deliverables:

  • Feature search service fixes.
  • CoreLib binding.

How to test:

  • Build with as usual.
  • Serve with mapget.

Note:

  • Limit rendering triggers on mouse actions (use zoom level change instead).

@Waguramu Waguramu self-assigned this Jul 8, 2024
@Waguramu Waguramu mentioned this pull request Jul 9, 2024
Closed
@josephbirkner josephbirkner changed the title Fix displaying a lot of markers performance issues Clusterign Jul 15, 2024
@josephbirkner josephbirkner changed the title Clusterign Clustering Jul 15, 2024
@MisterGC
Copy link
Contributor

MisterGC commented Jul 16, 2024 •
edited by Waguramu

My remarks (to be extended, starting with most-recent, I'll apply priorities soon):

  • Consider limiting the cluster result indicators to avoid small font sizes, imo 1000+ is enough - maybe we could even just write 1k+ which is probably also understood by most/all users (e.g. plain pin for 1, 2-10, 10+, 100+, 1k+, 10k+!?)
  • Search doesn't incorporate coverage information - when I deactivate all data layers, any search finishes at once, if I activate one layer which has a coverage of one tile which is far away from my current viewport, it search all tiles
  • Search Result Indicators should be removed if I completly change the viewport, deactivate the a datalayer (ideally only if I deactivate one that provided search results) and/or a style
    image
  • Why is there 1+ and 2+ - I'd say we could just show the 1+ (which is only one hit!?) without any text
    image
  • Increase the contrast between background of the cluster icons and the text (number of hits) - especially with a high number/small font size, it makes it unnecessarly hard to read - it seems the text color varies (based on which conditions!?) - but even if the contrast is higher (white text) it would be better to increase the font size (see second screenshot)
    image
    image
  • There seems to be something wrong with the number of hits or I misinterpret the numbers - have a look at the following screenshot:
    image
    image

@josephbirkner
Copy link
Collaborator

josephbirkner commented Jul 16, 2024 •
edited by Waguramu

  • Initial zoom level
  • Highlight color fix
  • Pick higher clustering level
  • Fix eyeOffset
  • Merge search controls branch into this one
  • Decrease list control item size

@josephbirkner
Copy link
Collaborator

josephbirkner commented Jul 16, 2024 •
edited by Waguramu

  • For parent nodes, consider to calculate their pin position as the average of the children to avoid the cluster from jumping around while zooming.

@Waguramu
Copy link
Contributor Author

Regarding search improvements, see #144

@josephbirkner josephbirkner changed the title Clustering Version 2024.3.2 Jul 17, 2024
Waguramu and others added 16 commits July 17, 2024 17:00
@Waguramu
* Add clustering for feature search pins
d1357b9 
* Change priority of processed tiles for feature search
@Waguramu
* Introduce partial quadtree DS for better marker representation
7fff502 
* Add auto level method
@Waguramu
Improve rerender only when zoom level changes
98e183d
@Waguramu
* Improve node creation in QuadTree
425df59 
* Add average position for the cluster
* Render partially at max zoom level
@Waguramu
Feature search speed optimizations
97c3b79
@Waguramu
Remove commented lines
146ef0a
@Waguramu
Fix pin tiers array
e035f3a
@Waguramu
Make degree cartographic position nullable (no need to store it further)
dce1737
@Waguramu
Set zoom level to lower level
f8fd350
@Waguramu
* Update pin labels
d3cf682 
* Add custom pins for clusters
* Fix initial zoom level
* Fix highlight color not updated correctly on zoom
* Pick better clustering levels
* Fix eyeOffsets too high
* Decrease list control item size
@Waguramu
Add feature search controls
20b6812
@josephbirkner @Waguramu
Do not attempt to convert tileId -1 to uint64.
55fddb9
@Waguramu
Set average clusters position as the parent centre
66ba3d3
@Waguramu
* Adjust feature search controls
d384390 
* Bump version
@Waguramu
Update default pin colour
2769937
@Waguramu
Bump lock version
4a218c0
@josephbirkner
Copy link
Collaborator

josephbirkner commented Jul 17, 2024 •
edited

  • Fix the jump-to-wrong-feature on selection
  • We need to catch simfil exceptions in the workers

josephbirkner
josephbirkner approved these changes Jul 17, 2024
View reviewed changes
Copy link
Collaborator

@josephbirkner josephbirkner left a comment

Choose a reason for hiding this comment

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

Let's goo

@josephbirkner josephbirkner merged commit fdec735 into main Jul 18, 2024
1 check passed
@josephbirkner josephbirkner deleted the quadtree-clustering branch July 18, 2024 06:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@josephbirkner josephbirkner josephbirkner approved these changes

@MisterGC MisterGC Awaiting requested review from MisterGC

Assignees

@Waguramu Waguramu

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Waguramu @MisterGC @josephbirkner