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

#1177 Added possibilities for import positions #1297

Merged
merged 1 commit into from
Dec 9, 2019

Conversation

davidlamhauge
Copy link
Contributor

@davidlamhauge davidlamhauge commented Nov 16, 2019

"Tredje gang er lykkens gang" (Danish proverb)
I've made this extension to the much discussed import feature.
Here you can import relative to four positions, as shown in the screenshot below.
Whether my naming for the four possibilities are precise enough is up to you. Let's discuss it.
importpos
When importing to follow camera, we have to set the viewManagers importView for each image, which called for 4 lines of code in the Editor::importImage() function. If any of you know a smarter way, please tell me how.
Finally, this only addresses the urgent problem of the merged feature, that could be more precise. We should have a feature where we could import to whatever position we want. That must be another time.

Copy link
Member

@MrStevns MrStevns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

It could be considered whether we should have an ImportManager now, since that functionality is growing in complexity, I also think it's unnecessary to look up the layer for every image is imported, since we know it's always the same.

It's not crucial right now, hence my approval but I think it should be done sooner rather than later.

@MrStevns MrStevns added the 🔹 Minor PR (only one reviewer required) label Nov 19, 2019
@MrStevns MrStevns merged commit 59b6fd6 into pencil2d:master Dec 9, 2019
@Jose-Moreno Jose-Moreno added this to the 0.6.5 milestone Jan 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔹 Minor PR (only one reviewer required)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants