fix: Movement handler stuck when clicking certain key combinations

[EMapGIS]

Description

fix #11903
When the mouse is pressed, the modifiers will be recorded. Modifying the modifiers during this process is invalid.

Issue number and link

#11903

Testing plan

Author checklist

• I have submitted a Contributor License Agreement
• I have added my name to CONTRIBUTORS.md
• I have updated CHANGES.md with a short summary of my change
• I have added or updated unit tests to ensure consistent code coverage
• I have updated the inline documentation, and included code examples where relevant
• I have performed a self-review of my code

[github-actions]

Thank you for the pull request, @EMapGIS! Welcome to the Cesium community!

In order for us to review your PR, please complete the following steps:

• Send in a Contributor License Agreement (CLA)
• Add yourself to the contributors file

Review Pull Request Guidelines to make sure your PR gets accepted quickly.

[ggetz]

Hi @EMapGIS, I don't see a signed CLA on file for you. We'll need one to review this PR. Could you please submit a Contributor License Agreement (CLA)?

[EMapGIS]

Hi @EMapGIS, I don't see a signed CLA on file for you. We'll need one to review this PR. Could you please submit a Contributor License Agreement (CLA)?

Sorry, I submitted a version before, but I submitted the wrong Github account. I have now resubmitted a version. Please check.

[ggetz]

I have now resubmitted a version. Please check.

@EMapGIS Perfect, thank you! I can confirm we received it.

[ggetz]

@jjspace Could you please review this PR?

[jjspace]

Hey @EMapGIS, thanks for the PR!

This solution does seem to fix the specific reproduction steps in the original issue however I don't think it's the right approach for a generalized answer. Changing the ScreenSpaceEventHandler to rely on a global variable creates an interdependence for mouse events that may not be obvious or intuitive to users and completely prevents listening to changes in the modifiers being held down throughout a drag if that's what the user wanted.

I believe the better approach would be to modify ScreenSpaceCameraController to better handle the cases where modifiers change and canceling ongoing events (like the initial drag event). I haven't dug into the the code to know the specific changes needed but I think it should be the responsibility of the default camera controller (in that class) to handle changes in modifiers similar to what's mentioned in this comment

(also please make sure CI is passing, seems there are some linting issues)

[EMapGIS]

Hey @EMapGIS, thanks for the PR!

This solution does seem to fix the specific reproduction steps in the original issue however I don't think it's the right approach for a generalized answer. Changing the ScreenSpaceEventHandler to rely on a global variable creates an interdependence for mouse events that may not be obvious or intuitive to users and completely prevents listening to changes in the modifiers being held down throughout a drag if that's what the user wanted.

I believe the better approach would be to modify ScreenSpaceCameraController to better handle the cases where modifiers change and canceling ongoing events (like the initial drag event). I haven't dug into the the code to know the specific changes needed but I think it should be the responsibility of the default camera controller (in that class) to handle changes in modifiers similar to what's mentioned in this comment

(also please make sure CI is passing, seems there are some linting issues)

Thank you very much for your reply. I have made some changes to my code and optimized the following aspects:

1. _lastMousePassModifier was modified to be a private property of ScreenSpaceEventHandler to prevent conflicts among multiple ScreenSpaceEventHandler.
2. Added "isMousePassModifierLock" as a static property of ScreenSpaceEventHandler. When its value is true, the modifier will be locked after entering the event. By default, it is false, still maintaining the original way of Cesium.

I have thought about what you said above about completely solving this problem. It is no small project. If there is no need to be compatible with the previous one, it would be fine. But if it needs to be compatible with the previous one, it will be very complicated. We will also sort this out to see if there is a better solution, and then we will communicate again.

[jjspace]

@EMapGIS We do not need to preserve the previous behavior, that was clearly a bug. But I believe the proper solution is probably a change to the default ScreenSpaceCameraController not the ScreenSpaceEventHandler. I do not think it should lock what modifiers were held down. It should handle changing modifiers during a drag "interrupting" ongoing events better.

[EMapGIS]

@EMapGIS We do not need to preserve the previous behavior, that was clearly a bug. But I believe the proper solution is probably a change to the default ScreenSpaceCameraController not the ScreenSpaceEventHandler. I do not think it should lock what modifiers were held down. It should handle changing modifiers during a drag "interrupting" ongoing events better.

@jjspace I restored the ScreenSpaceEventHandler. And according to your suggestion, I modified the CameraEventAggregator. Currently, it is possible to modify the modifiers during the movement process, stop the previous event and execute the new event. Thank you for the ideas you provided. You can take a look at the effect of the newly modified content.

7.21.1.mp4