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

Extend way operation blocked for the first second of selecting a node at the end of a way. #6289

Open
BjornRasmussen opened this issue May 2, 2019 · 15 comments
Labels
bug A bug - let's fix this! performance Optimizing for speed and efficiency

Comments

@BjornRasmussen
Copy link
Contributor

This is not present on the live version on osm.org, but is on preview.ideditor.com/master
To reproduce:

  1. Go to a place like http://preview.ideditor.com/master/#map=18.58/4.11633/15.13678
  2. Click on a node at the end of a highway, and then click A within a half second.
  3. The line will not be extended. Half of the time, the message Continued a line will show up, but no line continuation will actually happen.

This is not a very serious bug, but really slows down cleanup of disconnected ways.

@bhousel
Copy link
Member

bhousel commented May 2, 2019

Hmm I just tried it but wasn't able to reproduce it in Chrome 73...

@bhousel bhousel added the bug-cant-reproduce Having trouble reproducing this issue label May 2, 2019
@BjornRasmussen
Copy link
Contributor Author

I had validation set to everything everywhere - would that make a difference?

@bhousel
Copy link
Member

bhousel commented May 2, 2019

I had validation set to everything everywhere - would that make a difference?

Probably not - these validators run after the tile is downloaded and parsed during a browser idle callback. (some browsers don't support this, and will just run the task 1ms later.)

Can you let me know some more details about which browser you have? If you are able to use the browser's developer tools to profile what it's doing during that half second, that would help a lot.

@BjornRasmussen
Copy link
Contributor Author

I can reproduce this on both Chrome for Ubuntu and Windows easily. I can't reproduce it on Microsoft edge, though. I haven't tested it with other browsers yet.

@BjornRasmussen
Copy link
Contributor Author

It is actually possible to reproduce on Edge, just more difficult.
It seems to be something with the node not being selected yet.

@BjornRasmussen
Copy link
Contributor Author

If I click on a node at the end of a way, and then try extending the way before iD has selected the node, this issue pops up. For some reason, chrome takes longer than edge to select the node, but it is still reproducible on both.
I would not consider this a major issue, and it seems very difficult to fix - I'm not sure how speeding up the "select node" process would work.

@BjornRasmussen
Copy link
Contributor Author

BjornRasmussen commented May 2, 2019

The release version of iD also has the selection delay, but it somehow "remembers" that I pressed A once the node has been selected. I'm not sure why this is different in the master version.

@bhousel
Copy link
Member

bhousel commented May 2, 2019

For what it's worth... #6028 / 37557a7 introduced a 20ms delay between clicking and selection, to give blur events a change to fire.
But -

  • That was added yesterday, so I doubt it is what you are seeing
  • I don't think you are sneaking a keypress in that quickly

It's more likely that it takes a tiny bit longer between when you enter modeSelect and the menu is ready, because of it checking whether the operations are available. So I will check that and see whether we are doing unnecessary work.

bhousel added a commit that referenced this issue May 2, 2019
@bhousel
Copy link
Member

bhousel commented May 2, 2019

I did notice that we were frequently resetting the sidebar to scrollTop=0, (on every select or hover of a feature) so I added some code in 6181905 to avoid doing this so much. Each one of those was triggering about 40ms of reflow in my testing. That still doesn't add up to half a second, but every little bit will make iD feel more snappy.

@tordans
Copy link
Collaborator

tordans commented May 2, 2019

I believe I was able to reproduce the issue on Chrome, MacOS. I tried to record it with the performance tool on. In the bottom left there is https://github.com/keycastr/keycastr at work showing what keys I press.

Video: https://www.dropbox.com/s/9u69fd2wrwgq4l8/osm-ideditor-issue6289.mov?dl=0 (temporary)

Can you read anything from the performance-recording, Bryan? I still don't really know what to look for with those graphs.

@bhousel
Copy link
Member

bhousel commented May 2, 2019

Thanks @tordans - it does look like the style recalculation is causing the delays.

@BjornRasmussen
Copy link
Contributor Author

This is also blocking the split way operation if that helps.

@quincylvania quincylvania added bug A bug - let's fix this! performance Optimizing for speed and efficiency and removed bug-cant-reproduce Having trouble reproducing this issue labels May 14, 2019
@bhousel
Copy link
Member

bhousel commented May 17, 2019

I think this might be fixed now - @BjornRasmussen can you check it again in preview?

@BjornRasmussen
Copy link
Contributor Author

It hasn't been fixed yet. I've learned to wait before pressing action shortcuts, though, so this isn't affecting me anymore.

@BjornRasmussen
Copy link
Contributor Author

What exactly happens before showing the possible operations when someone selects a node? It seems to be taking 1.5 seconds for me now on master, which is quite strange. My computer is from 2010, but it's still quite strange that so much needs to happen before the available operations are returned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug - let's fix this! performance Optimizing for speed and efficiency
Projects
None yet
Development

No branches or pull requests

4 participants