MontyTRC89 / Tomb-Editor

Tomb Editor
55 stars 15 forks source link

Keep RoomGeometry allocated #818

Closed Raildex closed 2 months ago

Raildex commented 2 months ago

Keeps RoomGeometry allocated for a single room.

Instead of Destroying and Recreating the RoomGeometry, this PR keeps the RoomGeometry created once per room. This has the advantage that a single geometry edit does not result in a cascade of object destructions and constructions. This also keeps the capacity of the Lists within RoomGeometry, i.e. the Lists keep their memory instead of constructed with a low capacity and then reallocating often during vertex insertion.

Raildex commented 2 months ago

@Nickelony Is legacy code determined at runtime? if not, just use a compile constant instead of propagating it in the whole call chain

Nickelony commented 2 months ago

@Nickelony Is legacy code determined at runtime? if not, just use a compile constant instead of propagating it in the whole call chain

Yes, it's determined at runtime, specifically during the conversion from legacy .PRJ to .PRJ2.

Here is current geometry code used on coastal.prj: image

Legacy code result: image

Current geometry code causes a few textures to be missing as they are evaluated later during the conversion process.

Raildex commented 2 months ago

@Nickelony Is legacy code determined at runtime? if not, just use a compile constant instead of propagating it in the whole call chain

Yes, it's determined at runtime, specifically during the conversion from legacy .PRJ to .PRJ2.

Here is current geometry code used on coastal.prj: image

Legacy code result: image

Current geometry code causes a few textures to be missing as they are evaluated later during the conversion process.

Wouldn't that mean that the conversion of the PRJ is incorrect in this case or atleast needs to be adapted into the new geometry creation process?

Nickelony commented 2 months ago

@Nickelony Is legacy code determined at runtime? if not, just use a compile constant instead of propagating it in the whole call chain

Yes, it's determined at runtime, specifically during the conversion from legacy .PRJ to .PRJ2.

Here is current geometry code used on coastal.prj: image

Legacy code result: image

Current geometry code causes a few textures to be missing as they are evaluated later during the conversion process.

Wouldn't that mean that the conversion of the PRJ is incorrect in this case or atleast needs to be adapted into the new geometry creation process?

Yes, but the code is extremely fragile and it's very hard for me to understand it, so this current placeholder needs to stay or we just completely remove the ability to import legacy PRJ files.

Nickelony commented 2 months ago

The code of this PR is fine, but sadly, there have been some discoveries made in other parts of the TE code that really don't like this implementation and are quite possibly a bad memory leak hazard. Until that has been addressed, this PR may need reverting.