From b50cf8f252dc117a89352ca68afbbbbab520c6d9 Mon Sep 17 00:00:00 2001 From: Rubidium Date: Sun, 23 Feb 2025 09:25:26 +0100 Subject: [PATCH] Codechange: rework OpenGL backend to not need emplacement new and manual deconstructor calls --- src/misc/lrucache.hpp | 51 +++++++++++------------------- src/video/opengl.cpp | 73 ++++++++++++++++++------------------------- src/video/opengl.h | 12 +++++-- 3 files changed, 60 insertions(+), 76 deletions(-) diff --git a/src/misc/lrucache.hpp b/src/misc/lrucache.hpp index 0ff9db663a..b5f262a3a7 100644 --- a/src/misc/lrucache.hpp +++ b/src/misc/lrucache.hpp @@ -21,10 +21,10 @@ template class LRUCache { private: - typedef std::pair Tpair; + typedef std::pair> Tpair; typedef typename std::list::iterator Titer; - std::list data; ///< Ordered list of all items. + std::list data; ///< Ordered list of all items. std::unordered_map lookup; ///< Map of keys to items. const size_t capacity; ///< Number of items to cache. @@ -50,46 +50,33 @@ public: * Insert a new data item with a specified key. * @param key Key under which the item should be stored. * @param item Item to insert. - * @return Evicted item or nullptr, if no item had to be evicted. */ - Tdata *Insert(const Tkey key, Tdata *item) + void Insert(const Tkey key, std::unique_ptr &&item) { - Tdata *old = nullptr; - if (this->Contains(key)) { /* Replace old value. */ - old = this->lookup[key]->second; - this->lookup[key]->second = item; - } else { - /* Delete least used item if maximum items cached. */ - if (this->data.size() >= this->capacity) { - Tpair last = data.back(); - this->lookup.erase(last.first); - this->data.pop_back(); - - old = last.second; - } - - /* Insert new item. */ - this->data.emplace_front(key, item); - this->lookup.emplace(key, this->data.begin()); + this->lookup[key]->second = std::move(item); + return; } - return old; + /* Delete least used item if maximum items cached. */ + if (this->data.size() >= this->capacity) { + this->lookup.erase(data.back().first); + this->data.pop_back(); + } + + /* Insert new item. */ + this->data.emplace_front(key, std::move(item)); + this->lookup.emplace(key, this->data.begin()); } /** - * Pop the least recently used item. - * @return The item value or nullptr if no items cached. + * Clear the cache. */ - inline Tdata *Pop() + void Clear() { - if (this->data.empty()) return nullptr; - - Tdata *value = this->data.back().second; - this->lookup.erase(this->data.back().first); - this->data.pop_back(); - return value; + this->lookup.clear(); + this->data.clear(); } /** @@ -104,7 +91,7 @@ public: /* Move to front if needed. */ this->data.splice(this->data.begin(), this->data, this->lookup[key]); - return this->data.front().second; + return this->data.front().second.get(); } }; diff --git a/src/video/opengl.cpp b/src/video/opengl.cpp index cca460930d..362636a832 100644 --- a/src/video/opengl.cpp +++ b/src/video/opengl.cpp @@ -1077,9 +1077,9 @@ void OpenGLBackend::DrawMouseCursor() for (const auto &cs : this->cursor_sprites) { /* Sprites are cached by PopulateCursorCache(). */ if (this->cursor_cache.Contains(cs.image.sprite)) { - Sprite *spr = this->cursor_cache.Get(cs.image.sprite); + OpenGLSprite *spr = this->cursor_cache.Get(cs.image.sprite); - this->RenderOglSprite((OpenGLSprite *)spr->data, cs.image.pal, + this->RenderOglSprite(spr, cs.image.pal, this->cursor_pos.x + cs.pos.x + UnScaleByZoom(spr->x_offs, ZOOM_LVL_GUI), this->cursor_pos.y + cs.pos.y + UnScaleByZoom(spr->y_offs, ZOOM_LVL_GUI), ZOOM_LVL_GUI); @@ -1087,6 +1087,16 @@ void OpenGLBackend::DrawMouseCursor() } } +class OpenGLSpriteAllocator : public SpriteAllocator { +public: + LRUCache &lru; + SpriteID sprite; + + OpenGLSpriteAllocator(LRUCache &lru, SpriteID sprite) : lru(lru), sprite(sprite) {} +protected: + void *AllocatePtr(size_t) override { NOT_REACHED(); } +}; + void OpenGLBackend::PopulateCursorCache() { if (this->clear_cursor_cache) { @@ -1105,13 +1115,8 @@ void OpenGLBackend::PopulateCursorCache() this->cursor_sprites.emplace_back(sc); if (!this->cursor_cache.Contains(sc.image.sprite)) { - SimpleSpriteAllocator allocator; - Sprite *old = this->cursor_cache.Insert(sc.image.sprite, static_cast(GetRawSprite(sc.image.sprite, SpriteType::Normal, &allocator, this))); - if (old != nullptr) { - OpenGLSprite *gl_sprite = (OpenGLSprite *)old->data; - gl_sprite->~OpenGLSprite(); - free(old); - } + OpenGLSpriteAllocator allocator(this->cursor_cache, sc.image.sprite); + GetRawSprite(sc.image.sprite, SpriteType::Normal, &allocator, this); } } } @@ -1121,12 +1126,7 @@ void OpenGLBackend::PopulateCursorCache() */ void OpenGLBackend::InternalClearCursorCache() { - Sprite *sp; - while ((sp = this->cursor_cache.Pop()) != nullptr) { - OpenGLSprite *sprite = (OpenGLSprite *)sp->data; - sprite->~OpenGLSprite(); - free(sp); - } + this->cursor_cache.Clear(); } /** @@ -1260,23 +1260,11 @@ void OpenGLBackend::ReleaseAnimBuffer(const Rect &update_rect) /* virtual */ Sprite *OpenGLBackend::Encode(const SpriteLoader::SpriteCollection &sprite, SpriteAllocator &allocator) { - /* Allocate and construct sprite data. */ - Sprite *dest_sprite = allocator.Allocate(sizeof(*dest_sprite) + sizeof(OpenGLSprite)); + /* This encoding is only called for mouse cursors. We don't need real sprites but OpenGLSprites to show as cursor. These need to be put in the LRU cache. */ + OpenGLSpriteAllocator &gl_allocator = static_cast(allocator); + gl_allocator.lru.Insert(gl_allocator.sprite, std::make_unique(sprite)); - OpenGLSprite *gl_sprite = (OpenGLSprite *)dest_sprite->data; - new (gl_sprite) OpenGLSprite(sprite[ZOOM_LVL_MIN].width, sprite[ZOOM_LVL_MIN].height, sprite[ZOOM_LVL_MIN].type == SpriteType::Font ? 1 : ZOOM_LVL_END, sprite[ZOOM_LVL_MIN].colours); - - /* Upload texture data. */ - for (int i = 0; i < (sprite[ZOOM_LVL_MIN].type == SpriteType::Font ? 1 : ZOOM_LVL_END); i++) { - gl_sprite->Update(sprite[i].width, sprite[i].height, i, sprite[i].data); - } - - dest_sprite->height = sprite[ZOOM_LVL_MIN].height; - dest_sprite->width = sprite[ZOOM_LVL_MIN].width; - dest_sprite->x_offs = sprite[ZOOM_LVL_MIN].x_offs; - dest_sprite->y_offs = sprite[ZOOM_LVL_MIN].y_offs; - - return dest_sprite; + return nullptr; } /** @@ -1407,27 +1395,23 @@ void OpenGLBackend::RenderOglSprite(OpenGLSprite *gl_sprite, PaletteID pal, int /** * Create an OpenGL sprite with a palette remap part. - * @param width Width of the top-level texture. - * @param height Height of the top-level texture. - * @param levels Number of mip-map levels. - * @param components Indicates which sprite components are used. + * @param sprite The sprite to create the OpenGL sprite for */ -OpenGLSprite::OpenGLSprite(uint width, uint height, uint levels, SpriteComponents components) +OpenGLSprite::OpenGLSprite(const SpriteLoader::SpriteCollection &sprite) : + dim(sprite[ZOOM_LVL_MIN].width, sprite[ZOOM_LVL_MIN].height), x_offs(sprite[ZOOM_LVL_MIN].x_offs), y_offs(sprite[ZOOM_LVL_MIN].y_offs) { + int levels = sprite[ZOOM_LVL_MIN].type == SpriteType::Font ? 1 : ZOOM_LVL_END; assert(levels > 0); (void)_glGetError(); - this->dim.width = width; - this->dim.height = height; - MemSetT(this->tex, 0, NUM_TEX); _glActiveTexture(GL_TEXTURE0); _glBindBuffer(GL_PIXEL_UNPACK_BUFFER, 0); for (int t = TEX_RGBA; t < NUM_TEX; t++) { /* Sprite component present? */ - if (t == TEX_RGBA && components == SpriteComponent::Palette) continue; - if (t == TEX_REMAP && !components.Test(SpriteComponent::Palette)) continue; + if (t == TEX_RGBA && sprite[ZOOM_LVL_MIN].colours == SpriteComponent::Palette) continue; + if (t == TEX_REMAP && !sprite[ZOOM_LVL_MIN].colours.Test(SpriteComponent::Palette)) continue; /* Allocate texture. */ _glGenTextures(1, &this->tex[t]); @@ -1440,7 +1424,7 @@ OpenGLSprite::OpenGLSprite(uint width, uint height, uint levels, SpriteComponent _glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE); /* Set size. */ - for (uint i = 0, w = width, h = height; i < levels; i++, w /= 2, h /= 2) { + for (int i = 0, w = this->dim.width, h = this->dim.height; i < levels; i++, w /= 2, h /= 2) { assert(w * h != 0); if (t == TEX_REMAP) { _glTexImage2D(GL_TEXTURE_2D, i, GL_R8, w, h, 0, GL_RED, GL_UNSIGNED_BYTE, nullptr); @@ -1450,6 +1434,11 @@ OpenGLSprite::OpenGLSprite(uint width, uint height, uint levels, SpriteComponent } } + /* Upload texture data. */ + for (int i = 0; i < (sprite[ZOOM_LVL_MIN].type == SpriteType::Font ? 1 : ZOOM_LVL_END); i++) { + this->Update(sprite[i].width, sprite[i].height, i, sprite[i].data); + } + assert(_glGetError() == GL_NO_ERROR); } diff --git a/src/video/opengl.h b/src/video/opengl.h index 7351462acc..24837bff63 100644 --- a/src/video/opengl.h +++ b/src/video/opengl.h @@ -59,7 +59,7 @@ private: GLint sprite_rgb_loc = 0; ///< Uniform location for RGB mode flag. GLint sprite_crash_loc = 0; ///< Uniform location for crash remap mode flag. - LRUCache cursor_cache; ///< Cache of encoded cursor sprites. + LRUCache cursor_cache; ///< Cache of encoded cursor sprites. PaletteID last_sprite_pal = (PaletteID)-1; ///< Last uploaded remap palette. bool clear_cursor_cache = false; ///< A clear of the cursor cache is pending. @@ -123,6 +123,8 @@ private: Dimension dim; GLuint tex[NUM_TEX]; ///< The texture objects. + int16_t x_offs; ///< Number of pixels to shift the sprite to the right. + int16_t y_offs; ///< Number of pixels to shift the sprite downwards. static GLuint dummy_tex[NUM_TEX]; ///< 1x1 dummy textures to substitute for unused sprite components. @@ -136,7 +138,13 @@ private: bool BindTextures(); public: - OpenGLSprite(uint width, uint height, uint levels, SpriteComponents components); + OpenGLSprite(const SpriteLoader::SpriteCollection &sprite); + + /* No support for moving/copying the textures is implemented. */ + OpenGLSprite(const OpenGLSprite&) = delete; + OpenGLSprite(OpenGLSprite&&) = delete; + OpenGLSprite& operator=(const OpenGLSprite&) = delete; + OpenGLSprite& operator=(OpenGLSprite&&) = delete; ~OpenGLSprite(); void Update(uint width, uint height, uint level, const SpriteLoader::CommonPixel *data);