Fixes and improvements for item image animations (#16620)

src/client/item_visuals_manager.cpp

@@ -18,14 +18,16 @@ struct ItemVisualsManager::ItemVisuals
 	AnimationInfo inventory_normal;
 	AnimationInfo inventory_overlay;
 
+	// ItemVisuals owns the frames and AnimationInfo points to them
+	std::vector<FrameSpec> frames_normal;
+	std::vector<FrameSpec> frames_overlay;
+
 	ItemVisuals() :
 		palette(nullptr)
 	{}
 
 	~ItemVisuals()
 	{
-		inventory_normal.freeFrames();
-		inventory_overlay.freeFrames();
 		if (item_mesh.mesh)
 			item_mesh.mesh->drop();
 	}
@@ -65,30 +67,18 @@ ItemVisualsManager::ItemVisuals *ItemVisualsManager::createItemVisuals( const It
 
 	ITextureSource *tsrc = client->getTextureSource();
 
-	// Create new ItemVisuals
 	auto iv = std::make_unique<ItemVisuals>();
 
-	auto populate_texture_and_animation = [tsrc](
-			const ItemImageDef &image,
-			AnimationInfo &animation)
-	{
-		int frame_length_ms = 0;
-		auto frames = std::make_unique<std::vector<FrameSpec>>();
-		if (image.name.empty()) {
-			// no-op
-		} else if (image.animation.type == TileAnimationType::TAT_NONE) {
-			frames->push_back({0, tsrc->getTexture(image.name)});
-		} else {
-			// Animated
-			// Get inventory texture frames
-			*frames = createAnimationFrames(tsrc, image.name, image.animation, frame_length_ms);
-		}
-		animation = AnimationInfo(frames.release(), frame_length_ms);
-		// `frames` are freed in `ItemVisuals::~ItemVisuals`
-	};
+	// Create inventory image textures
+	int frame_length = 0;
+	iv->frames_normal = createAnimationFrames(tsrc, inventory_image.name,
+			inventory_image.animation, frame_length);
+	iv->inventory_normal = AnimationInfo(&iv->frames_normal, frame_length);
 
-	populate_texture_and_animation(inventory_image, iv->inventory_normal);
-	populate_texture_and_animation(inventory_overlay, iv->inventory_overlay);
+	// Create inventory overlay textures
+	iv->frames_overlay = createAnimationFrames(tsrc, inventory_overlay.name,
+			inventory_overlay.animation, frame_length);
+	iv->inventory_overlay = AnimationInfo(&iv->frames_overlay, frame_length);
 
 	createItemMesh(client, def,
 			iv->inventory_normal,

src/client/item_visuals_manager.h

@@ -41,7 +41,6 @@ struct ItemVisualsManager
 	AnimationInfo *getInventoryOverlayAnimation(const ItemStack &item, Client *client) const;
 
 	// Get item mesh
-	// Once said to return nullptr if there is an inventory image, but this is wrong
 	ItemMesh *getItemMesh(const ItemStack &item, Client *client) const;
 
 	// Get item palette

src/client/tile.cpp

@@ -5,7 +5,7 @@
 #include "tile.h"
 #include <cassert>
 
-video::ITexture *AnimationInfo::getTexture(float animation_time)
+video::ITexture *AnimationInfo::getTexture(float animation_time) const
 {
 	if (getFrameCount() == 0)
 		return nullptr;

src/client/tile.h

@@ -156,19 +156,13 @@ struct AnimationInfo {
 			m_frame_length_ms(tile.animation_frame_length_ms),
 			m_frame_count(tile.animation_frame_count),
 			m_frames(tile.frames)
-	{};
+	{}
 
 	AnimationInfo(std::vector<FrameSpec> *frames, u16 frame_length_ms) :
 			m_frame_length_ms(frame_length_ms),
 			m_frame_count(frames->size()),
 			m_frames(frames)
-	{};
-
-	void freeFrames()
-	{
-		delete m_frames;
-		m_frames = nullptr;
-	}
+	{}
 
 	size_t getFrameCount() const
 	{
@@ -178,14 +172,13 @@ struct AnimationInfo {
 	void updateTexture(video::SMaterial &material, float animation_time);
 
 	// Returns nullptr if texture did not change since last time
-	video::ITexture *getTexture(float animation_time);
+	video::ITexture *getTexture(float animation_time) const;
 
 private:
 	u16 m_frame_length_ms = 0;
 	u16 m_frame_count = 1;
 
 	/// @note by default not owned by this struct
-	/// TODO. Change this to a shared pointer.
 	std::vector<FrameSpec> *m_frames = nullptr;
 };
 

src/client/wieldmesh.cpp

@@ -415,9 +415,16 @@ std::vector<FrameSpec> createAnimationFrames(ITextureSource *tsrc,
 {
 	result_frame_length_ms = 0;
 
-	if (image_name.empty() || animation.type == TileAnimationType::TAT_NONE)
+	if (image_name.empty())
 		return {};
 
+	// Still create texture if not animated
+	if (animation.type == TileAnimationType::TAT_NONE) {
+		u32 id;
+		video::ITexture *texture = tsrc->getTextureForMesh(image_name, &id);
+		return {{id, texture}};
+	}
+
 	video::ITexture *orginal_texture = tsrc->getTexture(image_name);
 	if (!orginal_texture)
 		return {};
@@ -656,8 +663,8 @@ void WieldMeshSceneNode::changeToMesh(scene::IMesh *mesh)
 }
 
 void createItemMesh(Client *client, const ItemDefinition &def,
-		AnimationInfo &animation_normal,
-		AnimationInfo &animation_overlay,
+		const AnimationInfo &animation_normal,
+		const AnimationInfo &animation_overlay,
 		ItemMesh *result)
 {
 	ITextureSource *tsrc = client->getTextureSource();
@@ -700,7 +707,9 @@ void createItemMesh(Client *client, const ItemDefinition &def,
 		case NDT_PLANTLIKE: {
 			const TileLayer &l0 = f.tiles[0].layers[0];
 			const TileLayer &l1 = f.tiles[0].layers[1];
-			mesh = getExtrudedMesh(l0.texture, l1.texture);
+			mesh = getExtrudedMesh(
+				extractTexture(f.tiledef[0], l0, tsrc),
+				extractTexture(f.tiledef[1], l1, tsrc));
 			// Add color
 			result->buffer_info.emplace_back(0, l0);
 			result->buffer_info.emplace_back(1, l1);
@@ -709,7 +718,9 @@ void createItemMesh(Client *client, const ItemDefinition &def,
 		case NDT_PLANTLIKE_ROOTED: {
 			// Use the plant tile
 			const TileLayer &l0 = f.special_tiles[0].layers[0];
-			mesh = getExtrudedMesh(l0.texture);
+			mesh = getExtrudedMesh(
+				extractTexture(f.tiledef_special[0], l0, tsrc)
+			);
 			result->buffer_info.emplace_back(0, l0);
 			break;
 		}

src/client/wieldmesh.h

@@ -187,6 +187,6 @@ scene::SMesh *getExtrudedMesh(video::ITexture *texture,
 // This is only used to initially generate an ItemMesh
 // To get the mesh, use ItemVisualsManager::getItemMesh(item, client) instead
 void createItemMesh(Client *client, const ItemDefinition &def,
-		AnimationInfo &animation_normal,
-		AnimationInfo &animation_overlay,
+		const AnimationInfo &animation_normal,
+		const AnimationInfo &animation_overlay,
 		ItemMesh *result);

src/tileanimation.h

@@ -45,5 +45,6 @@ struct TileAnimationParams
 
 	// Modifies the texture name such that it only contains the first frame
 	// If the texture_size is know (client code), getTextureModifer should be used instead
+	// This function only exists for compatibility with old clients
 	void extractFirstFrame(std::string &name) const;
 };