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

Preliminary analysis of selection structure before initialising operations #3728

Closed
slhh opened this issue Jan 8, 2017 · 1 comment
Closed

Comments

@slhh
Copy link
Contributor

slhh commented Jan 8, 2017

This issue targets multiple things:

Currently each operation has to check the structure of selection itself to determine if it is available and enabled. We should not change this in general for flexibility reasons, but this seems to be inefficient due to repeating common checks multiple times. Therefore, we should take out common parts of the checks.

Instead of passing the array of selected iDs to the operation we can pass an object providing more information, which can also be reused for other purposes like the sidebar or task matching. The object should contain:

  • an array of selected ids
  • an array of selected entities
    This saves multiple lookup by id, and it avoid issues with ids becoming invalid while matching tasks asynchrously according to Context dependent task offering #3726 .
  • an array of geometry of the selected entities
  • an array of matched feature types of the selected entities
    This enables reusing this object for the selection list in the sidebar, it enables feature type specific task matching, and it enables offering specific operations like generating a public transport station based on selected platforms and stop_positions
  • an array per geometry containing indexes (for above arrays) of the selected objects with the respective geometry
    This makes also count per geometry available by using length.
  • the count of different geometries
  • the related parent
    This is enabling Make radial menu operations aware of related parent #3635.
    -Additonal properties identifing common selection structures (like way and one of its vertexes) and maybe specific information for it (e.g. end or inner vertex? connecting a route or MP?).

It looks like being time-consuming to provide all this information, but we can preprocess everything here without performance loss, provided that it has to be analysed by at least one operation otherwise.

@bhousel I would like to work on this issue in preparation for #3635, provided this is ok for you.

@bhousel
Copy link
Member

bhousel commented Jan 8, 2017

@bhousel I would like to work on this issue in preparation for #3635, provided this is ok for you.

I still don't understand what #3635 or #3726 are for, so I'd rather you don't work on this. Or it might be best for you to fork iD and take it in a different direction? My feelings would not be hurt! 😄

FWIW, I am pretty sure that id graph.entity lookups are not a significant performance bottleneck. The performance problem with selecting a lot of things is #3571.

@bhousel bhousel closed this as completed Jan 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants