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

Split TransformControls into an adaptor that switches between three separate Rotate/Translate/ScaleControls classes? #3402

Closed
bhouston opened this Issue Apr 29, 2013 · 6 comments

Comments

Projects
None yet
3 participants
@bhouston
Contributor

bhouston commented Apr 29, 2013

I am concerned that Three.TransformControls tries to do too much and thus it is overly complex. I've been looking at the code for a while this morning and it is pretty involved.

I was wondering if it is possible to split most of the functionality of TransformControls into three separate classes such as ScaleControls, TranslateControls and RotationControls. If there is some shared functionality one could create a base class that each of these three share.

This would focus the code to each of these three specific use cases.

TransformControls would then become a means to switch between these more specific controls, rather than being all three of these complex controls.

Besides making it a lot simpler, it would also enable me to use just TranslateControls in some cases instead of always having to use all three controls as TransformControls and then trying to build in some filtering on top (which will make it even more complex.)

@mrdoob

This comment has been minimized.

Owner

mrdoob commented Apr 29, 2013

I agree that it's quite involved. It's also early days for it and I'm still cleaning it up. So far I'm still not sure what direction it should head to, so I'll rather leave as it is for some weeks and see how it feels then.

@bhouston

This comment has been minimized.

Contributor

bhouston commented Apr 29, 2013

No worries. I can take a stab at this when you are ready. I was not trying to get someone else to implement this change :-), rather just trying to make sure it is worthwhile before I embarked on a major refactor.

@mrdoob

This comment has been minimized.

Owner

mrdoob commented Apr 29, 2013

Yup yup.

@arodic

This comment has been minimized.

Contributor

arodic commented Apr 29, 2013

I agree this is a good idea. It also needs to be cleaned up quite a bit. There is a lot of shared functionality for different transform modes so I would definitely avoid having too much redundant code for the sake of easy maintenance. Perhaps put it all in TransformUtils?

Also, we should keep in mind features that we might want to add in the future and design accordingly. For example, attachment to multiple objects or geometry components, compatibility with multiple viewports, touch events, etc.

@bhouston

This comment has been minimized.

Contributor

bhouston commented Apr 29, 2013

@arodic, we actually need the multiple object and geometry components, multiple viewports, and touch events support for a project we will be open sourcing in the near future, so it isn't that far of a future need but rather an immediate need. :-)

We actually have a lot of this working already in a separate code base but really why should we keep it separate when we can combine effort and get the best solution together.

@arodic

This comment has been minimized.

Contributor

arodic commented May 18, 2013

Is anyone working on this one? I was thinking of taking a stab at it next week.

@mrdoob mrdoob closed this Sep 3, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment