godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
88.95k stars 20.17k forks source link

Moving directory reimports image assets and recalculates all metadata #92083

Open CitrusWire opened 4 months ago

CitrusWire commented 4 months ago

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. 3) In godot, drag the directory of images into the subfolder

Minimal reproduction project (MRP)

N/A

huwpascoe commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

[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 commented 4 months ago

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.

KoBeWi commented 2 months ago

This is another case of #23107