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

PackedByteArray resource compression not triggered by sub-resources #92820

Open
eswartz opened this issue Jun 5, 2024 · 3 comments · May be fixed by #90902
Open

PackedByteArray resource compression not triggered by sub-resources #92820

eswartz opened this issue Jun 5, 2024 · 3 comments · May be fixed by #90902

Comments

@eswartz
Copy link
Contributor

eswartz commented Jun 5, 2024

Tested versions

  • Reproducible in 4.3 beta 1 and master 96a386f

System information

Godot v4.3.beta.mono (e2e6a4d2b) - Debian GNU/Linux trixie/sid trixie - X11 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3080 Ti (nvidia) - AMD Ryzen 7 5800X 8-Core Processor (16 Threads)

Issue description

The release notes mention PackedByteArray encoding in text resources (https://godotengine.org/article/dev-snapshot-godot-4-3-beta-1/#packedbytearrays-saved-with-base64-encoding), but it doesn't activate for me in certain cases.

I'm not sure this is easy to do without a @tool script which can create nested sub-resources with large data arrays, but that's what I was doing when I ran into this. I.e. I have in-editor scripts that generate Images based on chunks of data and store them in fields of nodes of the scene.

It seems from examining the code that #90889 may be responsible (and may go away later, according to #90889 (review)). It tries to avoid resource incompatibility situations by only triggering compression for large-ish arrays, but contents of variants embedded in sub-resources are not scanned directly, thus missing large arrays that may still be part of the same .tscn file.

Steps to reproduce

To repro:

  • open the project in the editor
  • look at the 3D view
  • confirm a sprite appears
  • save the project
  • check the fmt_test.tscn file on disk. Should be much less than 14 megs in size.

Minimal reproduction project (MRP)

fmttest.zip

@KoBeWi
Copy link
Member

KoBeWi commented Jun 5, 2024

Another related case is var_to_str().

@timothyqiu
Copy link
Member

This is caused by a bug when handling PROPERTY_USAGE_RESOURCE_NOT_PERSISTENT (which is only used by ImageTexture 🤣 ).

Would be fixed by #90902.

@eswartz
Copy link
Contributor Author

eswartz commented Jun 6, 2024

This is caused by a bug when handling PROPERTY_USAGE_RESOURCE_NOT_PERSISTENT (which is only used by ImageTexture 🤣 ).

Would be fixed by #90902.

Ah good. Yes, that PR works for me.

@AThousandShips AThousandShips added this to the 4.3 milestone Jun 6, 2024
@KoBeWi KoBeWi removed this from the 4.3 milestone Aug 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants