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

Rework GlobeControls #545

Open
nosy-b opened this issue Oct 20, 2017 · 2 comments
Open

Rework GlobeControls #545

nosy-b opened this issue Oct 20, 2017 · 2 comments
Milestone

Comments

@nosy-b
Copy link
Contributor

nosy-b commented Oct 20, 2017

The GlobeControls class has evolved a lot since the creation of itowns2. It is of great help but it is getting very messy with 2000 lines of delicious spaghetti.

A complete refactorization is necessary.

@autra
Copy link
Contributor

autra commented Oct 20, 2017

Yes!

A basic idea: the new GlobeControls should be a FrameRequester. We use the event listeners to only set states (panning, zooming, etc...) and relevant variables (mouseDown positions and stuff). We do the heavy work in the update function of this GlobeControls. See other controls for inspiration.

It might be necessary to add other anchors for FrameRequester in MainLoop (being called before rendering, after rendering, before update, after update etc...) to be able to do everything we want (without having to setup timeout, promises on command-empty, or dummy frame requesters).

Last but not least, I think we can work incrementally by having a GlobeControls2.js, and replacing the former GlobeControls once we reach feature parity, instead of trying to rewrite everything in one PR.

gchoqueux added a commit that referenced this issue Oct 26, 2017
In PR #545 updateCameraTransformation is missing setCameraTargetPosition
@Mkonini Mkonini added the design label Nov 3, 2017
@gchoqueux gchoqueux mentioned this issue Dec 6, 2017
9 tasks
@gchoqueux
Copy link
Contributor

Since #795, Globe Controls has been greatly refactoring and debugging.
It still remains

  • FrameRequester
  • Collision refacto

@mgermerie mgermerie added this to the 2.xx.x milestone Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants