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

Rework scene preview thumbnails #102313

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

daniel080400
Copy link

@daniel080400 daniel080400 commented Feb 2, 2025

Supersedes #102263
Will need to disable code from #96544 because of conflict

Make scene files (.glb, gltf, .tscn, .scn) to always generate missing/outdated thumbnails, captures in a fixed angle, and fits all visual elements into the image, operation is done on a separate thread. Respects environment settings at rendering/environment/defaults/default_clear_color.

This PR also fixes preview image caching, when testing, file has to be changed to generate a new thumbnail. (md5 != last_md5)

Caveats

  1. Loses the ability to capture custom thumbnails with view. Open and save a scene still result in fixed view thumbnails, but as discussed in Add an option to capture scene thumbnails with fixed view #102263, I think it would be better to implement custom thumbnails in another better approach.

  2. If scene has particle with large bounding box, preview camera will be set far away because of it. Meshes might be small in view. This is done intentionally, to accommodate scenes that are basically particle effects.
    image

3D

Camera will always point at world center, dolly out at 3/4 view angle, until all contents fit into screen.
For particles, fast-forwards them it to (lifetime * 0.5) in order to render something, with fixed seed.

Practical:
image

Production:
image

2D

Render in two passes, 2D and GUI, then combines 2D + GUI captures for the final thumbnail image.

Does NOT account the world center for 2D capture, this decision is made because the use case that will benefit most from scene thumbnails are prefabs, which normally don't care about world coordinates, thus dedicating more pixels to actual contents.

image

TODO:

  • Generate scene thumbnails on file create/rename/save/duplicate/import
  • Only apply preview lighting to scenes that have 0 light nodes
  • Prevent tool scripts from running when generating thumbnails
  • Support for 2d scenes (Scenes with 0 Node3D count)
  • Make thumbnail creation thread-safe
  • Do proper cache validation at editor_resource_preview.cpp to prevent outdated thumbnails
NOTES
  • Rip off scripts by using SceneState::get_bundled_scene() and SceneState::set_bundled_scene() to change the data of PackedScene before instantiating it.

  • In editor_resource_preview.cpp, suspect cache validation is somewhat wrong within these lines, will return early with a outdated thumbnail image:

    • line 276-281
    • line 305-307
    • line 488-491
  • Need to disable the old EditorNode::_save_scene_with_preview for this PR

  • Will result in outdated thumbnail if user quickly (save->edit->save) a scene within ~0.5 seconds, this should not happen, is there something wrong with how EditorResourcePreview::queue is managed?

  • Control anchors do not play well with Camera2D, thus 2d scene thumbnail need to be rendered twice (gui + node2d)

  • It might be possible to draw viewport texture outside of the tree using RenderingServer::viewport_create() with EditorResourcePreviewGenerator::draw_requester.request_and_wait(), also being thread safe.

___

Sorry, something went wrong.

@ryevdokimov
Copy link
Contributor

ryevdokimov commented Feb 2, 2025

The top one problem to solve for this PR to work is to find a way to instantiate scenes without triggering tool scripts

I'm sure there is a valid reason for this, but I'm curious since I haven't given it much thought, but what is the issue with tool scripts?

The reason I ask is because I have a project that uses tools scripts to automatically and dynamically generate objects to build out a scene when it is first opened, so if the tool scripts were not ran it would be an empty or broken scene with nothing to show for a thumbnail.

@daniel080400
Copy link
Author

I'm sure there is a valid reason for this, but I'm curious since I haven't given it much thought, but what is the issue with tool scripts?

Mainly about unexpected behavior, running scripts even when scenes are not opened might confuse many, like if someone decided to print() messages or access filesystem in their tool scripts. As for stability concerns, I think if a tool script will crash the editor, it might be written in a very wrong way at the first place.

@ryevdokimov
Copy link
Contributor

I think in general if a user is using tool scripts it is at their own risk, and they should handle edge cases themselves, but new functionality should still accommodate their existence because they're vital for various workflows, users just need the tools to be able to minimize that risk.

This isn't to say that there is a solution to this problem, and we may just have to disable tool scripts while generating thumbnails and deal with it, but just food for thought.

@daniel080400 daniel080400 changed the title Rework scene thumbnails Rework scene preview thumbnails Feb 2, 2025
@AThousandShips AThousandShips added this to the 4.x milestone Feb 2, 2025
@daniel080400
Copy link
Author

daniel080400 commented Feb 2, 2025

Also in this PR, thumbnails are generated in a separate thread, and works by adding the preview scene under the current EditorNode to make it render (in a SubViewport), this means if your tool script is not thread-safe (by calling add_child() in _ready() ), it will cause errors.

I experimented if creating a separate SceneTree is possible, turns out it's not easy, and tool script can do more than manipulating the tree, raises risk. So I decided to focus on providing this feature in a safest way, with scripts disabled.

@JoNax97
Copy link
Contributor

JoNax97 commented Feb 2, 2025

This is looking very good!
I think that, if merged, it would be enough to close godotengine/godot-proposals#7232. Additional tweaks and personalization settings can be proposed by users after real world usage.

I'm very glad someone got to implementing this!

@daniel080400
Copy link
Author

daniel080400 commented Feb 5, 2025

I discovered there's no universal way to accurately determine the global Rect2 of every Node2D types (some can have textures, or meshes with various sizes, and it does not affect their transform2d, rect2), I decided to only address the most important types of Node2D into Rect2 calculation, where it will cover most use cases.

Fow now only Sprite2D is accounted into thumbnails, plan to implement for AnimatedSprite2D, MeshInstance2D, MultimeshInstance2D, TileMapLayer, Polygon2D, TouchScreenButton later. By then will consider 2D scene thumbnail as completed.

@daniel080400
Copy link
Author

daniel080400 commented Feb 15, 2025

2D scene thumbnail is completed, Node2D types that participates scene Rect2 calculations are:
Sprite2D, AnimatedSprite2D, MeshInstance2D, MultimeshInstance2D, TileMapLayer, Polygon2D, Line2D, TouchScreenButton

One limitation for 2d thumbnail is the max render viewport size is set to 2048x2048 for now (consider web editor could only handle 2K textures at most in rendering, correct me if wrong), if the scene is bigger than that, preview area will be cropped and centered.

image

@daniel080400 daniel080400 force-pushed the generate_scene_thumbnails branch from 315033d to c5faf99 Compare February 21, 2025 20:14
@daniel080400
Copy link
Author

daniel080400 commented Feb 21, 2025

All planned features completed. Squashed and green lighting this. Now for testing and clean up, should have plenty of time to bug fix this before 4.5 cycle

@daniel080400 daniel080400 marked this pull request as ready for review February 21, 2025 20:21
@daniel080400 daniel080400 requested a review from a team as a code owner February 21, 2025 20:21
@daniel080400 daniel080400 force-pushed the generate_scene_thumbnails branch 5 times, most recently from 699b912 to ad95f5f Compare February 22, 2025 01:37
@Calinou
Copy link
Member

Calinou commented Feb 23, 2025

consider web editor could only handle 2K textures at most in rendering, correct me if wrong

The universal baseline across all platforms Godot 4 runs on is 4096x4096 nowadays.

@daniel080400 daniel080400 force-pushed the generate_scene_thumbnails branch from ad95f5f to eb6a476 Compare February 23, 2025 18:46
@daniel080400
Copy link
Author

The universal baseline across all platforms Godot 4 runs on is 4096x4096 nowadays.

Thanks to clarify that, anyway this should not be a problem now. I updated to only zoom out the preview Camera2D when capturing 2d thumbnails (previously is done by scaling the SubViewport size).

@daniel080400 daniel080400 force-pushed the generate_scene_thumbnails branch from eb6a476 to 88d48c3 Compare February 23, 2025 19:20
Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

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

Tested locally (rebased on top of master 39c201c), I get an issue in 2D. The 2D platformer demo freezes when opened in the editor after the "restoring scene layout" popup is done running. This seems to occur with every 2D project I've tried.

In comparison, it works in a 3D project like Truck Town and Platformer:

image

Note that player.tscn is very small in the 3D platformer demo because the Decal node's AABB is taken into account. (The decal is used for a blob shadow.) Decal nodes' AABBs should probably be excluded if possible, as we only care about the surface the decal is being projected on for the preview, not the decal itself.

This PR also gets rid of the progress dialog that appears when saving a scene:

image

I consider this to be a good change for scenes that save in less than 1 second – this should also help resolve #78753. However, for scenes that take a while to save, we should have some kind of progress dialog back (or at least a visible notice somewhere in the editor). You can create a scene that takes a while to save by adding a VoxelGI node, setting it to 512 subdivisions, baking it and choosing Make Unique on the VoxelGI node's Data property.

Additionally, you could do the same as #97468 to improve preview quality using MSAA and SSAA together. This should only increase the GPU requirements minimally since the previews are low-resolution. To cater for 2D previews, also enable 2D MSAA when relevant, not just 3D MSAA.

For a future PR, we can also further improve the preview by adding a checkered floor below the object and enabling shadows on the DirectionalLight3D. This way, you get a sense of the object's scale.

@daniel080400
Copy link
Author

daniel080400 commented Feb 25, 2025

I get an issue in 2D. The 2D platformer demo freezes when opened in the editor after the "restoring scene layout" popup is done running. This seems to occur with every 2D project I've tried.

Issue caused by renderer being set to Compatibility, EditorResourcePreview will force all preview plugins to run on the main thread. I tweaked the code to have it run safely on single thread. I'm experimenting on a workaround to enable multi-thread if possible.

Edit 2025/02/28:
In rasterizer_gles3.h, this line was written:

_ALWAYS_INLINE_ bool can_create_resources_async() const { return false; }

I have no idea why this is set to false, so let's leave it as it is. We should only get back to this if anyone ever run into performance issues when generating previews using Compatibility renderer.

@daniel080400 daniel080400 force-pushed the generate_scene_thumbnails branch from 015f276 to 2a772da Compare February 28, 2025 10:59
@AThousandShips AThousandShips removed request for a team March 7, 2025 13:40
@daniel080400 daniel080400 force-pushed the generate_scene_thumbnails branch from 0ef5a7e to 0cfaad9 Compare March 7, 2025 14:34
@daniel080400 daniel080400 force-pushed the generate_scene_thumbnails branch from 0cfaad9 to 12680de Compare March 7, 2025 14:37
@daniel080400
Copy link
Author

Good again. Sorry for the disturbance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve scene thumbnails
6 participants