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

Moving directory reimports image assets and recalculates all metadata #92083

Open
CitrusWire opened this issue May 18, 2024 · 7 comments
Open

Comments

@CitrusWire
Copy link

Tested versions

4.2.2 - reproducable

System information

Windows 7

Issue description

Moving a directory of already imported assets (jpgs in this case) re-imports them at great computational expense.

I don't know what the import process entails, but given the insane temporary resource usage (12GB of RAM for this use-case), I expect part of it is analysing the images themselves to gather metadata, which makes sense. However, when moving a directory, Godot should only need to change relative file locations in the metadata; not do anything computationally expensive like recalculate all of the imagery metadata.

Steps to reproduce

  1. Create a new project
  2. add a directory of several hundred MB of pictures to the project. Ideally some of them should be 30+MB in size.
  3. Create a subfolder.
  4. In godot, drag the directory of images into the subfolder

Minimal reproduction project (MRP)

N/A

@huwpascoe
Copy link

This always happened for me whenever files are moved, I assumed that's how it's supposed to work? If not then it'd be convenient to have that fixed.

@CitrusWire
Copy link
Author

This always happened for me whenever files are moved, I assumed that's how it's supposed to work? If not then it'd be convenient to have that fixed.

Yes, this may be how it works, however it's low-hanging fruit for optimisation. Well, low-hanging on a conceptual level at least, implementation I have no idea about. I guess this is why they flagged it as "performance".

@AThousandShips
Copy link
Member

AThousandShips commented May 18, 2024

It's far from simple to handle this necessarily, depending on how the test is done it isn't guaranteed it can be assumed the file hasn't changed and needs reimport, though it could be reasonable to assume when moved in the editor (the tagging has nothing to do with improvements or ease, it's about the fact that it's about performance)

@CitrusWire
Copy link
Author

though it could be reasonable to assume when moved in the editor

That's the use-case I'm referring to here.

it isn't guaranteed it can be assumed the file hasn't changed and needs reimport [if you're doing the move outside of godot editor]

That one seems fairly simply to solve too: Keep a hash of the file contents. Upon re-import, rehash the file. If it's the same, it hasn't changed and no need to re-do the computationally expensive stuff.

@AThousandShips
Copy link
Member

AThousandShips commented May 18, 2024

[if you're doing the move outside of godot editor]

Please do not inject your own assumptions into quotes by me, that's not what I meant at all

But as I said, it might be trivial, but needs investigation, hence the needs testing

Could you please test on 4.3.dev6, as there's been improvements to the file system management since 4.2

@CitrusWire
Copy link
Author

CitrusWire commented May 18, 2024

Please do not inject your own assumptions into quotes by me, that's not what I meant at all

I was clarifying what I meant, that's why it's in square brackets. Had I have not done that, given the ambiguity of your statement, my follow-on statement would have inherited the ambiguity and may have been incorrect to some people's parsing.

@AThousandShips
Copy link
Member

AThousandShips commented May 18, 2024

Then do not present it as a quote please, because you are not quoting me, you are putting words in my mouth :) So just say what you mean instead of making claims about what I mean, thank you.

However as I said it depends on the test and when and how it's done, it depends on what we assume even when moving files in the editor, but the solution of hashing is done, it might just not be used in this particular case, but hashing is used, it's not a simple solution that has been missed

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

3 participants