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

Add a 3D settings demo (Graphical settings) for 4.0-dev #713

Merged
merged 1 commit into from
Apr 6, 2022
Merged

Add a 3D settings demo (Graphical settings) for 4.0-dev #713

merged 1 commit into from
Apr 6, 2022

Conversation

voylin
Copy link
Contributor

@voylin voylin commented Apr 3, 2022

Could possible fix #600.
This demo has some of the basic graphical settings that game devs would want. I tried to keep it minimal because it's just a demo. I hope this is sufficient and that there are enough comments explaining everything. Everything works without any bugs or errors.

@aaronfranke aaronfranke added this to the 4.0 milestone Apr 3, 2022
@voylin
Copy link
Contributor Author

voylin commented Apr 3, 2022

Because this has to do with graphics, I guess that this has to be "Vulkan Clustered", right? I changed the README file already, if it is not correct, please let me know!

Copy link
Member

@aaronfranke aaronfranke left a comment

Choose a reason for hiding this comment

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

The folder of this demo should be renamed to graphics_settings, such that the path is 3d/graphics_settings.

3d/3d_settings_menu/project.godot Outdated Show resolved Hide resolved
3d/3d_settings_menu/project.godot Outdated Show resolved Hide resolved
3d/3d_settings_menu/settings.gd Outdated Show resolved Hide resolved
Copy link
Member

@aaronfranke aaronfranke left a comment

Choose a reason for hiding this comment

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

Some of the sides of these buttons are being cut off.

Screenshot from 2022-04-03 04-45-58

There are other settings that we should showcase in this demo. Some are nodes such as VoxelGI and ReflectionProbe. Some are WorldEnvironment settings such as SDFGI, SSAO, SSIL (looks like you already did this for SS reflections). It would also be good to showcase texture quality. Another one would be model quality (maybe using the GLTF auto-LOD system that @fire was working on). This will likely end up needing a menu with sections and a scroll bar.

This will also need a more detailed review from @Calinou to ensure that this includes what he was hoping for with a graphics settings demo. I would also appreciate if @Calinou can give thoughts on the current state, on what you've already done so far and on my suggestions, and maybe he has some other suggestions.


func _on_ui_scale_option_button_item_selected(index: int) -> void:
# For changing the UI, we take the viewport size, which we set in the project settings.
var new_size : Vector2
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
var new_size : Vector2
var new_size: Vector2

etc. https://docs.godotengine.org/en/latest/tutorials/scripting/gdscript/gdscript_styleguide.html

@aaronfranke aaronfranke requested a review from Calinou April 3, 2022 09:54
@voylin
Copy link
Contributor Author

voylin commented Apr 3, 2022

I was able to fix the border cut-off by giving the option buttons a bit more space.
Ambient Occlusion is SSAO and indirect lightning is SSIL, I renamed these to this option to make it more clear.

At this moment there are some graphical errors because of the alpha stage of the engine I'm working on Godot 4.0 alpha-5.
Texture quality, model quality and lod is something I will look into doing after @Calinou also gave his opinion of what he wants to get added. In the meantime I'll do a bit more research about those kind of settings as I don't have any experience with those kind of settings before.

Thank you for your input and advice!

@Calinou
Copy link
Member

Calinou commented Apr 5, 2022

I checked out the demo locally.

The demo looks like a good starting point to me (and should already be useful to users in its current form). Feel free to merge as-is and I'll make further changes in a future pull request.

Copy link
Member

@aaronfranke aaronfranke left a comment

Choose a reason for hiding this comment

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

Overall this looks great, the code is well-documented. I did encounter some bugs but I think they're all Godot bugs, not bugs with this project. I have a suggestion for getting the viewport size from project settings, and then this can be squashed and merged. Bonus points if you squash it and rebase yourself.

3d/graphics_settings/settings.gd Show resolved Hide resolved
3d/graphics_settings/settings.gd Outdated Show resolved Hide resolved
Could possible fix #600.
This demo has some of the basic graphical settings that game devs would want. I tried to keep it minimal because it's just a demo. I hope this is sufficient and that there are enough comments explaining everything. Everything works without any bugs or errors.
Small fix

Small script format fix
Small fix


Small fix


Changed readme


Update 3d/3d_settings_menu/project.godot

Co-authored-by: Aaron Franke <[email protected]>
Fixing many mistakes

Changed folder name
Deleted git files
changed the default size with the project settings variant
Removed some debug info


Added extra features

Changed the layout into sections,
Added some extra settings and features.
Waiting for Calinou to see what he would like to change/add.
Small fixes

Getting the start viewport in ready
@voylin
Copy link
Contributor Author

voylin commented Apr 6, 2022

Overall this looks great, the code is well-documented. I did encounter some bugs but I think they're all Godot bugs, not bugs with this project. I have a suggestion for getting the viewport size from project settings, and then this can be squashed and merged. Bonus points if you squash it and rebase yourself.

I think I earned myself some bonus points. ^o^

Copy link
Member

@aaronfranke aaronfranke left a comment

Choose a reason for hiding this comment

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

I think I earned myself some bonus points. ^o^

Well, you squashed, but did not rebase. The latter is optional but does make the Git history slightly easier to read.

