From 44f9a0f96176cbf23e5293bd3e000a10a58ee9f7 Mon Sep 17 00:00:00 2001 From: lawnjelly Date: Thu, 15 Jul 2021 11:37:04 +0100 Subject: [PATCH] Portals - fix autolink sprawling, refine logs It turned out the new autolinking feature was linking portals AFTER the static meshes had been added to rooms in the PortalRenderer. This meant that large meshes weren't being sprawled across these portals. The fix involves doing the autolinking BEFORE adding the static meshes. Fixes a bug in the warning for portals being in the wrong direction, they should have only been checkout for outgoing portals. This was resulting in erroneous warnings. Also the room conversion logs are refined to be more compact and informative. A warning icon is also added in the gizmo for portals where autolink fails. --- editor/spatial_editor_gizmos.cpp | 2 +- scene/3d/portal.h | 1 + scene/3d/room_manager.cpp | 143 ++++++++++++++++----- scene/3d/room_manager.h | 4 +- servers/visual/portals/portal_renderer.cpp | 50 ++++--- servers/visual/portals/portal_renderer.h | 6 +- 6 files changed, 150 insertions(+), 56 deletions(-) diff --git a/editor/spatial_editor_gizmos.cpp b/editor/spatial_editor_gizmos.cpp index c0386809626..9a29d4a2968 100644 --- a/editor/spatial_editor_gizmos.cpp +++ b/editor/spatial_editor_gizmos.cpp @@ -4546,7 +4546,7 @@ void PortalGizmoPlugin::redraw(EditorSpatialGizmo *p_gizmo) { if (portal) { // warnings - if (portal->_warning_outside_room_aabb || portal->_warning_facing_wrong_way) { + if (portal->_warning_outside_room_aabb || portal->_warning_facing_wrong_way || portal->_warning_autolink_failed) { Ref icon = get_material("portal_icon", p_gizmo); p_gizmo->add_unscaled_billboard(icon, 0.05); } diff --git a/scene/3d/portal.h b/scene/3d/portal.h index 2fbddae05d2..2265109b7ef 100644 --- a/scene/3d/portal.h +++ b/scene/3d/portal.h @@ -158,6 +158,7 @@ private: // warnings bool _warning_outside_room_aabb = false; bool _warning_facing_wrong_way = false; + bool _warning_autolink_failed = false; #endif // this is read from the gizmo diff --git a/scene/3d/room_manager.cpp b/scene/3d/room_manager.cpp index 549b0568e6a..a81bb4752a3 100644 --- a/scene/3d/room_manager.cpp +++ b/scene/3d/room_manager.cpp @@ -582,11 +582,17 @@ void RoomManager::rooms_convert() { _second_pass_rooms(roomgroups, portals); // third pass + // autolink portals that are not already manually linked // and finalize the portals _third_pass_portals(_roomlist, portals); - // finalize the room hulls + // Find the statics AGAIN and only this time add them to the PortalRenderer. + // We need to do this twice because the room points determine the room bound... + // but the bound is needed for autolinking, + // and the autolinking needs to be done BEFORE adding to the PortalRenderer so that + // the static objects will correctly sprawl. It is a chicken and egg situation. + // Also finalize the room hulls. _third_pass_rooms(portals); bool generate_pvs = false; @@ -658,7 +664,9 @@ void RoomManager::_second_pass_room(Room *p_room, const LocalVector } else if (_name_starts_with(child, "Bound", true)) { manual_bound_found = _convert_manual_bound(p_room, child, p_portals); } else { - _find_statics_recursive(p_room, child, room_pts); + // don't add the instances to the portal renderer on the first pass of _find_statics, + // just find the geometry points in order to make sure the bound is correct. + _find_statics_recursive(p_room, child, room_pts, false); } } } @@ -687,6 +695,11 @@ void RoomManager::_second_pass_room(Room *p_room, const LocalVector int portal_id = p_room->_portals[n]; Portal *portal = p_portals[portal_id]; + // only checking portals out from source room + if (portal->_linkedroom_ID[0] != p_room->_room_ID) { + continue; + } + // don't add portals to the world bound that are internal to this room! if (portal->is_portal_internal(p_room->_room_ID)) { continue; @@ -781,6 +794,26 @@ void RoomManager::_third_pass_rooms(const LocalVector &p_portals) { for (int n = 0; n < _rooms.size(); n++) { Room *room = _rooms[n]; + + String room_short_name = _find_name_after(room, "ROOM"); + convert_log("ROOM\t" + room_short_name); + + // log output the portals associated with this room + for (int p = 0; p < room->_portals.size(); p++) { + const Portal &portal = *p_portals[room->_portals[p]]; + + String in_or_out = (portal._linkedroom_ID[0] == room->_room_ID) ? "POUT" : "PIN "; + convert_log("\t\t" + in_or_out + "\t" + portal.get_name()); + } + + // do a second pass finding the statics, where they are + // finally added to the rooms in the portal_renderer. + Vector room_pts; + + // the true indicates we DO want to add to the portal renderer this second time + // we call _find_statics_recursive + _find_statics_recursive(room, room, room_pts, true); + if (!_convert_room_hull_final(room, p_portals)) { found_errors = true; } @@ -900,7 +933,7 @@ void RoomManager::_third_pass_portals(Spatial *p_roomlist, LocalVector if (!outside) { // great, we found a linked room! - convert_log("\tauto linked portal from room " + source_room->get_name() + " to room " + room->get_name(), 1); + convert_log("\t\tAUTOLINK OK from " + source_room->get_name() + " to " + room->get_name(), 1); portal->_linkedroom_ID[1] = r; // add the portal to the portals list for the receiving room @@ -919,8 +952,13 @@ void RoomManager::_third_pass_portals(Spatial *p_roomlist, LocalVector // error condition if (!autolink_found) { - WARN_PRINT("Portal autolink failed for Portal from " + source_room->get_name()); + WARN_PRINT("Portal AUTOLINK failed for " + portal->get_name() + " from " + source_room->get_name()); _warning_portal_autolink_failed = true; + +#ifdef TOOLS_ENABLED + portal->_warning_autolink_failed = true; + portal->update_gizmo(); +#endif } } // for portal } @@ -1040,8 +1078,6 @@ void RoomManager::_convert_room(Spatial *p_node, LocalVector &r_portal } } - convert_log("convert_room : " + string_full_name, 1); - // make sure the room is blank, especially if already created room->clear(); @@ -1087,8 +1123,11 @@ void RoomManager::_check_portal_for_warnings(Portal *p_portal, const AABB &p_roo // far outside the room? const Vector3 &pos = p_portal->get_global_transform().origin; - if (p_portal->_warning_outside_room_aabb != (!bb.has_point(pos))) { - p_portal->_warning_outside_room_aabb = !p_portal->_warning_outside_room_aabb; + + bool old_outside = p_portal->_warning_outside_room_aabb; + p_portal->_warning_outside_room_aabb = !bb.has_point(pos); + + if (p_portal->_warning_outside_room_aabb != old_outside) { changed = true; } @@ -1099,8 +1138,11 @@ void RoomManager::_check_portal_for_warnings(Portal *p_portal, const AABB &p_roo // facing wrong way? Vector3 offset = pos - bb.get_center(); real_t dot = offset.dot(p_portal->_plane.normal); - if (p_portal->_warning_facing_wrong_way != (dot < 0.0)) { - p_portal->_warning_facing_wrong_way = !p_portal->_warning_facing_wrong_way; + + bool old_facing = p_portal->_warning_facing_wrong_way; + p_portal->_warning_facing_wrong_way = dot < 0.0; + + if (p_portal->_warning_facing_wrong_way != old_facing) { changed = true; } @@ -1108,13 +1150,22 @@ void RoomManager::_check_portal_for_warnings(Portal *p_portal, const AABB &p_roo WARN_PRINT(String(p_portal->get_name()) + " possibly facing the wrong way."); } + // handled later + p_portal->_warning_autolink_failed = false; + if (changed) { p_portal->update_gizmo(); } #endif } -void RoomManager::_find_statics_recursive(Room *p_room, Spatial *p_node, Vector &r_room_pts) { +void RoomManager::_find_statics_recursive(Room *p_room, Spatial *p_node, Vector &r_room_pts, bool p_add_to_portal_renderer) { + // don't process portal MeshInstances that are being deleted + // (and replaced by proper Portal nodes) + if (p_node->is_queued_for_deletion()) { + return; + } + bool ignore = false; VisualInstance *vi = Object::cast_to(p_node); @@ -1131,21 +1182,36 @@ void RoomManager::_find_statics_recursive(Room *p_room, Spatial *p_node, Vector< } if (!ignore) { - // lights - Light *light = Object::cast_to(p_node); - if (light) { - convert_log("\tfound Light " + light->get_name()); + // We'll have a done flag. This isn't strictly speaking necessary + // because the types should be mutually exclusive, but this would + // break if something changes the inheritance hierarchy of the nodes + // at a later date, so having a done flag makes it more robust. + bool done = false; - Vector dummy_pts; - VisualServer::get_singleton()->room_add_instance(p_room->_room_rid, light->get_instance(), light->get_transformed_aabb(), dummy_pts); + Light *light = Object::cast_to(p_node); + + if (!done && light) { + done = true; + + // lights (don't affect bound, so aren't added in first pass) + if (p_add_to_portal_renderer) { + Vector dummy_pts; + VisualServer::get_singleton()->room_add_instance(p_room->_room_rid, light->get_instance(), light->get_transformed_aabb(), dummy_pts); + convert_log("\t\t\tLIGT\t" + light->get_name()); + } } GeometryInstance *gi = Object::cast_to(p_node); - if (gi) { + if (!done && gi) { + done = true; + + // MeshInstance is the most interesting type for portalling, so we handle this explicitly MeshInstance *mi = Object::cast_to(p_node); if (mi) { - convert_log("\tfound MeshInst " + mi->get_name()); + if (p_add_to_portal_renderer) { + convert_log("\t\t\tMESH\t" + mi->get_name()); + } Vector object_pts; AABB aabb; @@ -1161,15 +1227,20 @@ void RoomManager::_find_statics_recursive(Room *p_room, Spatial *p_node, Vector< r_room_pts.append_array(object_pts); } - VisualServer::get_singleton()->room_add_instance(p_room->_room_rid, mi->get_instance(), mi->get_transformed_aabb(), object_pts); + if (p_add_to_portal_renderer) { + VisualServer::get_singleton()->room_add_instance(p_room->_room_rid, mi->get_instance(), mi->get_transformed_aabb(), object_pts); + } } // if bound found points } else { - // geometry instance but not a mesh instance .. just use AABB - convert_log("\tfound GeomInst " + gi->get_name()); - // Vector dummy_pts; - // VisualServer::get_singleton()->room_add_instance(p_room->_room_rid, gi->get_instance(), gi->get_transformed_aabb(), dummy_pts); + // geometry instance but not a mesh instance .. + if (p_add_to_portal_renderer) { + convert_log("\t\t\tGEOM\t" + gi->get_name()); + } + Vector object_pts; AABB aabb; + + // attempt to recognise this GeometryInstance and read back the geometry if (_bound_findpoints_geom_instance(gi, object_pts, aabb)) { // need to keep track of room bound // NOTE the is_visible check MAY cause problems if conversion run on nodes that @@ -1179,17 +1250,22 @@ void RoomManager::_find_statics_recursive(Room *p_room, Spatial *p_node, Vector< r_room_pts.append_array(object_pts); } - VisualServer::get_singleton()->room_add_instance(p_room->_room_rid, gi->get_instance(), gi->get_transformed_aabb(), object_pts); + if (p_add_to_portal_renderer) { + VisualServer::get_singleton()->room_add_instance(p_room->_room_rid, gi->get_instance(), gi->get_transformed_aabb(), object_pts); + } } // if bound found points } } // if gi VisibilityNotifier *vn = Object::cast_to(p_node); - if (vn && ((vn->get_portal_mode() == CullInstance::PORTAL_MODE_DYNAMIC) || (vn->get_portal_mode() == CullInstance::PORTAL_MODE_STATIC))) { - convert_log("\tfound VisibilityNotifier " + vn->get_name()); + if (!done && vn && ((vn->get_portal_mode() == CullInstance::PORTAL_MODE_DYNAMIC) || (vn->get_portal_mode() == CullInstance::PORTAL_MODE_STATIC))) { + done = true; - AABB world_aabb = vn->get_global_transform().xform(vn->get_aabb()); - VisualServer::get_singleton()->room_add_ghost(p_room->_room_rid, vn->get_instance_id(), world_aabb); + if (p_add_to_portal_renderer) { + AABB world_aabb = vn->get_global_transform().xform(vn->get_aabb()); + VisualServer::get_singleton()->room_add_ghost(p_room->_room_rid, vn->get_instance_id(), world_aabb); + convert_log("\t\t\tVIS \t" + vn->get_name()); + } } } // if not ignore @@ -1198,7 +1274,7 @@ void RoomManager::_find_statics_recursive(Room *p_room, Spatial *p_node, Vector< Spatial *child = Object::cast_to(p_node->get_child(n)); if (child) { - _find_statics_recursive(p_room, child, r_room_pts); + _find_statics_recursive(p_room, child, r_room_pts, p_add_to_portal_renderer); } } } @@ -1353,7 +1429,9 @@ bool RoomManager::_convert_room_hull_final(Room *p_room, const LocalVector_planes, num_portals_added); - convert_log("\t\t\tcontained " + itos(num_planes_before_simplification) + " planes before simplification, " + itos(p_room->_planes.size()) + " planes after."); + if (num_planes_before_simplification != p_room->_planes.size()) { + convert_log("\t\t\tcontained " + itos(num_planes_before_simplification) + " planes before simplification, " + itos(p_room->_planes.size()) + " planes after."); + } // make a copy of the mesh data for debugging // note this could be avoided in release builds? NYI @@ -1455,9 +1533,6 @@ bool RoomManager::_add_plane_if_unique(const Room *p_room, LocalVector &portals) { - String string_full_name = p_node->get_name(); - convert_log("convert_portal : " + string_full_name); - Portal *portal = Object::cast_to(p_node); // if not a gportal already, convert the node type diff --git a/scene/3d/room_manager.h b/scene/3d/room_manager.h index d60cb63def9..a4aeb56f45d 100644 --- a/scene/3d/room_manager.h +++ b/scene/3d/room_manager.h @@ -157,7 +157,7 @@ private: bool _convert_manual_bound(Room *p_room, Spatial *p_node, const LocalVector &p_portals); void _check_portal_for_warnings(Portal *p_portal, const AABB &p_room_aabb_without_portals); - void _find_statics_recursive(Room *p_room, Spatial *p_node, Vector &r_room_pts); + void _find_statics_recursive(Room *p_room, Spatial *p_node, Vector &r_room_pts, bool p_add_to_portal_renderer); bool _convert_room_hull_preliminary(Room *p_room, const Vector &p_room_pts, const LocalVector &p_portals); bool _bound_findpoints_mesh_instance(MeshInstance *p_mi, Vector &r_room_pts, AABB &r_aabb); @@ -201,7 +201,7 @@ private: Error _build_convex_hull(const Vector &p_points, Geometry::MeshData &r_mesh, real_t p_epsilon = 3.0 * UNIT_EPSILON); // output strings during conversion process - void convert_log(String p_string, int p_priority = 0) { debug_print_line(p_string, p_priority); } + void convert_log(String p_string, int p_priority = 0) { debug_print_line(p_string, 1); } // only prints when user has set 'debug' in the room manager inspector // also does not show in non editor builds diff --git a/servers/visual/portals/portal_renderer.cpp b/servers/visual/portals/portal_renderer.cpp index 5a15901e044..dc774c262d1 100644 --- a/servers/visual/portals/portal_renderer.cpp +++ b/servers/visual/portals/portal_renderer.cpp @@ -315,7 +315,7 @@ void PortalRenderer::portal_link(PortalHandle p_portal, RoomHandle p_room_from, room_to._contains_internal_rooms = true; } - _log("portal_link from room " + itos(room_from._room_ID) + " to room " + itos(room_to._room_ID)); + // _log("portal_link from room " + itos(room_from._room_ID) + " to room " + itos(room_to._room_ID)); room_from._portal_ids.push_back(portal._portal_id); @@ -510,7 +510,9 @@ OcclusionHandle PortalRenderer::room_add_ghost(RoomHandle p_room, ObjectID p_obj // create a bitfield to indicate which rooms have been // visited already, to prevent visiting rooms multiple times _bitfield_rooms.blank(); - sprawl_static_ghost(ghost_id, p_aabb, p_room); + if (sprawl_static_ghost(ghost_id, p_aabb, p_room)) { + _log("\t\tSPRAWLED"); + } } return OCCLUSION_HANDLE_ROOM_BIT; @@ -547,9 +549,13 @@ OcclusionHandle PortalRenderer::room_add_instance(RoomHandle p_room, RID p_insta _bitfield_rooms.blank(); if (p_object_pts.size()) { - sprawl_static_geometry(static_id, st, st.source_room_id, p_object_pts); + if (sprawl_static_geometry(static_id, st, st.source_room_id, p_object_pts)) { + _log("\t\tSPRAWLED"); + } } else { - sprawl_static(static_id, st, st.source_room_id); + if (sprawl_static(static_id, st, st.source_room_id)) { + _log("\t\tSPRAWLED"); + } } } @@ -701,14 +707,16 @@ void PortalRenderer::rooms_finalize(bool p_generate_pvs, bool p_cull_using_pvs, print_line("Room conversion complete. " + itos(_room_pool_ids.size()) + " rooms, " + itos(_portal_pool_ids.size()) + " portals."); } -void PortalRenderer::sprawl_static_geometry(int p_static_id, const VSStatic &p_static, int p_room_id, const Vector &p_object_pts) { +bool PortalRenderer::sprawl_static_geometry(int p_static_id, const VSStatic &p_static, int p_room_id, const Vector &p_object_pts) { // set, and if room already done, ignore if (!_bitfield_rooms.check_and_set(p_room_id)) - return; + return false; VSRoom &room = get_room(p_room_id); room._static_ids.push_back(p_static_id); + bool sprawled = false; + // go through portals for (int p = 0; p < room._portal_ids.size(); p++) { const VSPortal &portal = get_portal(room._portal_ids[p]); @@ -716,22 +724,26 @@ void PortalRenderer::sprawl_static_geometry(int p_static_id, const VSStatic &p_s int room_to_id = portal.geometry_crosses_portal(p_room_id, p_static.aabb, p_object_pts); if (room_to_id != -1) { - _log(String(Variant(p_static.aabb)) + " crosses portal"); - + // _log(String(Variant(p_static.aabb)) + " crosses portal"); sprawl_static_geometry(p_static_id, p_static, room_to_id, p_object_pts); + sprawled = true; } } + + return sprawled; } -void PortalRenderer::sprawl_static_ghost(int p_ghost_id, const AABB &p_aabb, int p_room_id) { +bool PortalRenderer::sprawl_static_ghost(int p_ghost_id, const AABB &p_aabb, int p_room_id) { // set, and if room already done, ignore if (!_bitfield_rooms.check_and_set(p_room_id)) { - return; + return false; } VSRoom &room = get_room(p_room_id); room._static_ghost_ids.push_back(p_ghost_id); + bool sprawled = false; + // go through portals for (int p = 0; p < room._portal_ids.size(); p++) { const VSPortal &portal = get_portal(room._portal_ids[p]); @@ -739,22 +751,26 @@ void PortalRenderer::sprawl_static_ghost(int p_ghost_id, const AABB &p_aabb, int int room_to_id = portal.crosses_portal(p_room_id, p_aabb, true); if (room_to_id != -1) { - _log(String(Variant(p_aabb)) + " crosses portal"); - + // _log(String(Variant(p_aabb)) + " crosses portal"); sprawl_static_ghost(p_ghost_id, p_aabb, room_to_id); + sprawled = true; } } + + return sprawled; } -void PortalRenderer::sprawl_static(int p_static_id, const VSStatic &p_static, int p_room_id) { +bool PortalRenderer::sprawl_static(int p_static_id, const VSStatic &p_static, int p_room_id) { // set, and if room already done, ignore if (!_bitfield_rooms.check_and_set(p_room_id)) { - return; + return false; } VSRoom &room = get_room(p_room_id); room._static_ids.push_back(p_static_id); + bool sprawled = false; + // go through portals for (int p = 0; p < room._portal_ids.size(); p++) { const VSPortal &portal = get_portal(room._portal_ids[p]); @@ -762,11 +778,13 @@ void PortalRenderer::sprawl_static(int p_static_id, const VSStatic &p_static, in int room_to_id = portal.crosses_portal(p_room_id, p_static.aabb, true); if (room_to_id != -1) { - _log(String(Variant(p_static.aabb)) + " crosses portal"); - + // _log(String(Variant(p_static.aabb)) + " crosses portal"); sprawl_static(p_static_id, p_static, room_to_id); + sprawled = true; } } + + return sprawled; } void PortalRenderer::_load_finalize_roaming() { diff --git a/servers/visual/portals/portal_renderer.h b/servers/visual/portals/portal_renderer.h index 206e2bb3490..219067b176b 100644 --- a/servers/visual/portals/portal_renderer.h +++ b/servers/visual/portals/portal_renderer.h @@ -221,9 +221,9 @@ private: return _rooms_lookup_bsp.find_room_within(*this, p_pos, p_previous_room_id); } - void sprawl_static(int p_static_id, const VSStatic &p_static, int p_room_id); - void sprawl_static_geometry(int p_static_id, const VSStatic &p_static, int p_room_id, const Vector &p_object_pts); - void sprawl_static_ghost(int p_ghost_id, const AABB &p_aabb, int p_room_id); + bool sprawl_static(int p_static_id, const VSStatic &p_static, int p_room_id); + bool sprawl_static_geometry(int p_static_id, const VSStatic &p_static, int p_room_id, const Vector &p_object_pts); + bool sprawl_static_ghost(int p_ghost_id, const AABB &p_aabb, int p_room_id); void _load_finalize_roaming(); void sprawl_roaming(uint32_t p_mover_pool_id, MovingBase &r_moving, int p_room_id, bool p_moving_or_ghost);