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

[3.4 beta 5] usability issues w/ rooms&portals and occlusion #53005

Open
Zireael07 opened this issue Sep 24, 2021 · 18 comments
Open

[3.4 beta 5] usability issues w/ rooms&portals and occlusion #53005

Zireael07 opened this issue Sep 24, 2021 · 18 comments

Comments

@Zireael07
Copy link
Contributor

Zireael07 commented Sep 24, 2021

Godot version

3.4 beta 5

System information

Linux Manjaro, GLES 3, Intel Kaby Lake

Issue description

  1. If you somehow get out of the [Perspective] (portals enabled) mode in editor, the only way back seems to be to click 'convert rooms' again - this is a bit odd, not to mention wasteful
  2. No message printed to output log if the portals feature is deactivated/unloaded at runtime due to eg. removing a node in rooms list. This is crucial, as I just spent half an hour wondering why it works in editor and not at runtime
  3. A QoL wishlist thing - to check in editor if occlusion shapes work, the only way currently seems to be to carefully watch stuff in Overdraw or Wireframe node. It would be much easier for the user if we could toggle an 'occlusion debug view' that works similarly to the "portals enabled" mode, in which what is seen is normal and what is not is wireframe (Note, this applies only to 3.x, as my understanding is that 4.0 has a separate occlusion buffer that is white and black)

EDIT 4) I think merge meshes isn't disappearing the old meshes? Some meshes are renamed to DeleteMe, others that I later see in "mergedmesh xxx" aren't... I get a million vertices with portals active on a single road, which is ... ummm...

Steps to reproduce

Trying to use portals and rooms with an existing project

For 1), it exited portals enabled when I clicked "flip portals" while well, having NO portals set up yet :P

Minimal reproduction project

Don't anything to show yet as converting to rooms&portals broke my mapgen quite spectacularly

@Zireael07
Copy link
Contributor Author

Zireael07 commented Sep 24, 2021

Spot two shadows which don't belong, aka "I didn't get around to setting cars as roaming yet"...

Screenshot_20210924_130736

In other words, meshes that get culled still cast shadows (the scene has a Directional Light and my car has two spotlights, so I don't know which one is to blame, probably the Directional Light)

Also because of 2) above I can't tell why the scripting call still doesn't give me working rooms/portals, I have to click in editor for now...

@lawnjelly
Copy link
Member

lawnjelly commented Sep 24, 2021

(1) and (2) seem related, it has probably unloaded the portal system due to editing. A lot of editing will deactivate the portal system because the runtime data is invalidated, hence the need to reconvert. There is a warning message for unloading the portals I think, but it may be deactivated in this situation (there's a fine line between producing too much spam), I'll take a look at this.

EDIT: Can confirm if you open your output window, there should be a message Portal system unloaded. when you make an edit that invalidates the room system. So there are two visual things in the editor - The [portals active] label in the top left and the output message. I did at one point think about putting e.g. a red border around the 3d window to indicate when it was active but we didn't go that far yet.

Also note there is a keyboard shortcut for rooms convert (defaults to alt C). This can be handy when editing. It is possible that we could have it reconvert automatically after some types of edit, but it would be nice to get more feedback on this first, because auto conversion could be annoying with a large map (it's ok if it takes 1/10th second, but annoying if it takes e.g. 5 seconds).

(3) I agree it would be nice, but I don't have an in depth knowledge of how the wireframe is done, and it isn't available in GLES2 afaik. So it could potentially turn out to be quite a timesink, and at the moment there are several more pressing areas I'm working on so I have to prioritize. And it is possible to view occlusion at the moment by switching to GLES3 and selecting overdraw or wireframe. If anyone else wants to work on this though it would be welcome.

(4) It's possible there could be bugs in some situations with merge meshes (I don't know how many people have tested this, and there are a lot of possible mesh scenarios), if you can make a minimal reproduction project I can investigate.

@Zireael07
Copy link
Contributor Author

I'll point to a commit in my repo as soon as I get something that doesn't break the map ;)

@lawnjelly
Copy link
Member

lawnjelly commented Sep 24, 2021

Spot two shadows which don't belong, aka "I didn't get around to setting cars as roaming yet"...

