Some performance optimizations (#5424)
authorLoïc Blot <nerzhul@users.noreply.github.com>
Wed, 22 Mar 2017 20:41:02 +0000 (21:41 +0100)
committerGitHub <noreply@github.com>
Wed, 22 Mar 2017 20:41:02 +0000 (21:41 +0100)
* Some performance optimizations

This is globally removing some memory useless copy

* use a const ref return on std::string Settings::get to prevent data copy on getters which doesn't need to copy it
 * pass some stack created strings to static const as they are not modified anywhere
 * Camera: return nametags per const ref instead of a list pointer, we only need to read it
 * INodeDefManager: getAll should be a result ref writer instead of a return copy
 * INodeDefManager: getAlias should return a const std::string ref
 * Minimap: unroll a Scolor creation in blitMinimapPixersToImageRadar to prvent many variable construct/destruct which are unneeded (we rewrite the content in the loop)
 * CNodeDefManager::updateAliases: prevent a idef getall copy
 * Profiler: constness
 * rollback_interface: create real_name later, and use const ref
 * MapBlockMesh updateFastFaceRow: unroll TileSpec next_tile, which has a cost of 1.8% CPU due to variable allocation/destruction,
 * MapBlockMesh updateFastFaceRow: copy next_tile to tile only if it's a different tilespec
 * MapBlockMesh updateFastFaceRow: use memcpy to copy next_lights to lights to do it in a single cpu operation

16 files changed:
src/camera.h
src/client/clientlauncher.cpp
src/client/tile.cpp
src/drawscene.cpp
src/game.cpp
src/itemdef.cpp
src/itemdef.h
src/mapblock_mesh.cpp
src/minimap.cpp
src/nodedef.cpp
src/profiler.h
src/remoteplayer.cpp
src/rollback_interface.cpp
src/settings.cpp
src/settings.h
src/sky.h

index b5be26718fc5f7fa09d8c1d8ccbbd37480b7887c..f57efdf10a1854a5fec8967daab71a7f276aafde 100644 (file)
@@ -172,8 +172,7 @@ public:
 
        void removeNametag(Nametag *nametag);
 
-       std::list<Nametag *> *getNametags()
-       { return &m_nametags; }
+       const std::list<Nametag *> &getNametags() { return m_nametags; }
 
        void drawNametags();
 
index 2adac53c2d55d8e79275423abb7596d87da8a4d8..bdd16205fcc4e5fc776856dacf5b36955526519c 100644 (file)
@@ -530,7 +530,7 @@ bool ClientLauncher::create_engine_device()
 
        // Determine driver
        video::E_DRIVER_TYPE driverType = video::EDT_OPENGL;
-       std::string driverstring = g_settings->get("video_driver");
+       const std::string &driverstring = g_settings->get("video_driver");
        std::vector<video::E_DRIVER_TYPE> drivers
                = porting::getSupportedVideoDrivers();
        u32 i;
index 000c766d06b09999dca6ef042d129543ef871820..85d388d6ec3e11426102ae9cb9eddc29e058b4bd 100644 (file)
@@ -134,9 +134,8 @@ std::string getTexturePath(const std::string &filename)
        /*
                Check from texture_path
        */
-       std::string texture_path = g_settings->get("texture_path");
-       if (texture_path != "")
-       {
+       const std::string &texture_path = g_settings->get("texture_path");
+       if (texture_path != "") {
                std::string testpath = texture_path + DIR_DELIM + filename;
                // Check all filename extensions. Returns "" if not found.
                fullpath = getImagePath(testpath);
@@ -1854,7 +1853,7 @@ bool TextureSource::generateImagePart(std::string part_of_name,
                        for (u32 x = 0; x < dim.Width; x++)
                        {
                                video::SColor c = baseimg->getPixel(x, y);
-                               c.color ^= mask;        
+                               c.color ^= mask;
                                baseimg->setPixel(x, y, c);
                        }
                }
@@ -2266,7 +2265,8 @@ video::ITexture* TextureSource::getNormalTexture(const std::string &name)
        if (isKnownSourceImage("override_normal.png"))
                return getTexture("override_normal.png");
        std::string fname_base = name;
-       std::string normal_ext = "_normal.png";
+       static const char *normal_ext = "_normal.png";
+       static const uint32_t normal_ext_size = strlen(normal_ext);
        size_t pos = fname_base.find(".");
        std::string fname_normal = fname_base.substr(0, pos) + normal_ext;
        if (isKnownSourceImage(fname_normal)) {
@@ -2274,7 +2274,7 @@ video::ITexture* TextureSource::getNormalTexture(const std::string &name)
                size_t i = 0;
                while ((i = fname_base.find(".", i)) != std::string::npos) {
                        fname_base.replace(i, 4, normal_ext);
-                       i += normal_ext.length();
+                       i += normal_ext_size;
                }
                return getTexture(fname_base);
                }
index 3a03743ee2ddf7d599751bc3051b4107962f3254..663c8828c311f2f350923a6949f699433a0ed570 100644 (file)
@@ -494,7 +494,7 @@ void draw_scene(video::IVideoDriver *driver, scene::ISceneManager *smgr,
        catch(SettingNotFoundException) {}
 #endif
 
-       std::string draw_mode = g_settings->get("3d_mode");
+       const std::string &draw_mode = g_settings->get("3d_mode");
 
        smgr->drawAll();
 
index 5b49d34c0e42a22cd00ae57101113914c10af770..9d52e4326e85bca01e1ea773492fa69c0e683d01 100644 (file)
@@ -4094,7 +4094,7 @@ void Game::updateFrame(ProfilerGraph *graph, RunStats *stats, f32 dtime,
                Drawing begins
        */
 
-       video::SColor skycolor = sky->getSkyColor();
+       const video::SColor &skycolor = sky->getSkyColor();
 
        TimeTaker tt_draw("mainloop: draw");
        driver->beginScene(true, true, skycolor);
index f43e5c970cb3d4212335a6b5e8dac11d4a1de7b4..627ad1b6c45164a7e57d6999b39d6978bb3bf1dc 100644 (file)
@@ -275,16 +275,16 @@ public:
                assert(i != m_item_definitions.end());
                return *(i->second);
        }
-       virtual std::string getAlias(const std::string &name) const
+       virtual const std::string &getAlias(const std::string &name) const
        {
                StringMap::const_iterator it = m_aliases.find(name);
                if (it != m_aliases.end())
                        return it->second;
                return name;
        }
-       virtual std::set<std::string> getAll() const
+       virtual void getAll(std::set<std::string> &result) const
        {
-               std::set<std::string> result;
+               result.clear();
                for(std::map<std::string, ItemDefinition *>::const_iterator
                                it = m_item_definitions.begin();
                                it != m_item_definitions.end(); ++it) {
@@ -295,7 +295,6 @@ public:
                                it != m_aliases.end(); ++it) {
                        result.insert(it->first);
                }
-               return result;
        }
        virtual bool isKnown(const std::string &name_) const
        {
index 2ade6116a03fa0a8efbbc4aa7f999f9c4692a2d2..01ec4fa2fed37623c606cc472b0553043bc7add7 100644 (file)
@@ -100,9 +100,9 @@ public:
        // Get item definition
        virtual const ItemDefinition& get(const std::string &name) const=0;
        // Get alias definition
-       virtual std::string getAlias(const std::string &name) const=0;
+       virtual const std::string &getAlias(const std::string &name) const=0;
        // Get set of all defined item names and aliases
-       virtual std::set<std::string> getAll() const=0;
+       virtual void getAll(std::set<std::string> &result) const=0;
        // Check if item is known
        virtual bool isKnown(const std::string &name) const=0;
 #ifndef SERVER
@@ -126,9 +126,9 @@ public:
        // Get item definition
        virtual const ItemDefinition& get(const std::string &name) const=0;
        // Get alias definition
-       virtual std::string getAlias(const std::string &name) const=0;
+       virtual const std::string &getAlias(const std::string &name) const=0;
        // Get set of all defined item names and aliases
-       virtual std::set<std::string> getAll() const=0;
+       virtual void getAll(std::set<std::string> &result) const=0;
        // Check if item is known
        virtual bool isKnown(const std::string &name) const=0;
 #ifndef SERVER
index f76033ea8a5bb16b8fab0ad27275a6b4fd8e32ef..eddb061b45b582353c9d83a6bc29b1e5339d2acb 100644 (file)
@@ -855,8 +855,9 @@ static void updateFastFaceRow(
                        makes_face, p_corrected, face_dir_corrected,
                        lights, tile);
 
-       for(u16 j=0; j<MAP_BLOCKSIZE; j++)
-       {
+       // Unroll this variable which has a significant build cost
+       TileSpec next_tile;
+       for (u16 j = 0; j < MAP_BLOCKSIZE; j++) {
                // If tiling can be done, this is set to false in the next step
                bool next_is_different = true;
 
@@ -866,12 +867,11 @@ static void updateFastFaceRow(
                v3s16 next_p_corrected;
                v3s16 next_face_dir_corrected;
                u16 next_lights[4] = {0,0,0,0};
-               TileSpec next_tile;
+
 
                // If at last position, there is nothing to compare to and
                // the face must be drawn anyway
-               if(j != MAP_BLOCKSIZE - 1)
-               {
+               if (j != MAP_BLOCKSIZE - 1) {
                        p_next = p + translate_dir;
 
                        getTileInfo(data, p_next, face_dir,
@@ -879,7 +879,7 @@ static void updateFastFaceRow(
                                        next_face_dir_corrected, next_lights,
                                        next_tile);
 
-                       if(next_makes_face == makes_face
+                       if (next_makes_face == makes_face
                                        && next_p_corrected == p_corrected + translate_dir
                                        && next_face_dir_corrected == face_dir_corrected
                                        && next_lights[0] == lights[0]
@@ -894,38 +894,14 @@ static void updateFastFaceRow(
                                        && tile.emissive_light == next_tile.emissive_light) {
                                next_is_different = false;
                                continuous_tiles_count++;
-                       } else {
-                               /*if(makes_face){
-                                       g_profiler->add("Meshgen: diff: next_makes_face != makes_face",
-                                                       next_makes_face != makes_face ? 1 : 0);
-                                       g_profiler->add("Meshgen: diff: n_p_corr != p_corr + t_dir",
-                                                       (next_p_corrected != p_corrected + translate_dir) ? 1 : 0);
-                                       g_profiler->add("Meshgen: diff: next_f_dir_corr != f_dir_corr",
-                                                       next_face_dir_corrected != face_dir_corrected ? 1 : 0);
-                                       g_profiler->add("Meshgen: diff: next_lights[] != lights[]",
-                                                       (next_lights[0] != lights[0] ||
-                                                       next_lights[0] != lights[0] ||
-                                                       next_lights[0] != lights[0] ||
-                                                       next_lights[0] != lights[0]) ? 1 : 0);
-                                       g_profiler->add("Meshgen: diff: !(next_tile == tile)",
-                                                       !(next_tile == tile) ? 1 : 0);
-                               }*/
                        }
-                       /*g_profiler->add("Meshgen: Total faces checked", 1);
-                       if(makes_face)
-                               g_profiler->add("Meshgen: Total makes_face checked", 1);*/
-               } else {
-                       /*if(makes_face)
-                               g_profiler->add("Meshgen: diff: last position", 1);*/
                }
 
-               if(next_is_different)
-               {
+               if (next_is_different) {
                        /*
                                Create a face if there should be one
                        */
-                       if(makes_face)
-                       {
+                       if (makes_face) {
                                // Floating point conversion of the position vector
                                v3f pf(p_corrected.X, p_corrected.Y, p_corrected.Z);
                                // Center point of face (kind of)
@@ -957,11 +933,9 @@ static void updateFastFaceRow(
                makes_face = next_makes_face;
                p_corrected = next_p_corrected;
                face_dir_corrected = next_face_dir_corrected;
-               lights[0] = next_lights[0];
-               lights[1] = next_lights[1];
-               lights[2] = next_lights[2];
-               lights[3] = next_lights[3];
-               tile = next_tile;
+               std::memcpy(lights, next_lights, ARRLEN(lights) * sizeof(u16));
+               if (next_is_different)
+                       tile = next_tile;
                p = p_next;
        }
 }
index ee29c58bac376c295d009679cd1937c4b9df56ca..a7f4822c901b572a6f3397693fce50e00c269381 100644 (file)
@@ -105,7 +105,7 @@ void MinimapUpdateThread::doUpdate()
                        // Swap two values in the map using single lookup
                        std::pair<std::map<v3s16, MinimapMapblock*>::iterator, bool>
                            result = m_blocks_cache.insert(std::make_pair(update.pos, update.data));
-                       if (result.second == false) {
+                       if (!result.second) {
                                delete result.first->second;
                                result.first->second = update.data;
                        }
@@ -322,13 +322,15 @@ void Minimap::setAngle(f32 angle)
 
 void Minimap::blitMinimapPixelsToImageRadar(video::IImage *map_image)
 {
+       video::SColor c(240, 0, 0, 0);
        for (s16 x = 0; x < data->map_size; x++)
        for (s16 z = 0; z < data->map_size; z++) {
                MinimapPixel *mmpixel = &data->minimap_scan[x + z * data->map_size];
 
-               video::SColor c(240, 0, 0, 0);
                if (mmpixel->air_count > 0)
                        c.setGreen(core::clamp(core::round32(32 + mmpixel->air_count * 8), 0, 255));
+               else
+                       c.setGreen(0);
 
                map_image->setPixel(x, data->map_size - z - 1, c);
        }
@@ -337,21 +339,23 @@ void Minimap::blitMinimapPixelsToImageRadar(video::IImage *map_image)
 void Minimap::blitMinimapPixelsToImageSurface(
        video::IImage *map_image, video::IImage *heightmap_image)
 {
+       // This variable creation/destruction has a 1% cost on rendering minimap
+       video::SColor tilecolor;
        for (s16 x = 0; x < data->map_size; x++)
        for (s16 z = 0; z < data->map_size; z++) {
                MinimapPixel *mmpixel = &data->minimap_scan[x + z * data->map_size];
 
                const ContentFeatures &f = m_ndef->get(mmpixel->n);
                const TileDef *tile = &f.tiledef[0];
+
                // Color of the 0th tile (mostly this is the topmost)
-               video::SColor tilecolor;
                if(tile->has_color)
                        tilecolor = tile->color;
                else
                        mmpixel->n.getColor(f, &tilecolor);
+
                tilecolor.setRed(tilecolor.getRed() * f.minimap_color.getRed() / 255);
-               tilecolor.setGreen(tilecolor.getGreen() * f.minimap_color.getGreen()
-                       / 255);
+               tilecolor.setGreen(tilecolor.getGreen() * f.minimap_color.getGreen() / 255);
                tilecolor.setBlue(tilecolor.getBlue() * f.minimap_color.getBlue() / 255);
                tilecolor.setAlpha(240);
 
@@ -391,7 +395,7 @@ video::ITexture *Minimap::getMinimapTexture()
        if (minimap_mask) {
                for (s16 y = 0; y < MINIMAP_MAX_SY; y++)
                for (s16 x = 0; x < MINIMAP_MAX_SX; x++) {
-                       video::SColor mask_col = minimap_mask->getPixel(x, y);
+                       const video::SColor &mask_col = minimap_mask->getPixel(x, y);
                        if (!mask_col.getAlpha())
                                minimap_image->setPixel(x, y, video::SColor(0,0,0,0));
                }
@@ -430,7 +434,7 @@ scene::SMeshBuffer *Minimap::getMinimapMeshBuffer()
        scene::SMeshBuffer *buf = new scene::SMeshBuffer();
        buf->Vertices.set_used(4);
        buf->Indices.set_used(6);
-       video::SColor c(255, 255, 255, 255);
+       static const video::SColor c(255, 255, 255, 255);
 
        buf->Vertices[0] = video::S3DVertex(-1, -1, 0, 0, 0, 1, c, 0, 1);
        buf->Vertices[1] = video::S3DVertex(-1,  1, 0, 0, 0, 1, c, 0, 0);
@@ -550,15 +554,13 @@ void Minimap::updateActiveMarkers()
        video::IImage *minimap_mask = data->minimap_shape_round ?
                data->minimap_mask_round : data->minimap_mask_square;
 
-       std::list<Nametag *> *nametags = client->getCamera()->getNametags();
+       const std::list<Nametag *> &nametags = client->getCamera()->getNametags();
 
        m_active_markers.clear();
 
-       for (std::list<Nametag *>::const_iterator
-                       i = nametags->begin();
-                       i != nametags->end(); ++i) {
-               Nametag *nametag = *i;
-               v3s16 pos = floatToInt(nametag->parent_node->getPosition() +
+       for (std::list<Nametag *>::const_iterator i = nametags.begin();
+                       i != nametags.end(); ++i) {
+               v3s16 pos = floatToInt((*i)->parent_node->getPosition() +
                        intToFloat(client->getCamera()->getOffset(), BS), BS);
                pos -= data->pos - v3s16(data->map_size / 2,
                                data->scan_height / 2,
@@ -570,7 +572,7 @@ void Minimap::updateActiveMarkers()
                }
                pos.X = ((float)pos.X / data->map_size) * MINIMAP_MAX_SX;
                pos.Z = ((float)pos.Z / data->map_size) * MINIMAP_MAX_SY;
-               video::SColor mask_col = minimap_mask->getPixel(pos.X, pos.Z);
+               const video::SColor &mask_col = minimap_mask->getPixel(pos.X, pos.Z);
                if (!mask_col.getAlpha()) {
                        continue;
                }
index 3532eea1e6951686095b17450b9be505c7b102db..2745a45e8cb1644f9eee09f7f93998ba1743f68b 100644 (file)
@@ -1332,12 +1332,13 @@ void CNodeDefManager::removeNode(const std::string &name)
 
 void CNodeDefManager::updateAliases(IItemDefManager *idef)
 {
-       std::set<std::string> all = idef->getAll();
+       std::set<std::string> all;
+       idef->getAll(all);
        m_name_id_mapping_with_aliases.clear();
-       for (std::set<std::string>::iterator
+       for (std::set<std::string>::const_iterator
                        i = all.begin(); i != all.end(); ++i) {
-               std::string name = *i;
-               std::string convert_to = idef->getAlias(name);
+               const std::string &name = *i;
+               const std::string &convert_to = idef->getAlias(name);
                content_t id;
                if (m_name_id_mapping.getId(convert_to, id)) {
                        m_name_id_mapping_with_aliases.insert(
index e8eac86b13e5b8c0ddf130b11fb3d99233331f2e..6da11597262b384d54483a31918e744db7264117 100644 (file)
@@ -119,39 +119,34 @@ public:
                u32 minindex, maxindex;
                paging(m_data.size(), page, pagecount, minindex, maxindex);
 
-               for(std::map<std::string, float>::iterator
-                               i = m_data.begin();
-                               i != m_data.end(); ++i)
-               {
-                       if(maxindex == 0)
+               for (std::map<std::string, float>::const_iterator i = m_data.begin();
+                               i != m_data.end(); ++i) {
+                       if (maxindex == 0)
                                break;
                        maxindex--;
 
-                       if(minindex != 0)
-                       {
+                       if (minindex != 0) {
                                minindex--;
                                continue;
                        }
 
-                       std::string name = i->first;
                        int avgcount = 1;
-                       std::map<std::string, int>::iterator n = m_avgcounts.find(name);
-                       if(n != m_avgcounts.end()){
+                       std::map<std::string, int>::const_iterator n = m_avgcounts.find(i->first);
+                       if (n != m_avgcounts.end()) {
                                if(n->second >= 1)
                                        avgcount = n->second;
                        }
-                       o<<"  "<<name<<": ";
+                       o << "  " << i->first << ": ";
                        s32 clampsize = 40;
-                       s32 space = clampsize - name.size();
-                       for(s32 j=0; j<space; j++)
-                       {
-                               if(j%2 == 0 && j < space - 1)
-                                       o<<"-";
+                       s32 space = clampsize - i->first.size();
+                       for(s32 j = 0; j < space; j++) {
+                               if (j % 2 == 0 && j < space - 1)
+                                       o << "-";
                                else
-                                       o<<" ";
+                                       o << " ";
                        }
-                       o<<(i->second / avgcount);
-                       o<<std::endl;
+                       o << (i->second / avgcount);
+                       o << std::endl;
                }
        }
 
index 0a4591410637d269df8d4943577583b49f1282ce..c8e5b91326ba06dd02d7c8763176e434f0e0a226 100644 (file)
@@ -141,7 +141,7 @@ void RemotePlayer::deSerialize(std::istream &is, const std::string &playername,
 
        m_dirty = true;
        //args.getS32("version"); // Version field value not used
-       std::string name = args.get("name");
+       const std::string &name = args.get("name");
        strlcpy(m_name, name.c_str(), PLAYERNAME_SIZE);
 
        if (sao) {
@@ -167,7 +167,7 @@ void RemotePlayer::deSerialize(std::istream &is, const std::string &playername,
                } catch (SettingNotFoundException &e) {}
 
                try {
-                       std::string extended_attributes = args.get("extended_attributes");
+                       const std::string &extended_attributes = args.get("extended_attributes");
                        Json::Reader reader;
                        Json::Value attr_root;
                        reader.parse(extended_attributes, attr_root);
index 7345c4a7dcecf50b2c4ded0e2204132872c527a6..40a33a51da470010c3f4570dd53133d6ad1e10ce 100644 (file)
@@ -190,7 +190,6 @@ bool RollbackAction::applyRevert(Map *map, InventoryManager *imgr, IGameDef *gam
                case TYPE_MODIFY_INVENTORY_STACK: {
                        InventoryLocation loc;
                        loc.deSerialize(inventory_location);
-                       std::string real_name = gamedef->idef()->getAlias(inventory_stack.name);
                        Inventory *inv = imgr->getInventory(loc);
                        if (!inv) {
                                infostream << "RollbackAction::applyRevert(): Could not get "
@@ -211,10 +210,12 @@ bool RollbackAction::applyRevert(Map *map, InventoryManager *imgr, IGameDef *gam
                                        << inventory_location << std::endl;
                                return false;
                        }
+                       
                        // If item was added, take away item, otherwise add removed item
                        if (inventory_add) {
                                // Silently ignore different current item
-                               if (list->getItem(inventory_index).name != real_name)
+                               if (list->getItem(inventory_index).name !=
+                                               gamedef->idef()->getAlias(inventory_stack.name))
                                        return false;
                                list->takeItem(inventory_index, inventory_stack.count);
                        } else {
index c4c3c90737dd680e42679f609853accb436d5d5c..b4083264e1cf78a270f91637e245fd688d9638cd 100644 (file)
@@ -90,33 +90,6 @@ bool Settings::checkValueValid(const std::string &value)
        return true;
 }
 
-
-std::string Settings::sanitizeName(const std::string &name)
-{
-       std::string n = trim(name);
-
-       for (const char *s = "=\"{}#"; *s; s++)
-               n.erase(std::remove(n.begin(), n.end(), *s), n.end());
-
-       return n;
-}
-
-
-std::string Settings::sanitizeValue(const std::string &value)
-{
-       std::string v(value);
-       size_t p = 0;
-
-       if (v.substr(0, 3) == "\"\"\"")
-               v.erase(0, 3);
-
-       while ((p = v.find("\n\"\"\"")) != std::string::npos)
-               v.erase(p, 4);
-
-       return v;
-}
-
-
 std::string Settings::getMultiline(std::istream &is, size_t *num_lines)
 {
        size_t lines = 1;
@@ -398,7 +371,7 @@ Settings *Settings::getGroup(const std::string &name) const
 }
 
 
-std::string Settings::get(const std::string &name) const
+const std::string &Settings::get(const std::string &name) const
 {
        const SettingsEntry &entry = getEntry(name);
        if (entry.is_group)
index b19733514daed23e09a10b2742f8e21ae54c3874..777d0eff5e3b6f0b1098b0e58f869adab15b29a9 100644 (file)
@@ -129,8 +129,6 @@ public:
 
        static bool checkNameValid(const std::string &name);
        static bool checkValueValid(const std::string &value);
-       static std::string sanitizeName(const std::string &name);
-       static std::string sanitizeValue(const std::string &value);
        static std::string getMultiline(std::istream &is, size_t *num_lines=NULL);
        static void printEntry(std::ostream &os, const std::string &name,
                const SettingsEntry &entry, u32 tab_depth=0);
@@ -141,7 +139,7 @@ public:
 
        const SettingsEntry &getEntry(const std::string &name) const;
        Settings *getGroup(const std::string &name) const;
-       std::string get(const std::string &name) const;
+       const std::string &get(const std::string &name) const;
        bool getBool(const std::string &name) const;
        u16 getU16(const std::string &name) const;
        s16 getS16(const std::string &name) const;
index f1989177351a7c9420459353b5165851ac1e9789..17d6c5f73ec3249f7119ff64dfec86d03912fccd 100644 (file)
--- a/src/sky.h
+++ b/src/sky.h
@@ -56,18 +56,21 @@ public:
        void update(float m_time_of_day, float time_brightness,
                        float direct_brightness, bool sunlight_seen, CameraMode cam_mode,
                        float yaw, float pitch);
-       
+
        float getBrightness(){ return m_brightness; }
 
-       video::SColor getBgColor(){
+       const video::SColor &getBgColor() const
+       {
                return m_visible ? m_bgcolor : m_fallback_bg_color;
        }
-       video::SColor getSkyColor(){
+
+       const video::SColor &getSkyColor() const
+       {
                return m_visible ? m_skycolor : m_fallback_bg_color;
        }
-       
-       bool getCloudsVisible(){ return m_clouds_visible && m_visible; }
-       video::SColorf getCloudColor(){ return m_cloudcolor_f; }
+
+       bool getCloudsVisible() { return m_clouds_visible && m_visible; }
+       const video::SColorf &getCloudColor() { return m_cloudcolor_f; }
 
        void setVisible(bool visible){ m_visible = visible; }
        void setFallbackBgColor(const video::SColor &fallback_bg_color){