@aaronfranke aaronfranke merged commit aeba3a0 into godotengine:4.0-dev Apr 6, 2022
@aaronfranke
Copy link
Member

Thanks!

@voylin voylin deleted the Add_3D_Settings_Demo branch April 6, 2022 03:32
@Calinou
Copy link
Member

Calinou commented Apr 8, 2022

This demo crashes as soon as I start it, both on 4.0alpha6 and latest master as of writing:

================================================================
handle_crash: Program crashed with signal 11
Engine version: Godot Engine v4.0.alpha.custom_build (bf153b82c73755bf898df9de33ac2116ebe914b9)
Dumping the backtrace. Please include this when reporting the bug on https://github.com/godotengine/godot/issues
[1] /lib64/libc.so.6(+0x3d320) [0x7fc7a0c0e320] (??:0)
[2] Object::get(StringName const&, bool*) const (/home/hugo/Documents/Git/godotengine/godot/core/object/object.cpp:467)
[3] ProjectSettings::get_setting(String const&) const (/home/hugo/Documents/Git/godotengine/godot/core/config/project_settings.cpp:1117)
[4] void call_with_variant_args_retc_helper<__UnexistingClass, Variant, String const&, 0ul>(__UnexistingClass*, Variant (__UnexistingClass::*)(String const&) const, Variant const**, Variant&, Callable::CallError&, IndexSequence<0ul>) (/home/hugo/Documents/Git/godotengine/godot/./core/variant/binder_common.h:705)
[5] void call_with_variant_args_retc_dv<__UnexistingClass, Variant, String const&>(__UnexistingClass*, Variant (__UnexistingClass::*)(String const&) const, Variant const**, int, Variant&, Callable::CallError&, Vector<Variant> const&) (/home/hugo/Documents/Git/godotengine/godot/./core/variant/binder_common.h:501)
[6] MethodBindTRC<Variant, String const&>::call(Object*, Variant const**, int, Callable::CallError&) (/home/hugo/Documents/Git/godotengine/godot/./core/object/method_bind.h:582)
[7] GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Callable::CallError&, GDScriptFunction::CallState*) (/home/hugo/Documents/Git/godotengine/godot/modules/gdscript/gdscript_vm.cpp:1742)
[8] GDScriptInstance::callp(StringName const&, Variant const**, int, Callable::CallError&) (/home/hugo/Documents/Git/godotengine/godot/modules/gdscript/gdscript.cpp:1539)
[9] bool Node::_gdvirtual__ready_call<false>() (/home/hugo/Documents/Git/godotengine/godot/./scene/main/node.h:233)
[10] Node::_notification(int) (/home/hugo/Documents/Git/godotengine/godot/scene/main/node.cpp:155)
[11] Node::_notificationv(int, bool) (/home/hugo/Documents/Git/godotengine/godot/./scene/main/node.h:45)
[12] CanvasItem::_notificationv(int, bool) (/home/hugo/Documents/Git/godotengine/godot/./scene/main/canvas_item.h:?)
[13] Control::_notificationv(int, bool) (/home/hugo/Documents/Git/godotengine/godot/./scene/gui/control.h:?)
[14] Object::notification(int, bool) (/home/hugo/Documents/Git/godotengine/godot/core/object/object.cpp:848)
[15] Node::_propagate_ready() (/home/hugo/Documents/Git/godotengine/godot/scene/main/node.cpp:187)
[16] Node::_propagate_ready() (/home/hugo/Documents/Git/godotengine/godot/scene/main/node.cpp:178)
[17] Node::_set_tree(SceneTree*) (/home/hugo/Documents/Git/godotengine/godot/scene/main/node.cpp:?)
[18] SceneTree::initialize() (/home/hugo/Documents/Git/godotengine/godot/scene/main/scene_tree.cpp:399)
[19] OS_LinuxBSD::run() (/home/hugo/Documents/Git/godotengine/godot/platform/linuxbsd/os_linuxbsd.cpp:?)
[20] bin/godot.linuxbsd.tools.64.llvm(main+0x1c6) [0x42e5fb6] (/home/hugo/Documents/Git/godotengine/godot/platform/linuxbsd/godot_linuxbsd.cpp:68)
[21] /lib64/libc.so.6(__libc_start_main+0xd5) [0x7fc7a0bf8b75] (??:0)
[22] bin/godot.linuxbsd.tools.64.llvm(_start+0x2e) [0x42e5d2e] (??:?)
-- END OF BACKTRACE --
================================================================

The demo can be successfully opened in the editor, but it will crash instantly on start before a single frame is even rendered.

The last version that can run the demo correctly is 4.0alpha5.

@voylin
Copy link
Contributor Author

voylin commented Apr 8, 2022

Just downloaded alpha 6 to test if I have the same issue, but for me everything runs fine.

Screenshot from 2022-04-09 05-46-19

But I do get errors like these:

ERROR: Condition "!rb" is true. Returning: false
   at: render_buffers_has_volumetric_fog (servers/rendering/renderer_rd/renderer_scene_render_rd.cpp:2900)
WARNING: 4 RIDs of type "Texture" were leaked.
     at: finalize (drivers/vulkan/rendering_device_vulkan.cpp:9435)

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

Successfully merging this pull request may close these issues.

3 participants