In other words, meshes that get culled still cast shadows (the scene has a Directional Light and my car has two spotlights, so I don't know which one is to blame, probably the Directional Light)

I suspect this is to be expected. Directional lights don't really occlusion cull that well as they don't have a source in a room, so if you have an occlusion error elsewhere (e.g. you haven't set a car to roaming) the shadow appearing isn't entirely unexpected, as the car is still being rendered into the shadow map.

One thing it is important to realise is that occlusion culling doesn't just happen from the camera point of view, it also happens for lights too (except not very effectively for DirectionalLights). If lights didn't occlusion cull, you had e.g. 10,000 objects, 100 showing from the camera, and 10,000 from the light, your scene would still run very slowly.

The limitations of DirectionalLights is why you are encouraged to turn them off when entering an indoor area, see:
https://docs.godotengine.org/en/3.4/tutorials/3d/portals/advanced_room_and_portal_usage.html#roomgroups

Although this may not be relevant in your case as the game seems to be completely outdoors.

Two things that are highly likely to be causing your problems on loading:

  1. You haven't yet marked objects as ROAMING in your roomlist. Objects default to STATIC, and if you start creating / deleting these after calling rooms_convert, it may unload your room system. See:
    https://docs.godotengine.org/en/3.4/tutorials/3d/portals/using_objects_in_rooms_and_portals.html#object-lifetimes

This is part of the reason for the roomlist branch, and the encouragement to place ROAMING / GLOBAL objects in a different branch to the rooms, to help avoid this situation.

This kind of thing is much easier to deal with if you start working with portals from day 1 on a game, but it is possible to retrofit, just takes some patience and experimentation. It is very possible we can tweak the engine to give more informative error messages to help with this process (especially the unloading rooms due to invalidation, I'll take a look at that now).

  1. If you are generating levels procedurally on startup, this needs to be complete before rooms_convert is called. It is possible that as far as gdscript is concerned this is 'complete', but when traversing the scene tree during there could be something waiting for a final update.

@Zireael07
Copy link
Contributor Author

Yet another nit, I am now marking cars as roaming, there is no way to mark 'all meshes in this scene'... This would be very useful for such retrofitting (and considering rooms and portals are only now available and very useful for most mid to large projects, I expect many people will be retrofitting r&p in)

@lawnjelly
Copy link
Member

lawnjelly commented Sep 24, 2021

Yet another nit, I am now marking cars as roaming, there is no way to mark 'all meshes in this scene'... This would be very useful for such retrofitting (and considering rooms and portals are only now available and very useful for most mid to large projects, I expect many people will be retrofitting r&p in)

I absolutely agree actually. I think I mentioned this on rocket chat, we could really do with a way in Godot of selecting all nodes of a particular type in a branch of the scene tree. Like right click and 'select all by type', or something like that.

I'm doing a PR to add a 'reason' string for the room system unloading message. This should be useful for helping to track down these issues and isn't very difficult to add, I'll try and put in as much info as possible. 👍

@Zireael07
Copy link
Contributor Author

Ok, managed to get rooms and portals to work (more or less - one or two roads aren't really cooperating and are completely invisible when I'm on them) w/o breaking stuff: Zireael07/FreeRoamRoguelikeRacerPrototype@3c059d7

One more issue I'm seeing: when I change rooms, the car (which is roaming) flickers briefly.

(Lack of wheels on cars is me not having set wheel meshes to roaming because they're procedural)

@lawnjelly
Copy link
Member

lawnjelly commented Sep 24, 2021

I've managed to download the branch but I have to admit I'm pretty lost to how it works, it seems to be procedurally generating some content in the map spatial but they are not showing the scene tree in the editor.

This has happened to me before, I think it could be that you have not set the owner on the procedural nodes, for some reason you need to set this for them to show in the scene tree (don't ask me I have the same problem in the past! 😁 ). I think I just set_owner to the owner of the node I'm attaching them to.

When I do a room_convert I'm getting a lot of warnings about overlapping rooms. This is probably the cause of your flickering. Rooms should not overlap, that is one of the requirements, they need to be convex hulls, and non-overlapping. If they overlap, the system has no way of determining which room a camera (or any object) should be inside.

This is detailed in the docs, I highly recommend reading through them and doing the tutorial if you haven't already:
https://docs.godotengine.org/en/3.4/tutorials/3d/portals/index.html

@Zireael07
Copy link
Contributor Author

I am stumped however as to how the rooms could overlap (just look at the map png in the project folder, they should not...). The docs, which I did read before attempting this today, suggest that I should be seeing room bounds in editor, but I am not! The only thing I see after converting rooms is the visible (normal)/not visible (wireframe) split...

If you run the project, you'll see all the nodes in the remote tree, unfortunately this doesn't allow me to check the bounds of the rooms.

Setting get_owner() meant some problems cleaning up when regenerating the level, but I think I'll have to bite that bullet finally...

@Zireael07
Copy link
Contributor Author

Nitpick: the orange (aabb extents) can cover the green bounds. Can the orange be switched off when we're viewing the green bounds? Or the green work like the red overlap zones?

@Calinou
Copy link
Member

Calinou commented Sep 24, 2021

Nitpick: the orange (aabb extents) can cover the green bounds. Can the orange be switched off when we're viewing the green bounds? Or the green work like the red overlap zones?

We'd need to modify the 3D editor in general to hide the orange selection box when specific nodes are selected:

  • Nodes that have their own extents (GIProbe and so on).
  • Nodes that don't have a "corporeal" form (Node3D without children, Position3D, etc).

This would make the 3D editor behavior similar to the 2D editor.

@Zireael07
Copy link
Contributor Author

Zireael07 commented Sep 24, 2021

What the bl***p happened here?

Screenshot_20210924_194749

Compare: the street that is visible in editor that has rectangular bounds that are basically "smallest x + smallest y to biggest x biggest y" with the curvy road in wireframe in the center of the view, that has much more complex bounds. (Basically two roads with such braindead rectangular bounds are the reason why the system has so many overlaps)

Related: can we please get the option to generate bounds not just from meshes, but also from Areas/Shapes? I already have shapes covering the roads, with a bit of leeway, to detect when I'm on road, so those would be perfect for generating fixed-up bounds for those two nasties...

EDIT: Oh, and in the process of figuring this out I had the rooms get invalidated by such random things as: switching tabs, pressing 'generate points' on one of the roads... Especially the switching tabs is egregious

@lawnjelly
Copy link
Member

Switching tabs is a new one on me for causing unloading! 😁
I'm not sure what happened there, unless you had previously made a change which caused an unload and the garbage collection had been delayed until switching tabs? Not sure. I would need a minimum reproduction project to investigate.

Generate points does sound correct to invalidate the room data (it makes internal changes which may not be obvious as a user). Generally in the editor, most things you change will invalidate the current room system. It is really there as a means of previewing whether things work correctly visibility wise - it is not a 'live edit' type system.

With your screenshot, I'm not sure, not being familiar with the game assets I don't know what they should look like to know when something is incorrect. With your irregular shaped rooms you will probably find you are better off either starting off with a MeshInstance box shape named my-bound, converting it to points and editing them manually (or just choosing a high simplify value, generating points and editing them manually).

The automatic bound generation for rooms works for simple rooms, but for freeform geometry like yours you are going to have to do some editing.
See:
https://docs.godotengine.org/en/3.4/tutorials/3d/portals/first_steps_with_rooms_and_portals.html#how-do-i-define-the-shape-and-position-of-my-room-convex-hull

@Zireael07
Copy link
Contributor Author

Yep, and I'll probably split the longer roads (since they seem to be culprits) into two rooms or more.
I'll probably have to convert the areas/shapes back to mesh instances to achieve that, though. That's why an option to use areas for bounds would be great.

@lawnjelly
Copy link
Member

lawnjelly commented Sep 24, 2021

If you run the project, you'll see all the nodes in the remote tree, unfortunately this doesn't allow me to check the bounds of the rooms.

It seems likely that without the owner being set on the nodes, they aren't considered properly in the scene tree, and therefore the gizmos also may not work, and thus the room editor and portal editor plugin (I'm not familiar with that bit of the engine). You may have to bite the bullet and fix this, it must be annoying to not be able to see the scenes properly except through remote debug! 😨

Nitpick: the orange (aabb extents) can cover the green bounds. Can the orange be switched off when we're viewing the green bounds? Or the green work like the red overlap zones?

I also agree it would be nice to be able to turn off the orange AABB extents display (there may even have been a way, I remember asking in chat, but I forgot!).

That's why an option to use areas for bounds would be great.

This probably can be done, I'll try and remember to have a look tomorrow.
EDIT: Looks like converting areas to bounds would be a bit involved and should go through a proposal (it may be possible with gdscript).

@lucassene
Copy link

Just to add something to this issue:
I have a CubeMesh set for the room bounds (named "east-bound"), which worked fine a couple of times, but now any changes are being ignored by the RoomManager.
Since the changes are fairly small, I can easily move the points generated by the "Generate Points" button. The problem is, if I change the points, the only way to test the occlusion culling again is by pressing the "Convert Rooms" button, which then erases the changes I've made to the room bound.

@lawnjelly
Copy link
Member

Just to add something to this issue: I have a CubeMesh set for the room bounds (named "east-bound"), which worked fine a couple of times, but now any changes are being ignored by the RoomManager. Since the changes are fairly small, I can easily move the points generated by the "Generate Points" button. The problem is, if I change the points, the only way to test the occlusion culling again is by pressing the "Convert Rooms" button, which then erases the changes I've made to the room bound.

I'll take a look at this, this is probably something I didn't think of with a simple fix. 👍

@lawnjelly
Copy link
Member

Just to add something to this issue:
I have a CubeMesh set for the room bounds (named "east-bound"), which worked fine a couple of times, but now any changes are being ignored by the RoomManager.
Since the changes are fairly small, I can easily move the points generated by the "Generate Points" button. The problem is, if I change the points, the only way to test the occlusion culling again is by pressing the "Convert Rooms" button, which then erases the changes I've made to the room bound.

I'm just having a look at this, I can't seem to replicate / not sure exactly what you mean. If you are able to create a new issue for this with a minimum reproduction project and some example steps I can see if I can work out what isn't working for you. 👍

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