Avoid const_cast in mesh_storage.h

[rune-scape]

Avoids const_cast in mesh_storage.h by changing the virtual method RendererMeshStorage::multimesh_get_aabb to match the constness of RendererMeshStorage::mesh_get_aabb, which has to do similar dirty checks

this kind of const_cast can be very nasty as the compiler might be able to choose to return aabb before its changed by _update_dirty_multimeshes (like in #93759)

[clayjohn]

Something fishy is happening here. From reading the code I would expect this would fail to compile because the definition of multimesh_get_aabb() is still const:

godot/servers/rendering/rendering_server_default.h

Line 337 in 61598c5

FUNC1RC(AABB, multimesh_get_aabb, RID)

The fact that you can just change the definition in servers/rendering/storage/mesh_storage.h makes me think that the definition in servers/rendering/rendering_server_default.h is being ignored. Which would have very bad consequences (as it is our thread safe wrapper)

[rune-scape]

FUNC1RC is defined as:

#define FUNC1RC(m_r, m_type, m_arg1)                                                \
	virtual m_r m_type(m_arg1 p1) const override {                                  \
		if (Thread::get_caller_id() != server_thread) {                             \
			m_r ret;                                                                \
			command_queue.push_and_ret(server_name, &ServerName::m_type, p1, &ret); \
			SYNC_DEBUG                                                              \
			MAIN_THREAD_SYNC_CHECK                                                  \
			return ret;                                                             \
		} else {                                                                    \
			command_queue.flush_if_pending();                                       \
			return server_name->m_type(p1);                                         \
		}                                                                           \
	}

and server_name is defined as RSG::mesh_storage which is a mutable instance of RendererMeshStorage which does not extend any class, the function definition doesnt depend on the FUNC1RC definition

i agree it doesnt make so much sense, but i dont see so much of a downside to the definitions not matching up, but const_casts like this are prone to compiler optimizations
and it lines up with the const-ness of the similar function mesh_get_aabb

[RandomShaper]

Just my two cent... In cases where one wants getters to look like they are read-only from the surface, but there's a case that needs to update some internal cache in order to return results, one can decorate the data involved with mutable. I think it'd boil down to multimesh_dirty_list in this case. We're already using that pattern throughout the codebase.

[clayjohn]

Looks fine to me.

Thank you for the clarification!

[clayjohn]

Just my two cent... In cases where one wants getters to look like they are read-only from the surface, but there's a case that needs to update some internal cache in order to return results, one can decorate the data involved with mutable. I think it'd boil down to multimesh_dirty_list in this case. We're already using that pattern throughout the codebase.

In this case it works out because nobody calls the function directly, it always goes through an intermediate layer. So from the user point of view they are calling a const function still. I agree with your suggestion in other cases though

[rune-scape]

Just my two cent... In cases where one wants getters to look like they are read-only from the surface, but there's a case that needs to update some internal cache in order to return results, one can decorate the data involved with mutable.

i would prefer it too, the reason i couldn't is because _update_dirty_multimeshes calls _multimesh_re_create_aabb which calls mesh_get_aabb which is an exposed api method that is not const

[akien-mga]

Thanks!