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

Gizmos are culled after converting room in editor #50999

Closed
timothyqiu opened this issue Jul 29, 2021 · 5 comments
Closed

Gizmos are culled after converting room in editor #50999

timothyqiu opened this issue Jul 29, 2021 · 5 comments

Comments

@timothyqiu
Copy link
Member

Godot version

3.4.beta2, 3.x (5d4352f)

System information

macOS 11.4

Issue description

When using a Preview Camera in RoomManager, editor gizmos may also be culled after clicking "Convert Rooms".

This also includes the gizmo of the preview camera itself, so one couldn't move the camera around freely.

Camera gizmo before conversion:
before

Camera gizmo after conversion:
after

Steps to reproduce

  1. Setup a simple scene with camera, or use the MRP
  2. Set the camera as Preview Camera in RoomManager
  3. Click "Convert Rooms" in RoomManager
  4. Select the floor (make sure the gizmo should appear behind / beside the camera) or the camera itself

Minimal reproduction project

GizmoCull.zip

@lawnjelly
Copy link
Member

lawnjelly commented Jul 29, 2021

Yes this is expected at the moment, I did write a small note about this in the original PR : #46130 in the Main Functionality section.

When the portal system is active, everything runs through it, instead of the normal culling. This means it has to cull editor gizmos as well as game items. The default portal mode for all objects is STATIC (which makes such objects not show up), and I have been gradually going through the gizmos to set them to portal mode GLOBAL, which makes them operate in similar culling fashion to before.

This is a minor pain. The alternative would have been to set the default to GLOBAL, but then we would have the problem of beginners not realising that objects were not working correctly through the portal system (if they see them, they assume they are set correctly).

At the moment the solution is when you have converted, to switch the portal system off in the RoomManager Main-> Active section. We can try and come up with a more user friendly way of switching this, and indicating the state to the user in the editor (perhaps e.g. a red border around the 3d viewport or something, and a keyboard shortcut to turn portal active on and off). The portal rendering active in the viewport is really a 'preview' mode. It is uncharted territory really, as the editor has built in functionality for a bunch of stuff, but recognise that the portal renderer is completely taking over.

But yeah it's fine to keep this open and remind me of which objects are not showing and I'll try and get them all showing 👍 or come up with a more elegant way of handling this.

@timothyqiu
Copy link
Member Author

The alternative would have been to set the default to GLOBAL, but then we would have the problem of beginners not realising that objects were not working correctly through the portal system (if they see them, they assume they are set correctly).

Not sure if this is a proper place to say this, but I think GLOBAL might be a better default.

Switching GLOBAL to STATIC is happy because you're marking things for optimization, while switching STATIC to GLOBAL is annoying because you're fixing the broken scene.

I wonder if there could be a configuration warning when everything is GLOBAL in a Room, just like when KinematicBody is missing collision shapes.

@lawnjelly
Copy link
Member

It's a tricky one, there are pros and cons whatever we choose for default, and I'm open to changing this based on feedback, but not immediately I think we need more feedback. There's no easy way of knowing ahead of time what the best default will be, as it is all about compromise. And there's no pressure to make changes to this right away.
(The user interface items shouldn't figure into this, as that is fairly easy to change in the grand scheme of things. What is most important is the user experience.)

Pros for STATIC

  • Usually the majority of objects will be destined to be STATIC. If you place walls, props and the default is not STATIC, you are going to have to manually change the portal_mode of every one of these, which could become a chore.
  • Shows room graph errors quickly
  • Works for setting room bounds

Pros for GLOBAL

  • Makes it look as though everything is working

Cons for GLOBAL

  • Makes it look as though everything is working (when it isn't)
  • Only STATIC objects will contribute to the bound of rooms. Unless manual bound is specified for the room, rooms containing only GLOBAL objects will have invalid bounds, and will not work. This means DYNAMIC, ROAMING and autoplaced objects will not work.

One way we could attempt to satisfy everyone is to have a user-definable default portal mode that you could change (maybe in the project settings?) for new objects. Kind of like how in a paint program what you paint depends on what mode the program is in. If you are manually setting room bounds, then it does become practical to start with GLOBAL as a default. If you are not, it doesn't seem to make a lot of sense.

I wonder if there could be a configuration warning when everything is GLOBAL in a Room, just like when KinematicBody is missing collision shapes.

It is fairly easy to put in a configuration warning when a Room has no STATICS and no manual bound (in fact I'll put one, there may be one already I'll check).

Switching GLOBAL to STATIC is happy because you're marking things for optimization, while switching STATIC to GLOBAL is annoying because you're fixing the broken scene.

I do see the logic here, but what will happen currently is that users will inevitably think they are using portal system, when they are not. A lot of users don't even read manuals until they see a problem. Leaving everything as GLOBAL will actually be less efficient than not using portals at all (as there is no BVH - it is anticipating only using a small number of GLOBAL objects at runtime).

If we had some way of highlighting the GLOBAL objects in the editor this might make it slightly less of a problem (a kind of 'you haven't dealt with this object yet' glow or something).

while switching STATIC to GLOBAL is annoying because you're fixing the broken scene.

If there is a problem rendering a STATIC object that is non-moving, then it indicates a problem in the room / portal room graph. Trying to bodge around this won't work, because even if you make problem objects GLOBAL, other moving objects will probably move into the room and render incorrectly. Rooms and portals are alas not very forgiving of errors. They need to be identified and fixed asap, which is why we have alerts for overlapping areas and conversion warnings etc.

@lawnjelly
Copy link
Member

Back to the original issue, I actually had some time to properly look at it. I had expected it was a problem due to the portal_mode of the gizmo, but it turns out it is something very simple:

When the preview_camera is operational, all the 3d culling is taking place from the view of the preview_camera, and that includes the gizmos! So if you have a gizmo selected that is behind the preview_camera, it will be frustum culled! 😁

I'll see if there is anyway to differentiate debug gizmo elements in the VisualServer. The actual call is made from VisualServerScene::_cull_convex_from_point. It may be that there isn't, as the idea is the VisualServer is just a dumb thing that gets told to create meshes, and where to draw them, it may not know whether something is a gizmo or an object in the SceneTree.

@akien-mga
Copy link
Member

Fixed by #51092.

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

4 participants