Skip to content

Commit

Permalink
display3d.cpp: don't use heap allocation for buildBlueprint()
Browse files Browse the repository at this point in the history
This function is called for every frame, so to avoid
constantly inserting/deleting to/from `GlobalStructContainer()`,
just construct the pseudo-structure for the blueprint on stack.

This seems to work well enough, meaning that `mouseTarget()`,
which, in turn, calls `getTileBlueprintStructure()`,
doesn't do anything horribly wrong.

Signed-off-by: Pavel Solodovnikov <pavel.al.solodovnikov@gmail.com>
  • Loading branch information
ManManson authored and past-due committed Mar 27, 2024
1 parent 7698008 commit 151093d
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 19 deletions.
30 changes: 19 additions & 11 deletions src/display3d.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -352,19 +352,17 @@ struct Blueprint
{
return compare(b) == 0;
}
STRUCTURE *buildBlueprint() const ///< Must delete after use.
optional<STRUCTURE> buildBlueprint() const
{
return ::buildBlueprint(stats, pos, dir, index, state, player);
}
void renderBlueprint(const glm::mat4 &viewMatrix, const glm::mat4 &perspectiveViewMatrix) const
{
if (clipXY(pos.x, pos.y))
{
STRUCTURE *psStruct = buildBlueprint();
ASSERT_OR_RETURN(, psStruct != nullptr, "buildBlueprint returned nullptr");
renderStructure(psStruct, viewMatrix, perspectiveViewMatrix);

objmemDestroy(psStruct, false);
auto optStruct = buildBlueprint();
ASSERT_OR_RETURN(, optStruct.has_value(), "buildBlueprint returned nullopt");
renderStructure(&*optStruct, viewMatrix, perspectiveViewMatrix);
}
}

Expand Down Expand Up @@ -688,18 +686,28 @@ static Blueprint getTileBlueprint(int mapX, int mapY)

STRUCTURE *getTileBlueprintStructure(int mapX, int mapY)
{
static STRUCTURE *psStruct = nullptr;
static optional<STRUCTURE> optStruct;

Blueprint blueprint = getTileBlueprint(mapX, mapY);
if (blueprint.state == SS_BLUEPRINT_PLANNED)
{
if (psStruct)
if (optStruct)
{
// Delete previously returned structure, if any.
objmemDestroy(psStruct, false);
optStruct.reset();
}
auto b = blueprint.buildBlueprint();
// Don't use `operator=(optional&&)` to avoid declaring custom copy/move-assignment operators
// for `STRUCTURE` (more specifically, for `SIMPLE_OBJECT`, which has some const-qualified data members).
if (b)
{
optStruct.emplace(*b);
}
else
{
optStruct.reset();
}
psStruct = blueprint.buildBlueprint();
return psStruct; // This blueprint was clicked on.
return optStruct.has_value() ? &optStruct.value() : nullptr; // This blueprint was clicked on.
}

return nullptr;
Expand Down
12 changes: 5 additions & 7 deletions src/structure.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1829,11 +1829,11 @@ STRUCTURE *buildStructureDir(STRUCTURE_STATS *pStructureType, UDWORD x, UDWORD y
return psBuilding;
}

STRUCTURE *buildBlueprint(STRUCTURE_STATS const *psStats, Vector3i pos, uint16_t direction, unsigned moduleIndex, STRUCT_STATES state, uint8_t ownerPlayer)
optional<STRUCTURE> buildBlueprint(STRUCTURE_STATS const *psStats, Vector3i pos, uint16_t direction, unsigned moduleIndex, STRUCT_STATES state, uint8_t ownerPlayer)
{
ASSERT_OR_RETURN(nullptr, psStats != nullptr, "No blueprint stats");
ASSERT_OR_RETURN(nullptr, psStats->pIMD[0] != nullptr, "No blueprint model for %s", getStatsName(psStats));
ASSERT_OR_RETURN(nullptr, ownerPlayer < MAX_PLAYERS, "invalid ownerPlayer: %" PRIu8 "", ownerPlayer);
ASSERT_OR_RETURN(nullopt, psStats != nullptr, "No blueprint stats");
ASSERT_OR_RETURN(nullopt, psStats->pIMD[0] != nullptr, "No blueprint model for %s", getStatsName(psStats));
ASSERT_OR_RETURN(nullopt, ownerPlayer < MAX_PLAYERS, "invalid ownerPlayer: %" PRIu8 "", ownerPlayer);

Rotation rot(direction, 0, 0);

Expand Down Expand Up @@ -1909,9 +1909,7 @@ STRUCTURE *buildBlueprint(STRUCTURE_STATS const *psStats, Vector3i pos, uint16_t
}
}

auto& stableBlueprint = GlobalStructContainer().emplace(std::move(blueprint));

return &stableBlueprint;
return blueprint;
}

static Vector2i defaultAssemblyPointPos(STRUCTURE *psBuilding)
Expand Down
6 changes: 5 additions & 1 deletion src/structure.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
#include "visibility.h"
#include "baseobject.h"

#include <nonstd/optional.hpp>

// how long to wait between CALL_STRUCT_ATTACKED's - plus how long to flash on radar for
#define ATTACK_CB_PAUSE 5000

Expand Down Expand Up @@ -105,7 +107,9 @@ STRUCTURE *buildStructure(STRUCTURE_STATS *pStructureType, UDWORD x, UDWORD y, U
STRUCTURE *buildStructureDir(STRUCTURE_STATS *pStructureType, UDWORD x, UDWORD y, uint16_t direction, UDWORD player, bool FromSave, uint32_t id);
STRUCTURE *buildStructureDir(STRUCTURE_STATS *pStructureType, UDWORD x, UDWORD y, uint16_t direction, UDWORD player, bool FromSave);
/// Create a blueprint structure, with just enough information to render it
STRUCTURE *buildBlueprint(STRUCTURE_STATS const *psStats, Vector3i xy, uint16_t direction, unsigned moduleIndex, STRUCT_STATES state, uint8_t ownerPlayer);
/// IMPORTANT: Do not save the reference to this instance anywhere, since it's
/// not heap-allocated and thus doesn't have a stable address!
nonstd::optional<STRUCTURE> buildBlueprint(STRUCTURE_STATS const *psStats, Vector3i xy, uint16_t direction, unsigned moduleIndex, STRUCT_STATES state, uint8_t ownerPlayer);
/* The main update routine for all Structures */
void structureUpdate(STRUCTURE *psBuilding, bool bMission);

Expand Down

0 comments on commit 151093d

Please sign in to comment.