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

Editor: import images as planes #28342

Closed
wants to merge 1 commit into from

Conversation

linbingquan
Copy link
Contributor

Related issue: #XXXX

Description

As the title says.

editor/js/Loader.js Outdated Show resolved Hide resolved
editor/js/Loader.js Outdated Show resolved Hide resolved
editor/js/Loader.js Outdated Show resolved Hide resolved
@Mugen87
Copy link
Collaborator

Mugen87 commented May 12, 2024

@mrdoob I'm not sure if this feature is appropriate for a scene editor. What do you think?

@linbingquan
Copy link
Contributor Author

linbingquan commented May 12, 2024

Um, what if the user imports a normal map?

I feel this problem is a bit like blender's one-click batch Add texture function.

Noteside: I was use examples\textures\ambientcg\*.jpg of images to test it

image

@Mugen87
Copy link
Collaborator

Mugen87 commented May 13, 2024

@Mugen87 Mugen87 added this to the r165 milestone May 13, 2024
@Mugen87
Copy link
Collaborator

Mugen87 commented May 13, 2024

It seems there are merge conflicts now.

@linbingquan
Copy link
Contributor Author

It seems there are merge conflicts now.

Done.

editor/js/Loader.js Outdated Show resolved Hide resolved
@Mugen87
Copy link
Collaborator

Mugen87 commented May 13, 2024

It seems the menu point is still missing (see #28342 (comment)). Or maybe it was lost when fixing the merge conflicts?

@linbingquan
Copy link
Contributor Author

linbingquan commented May 13, 2024

It seems the menu point is still missing (see #28342 (comment)). Or maybe it was lost when fixing the merge conflicts?

Honestly, I am not write this part of the code, because now click the import button directly into the file selector, I do not know how to add this part of the code is more appropriate.

@linbingquan
Copy link
Contributor Author

It seems the menu point is still missing (see #28342 (comment)). Or maybe it was lost when fixing the merge conflicts?

If we add a submenu directly to the import button, the user experience should be bad, such as adding a submenu, the new user should be unaware that the import button can be clicked.

@linbingquan
Copy link
Contributor Author

linbingquan commented May 13, 2024

Do we make Import Images as Plane to be the same level as the import button

@Mugen87
Copy link
Collaborator

Mugen87 commented May 13, 2024

I see. Let's just use File > Import. I now recognize the editor does not have separate Import entries for file formats like in Blender so we don't need a separate entry for images.

Copy link
Collaborator

@Mugen87 Mugen87 left a comment

Choose a reason for hiding this comment

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

If you drag a glTF with its bin file and textures into the editor, this happens:

image

Files for testing: https://github.com/mrdoob/three.js/tree/dev/examples/models/gltf/DamagedHelmet/glTF

Meaning the editor loads all model textures as image planes which is of course not intended.

It seems loading "image as plane" can only work via the menu and not via drag'n'drop.

@Mugen87 Mugen87 removed this from the r165 milestone May 15, 2024
@linbingquan
Copy link
Contributor Author

@Mugen87 I added a new function button entry File -> Import Images as Planes.

图片

@linbingquan linbingquan changed the title Editor: load image as Plane Editor: import images as planes May 15, 2024
@Mugen87
Copy link
Collaborator

Mugen87 commented May 16, 2024

After some more testing I'm not sure its worth adding the feature. I doubt it's super helpful for a scene editor...

In any event, I'll defer to @mrdoob .

@Mugen87 Mugen87 dismissed their stale review May 16, 2024 09:48

Fix was applied.

@mrdoob
Copy link
Owner

mrdoob commented May 17, 2024

Sorry, I don't think we should be doing this.

@mrdoob mrdoob closed this May 17, 2024
@linbingquan linbingquan deleted the dev-editor-loader-image branch May 17, 2024 10:50
@Mugen87 Mugen87 added this to the r165 milestone May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants