From c406a2067fbb33fac9ec6261746931082d89f2ea Mon Sep 17 00:00:00 2001 From: Charles Dang Date: Wed, 21 Jun 2017 16:43:41 +1100 Subject: [PATCH] Refactored halo code to bring it in line with new drawing methods This basically rips out all the unrendering and invalidation code. Neither is needed anymore. Instead, we can have a simple, clean, "just draw the managed halos" interface. Note there's an issue unrelated to this where halos aren't removed when a unit is killed with the debug kill menu command. --- src/display.cpp | 4 +- src/game_display.cpp | 1 - src/halo.cpp | 524 +++++++++++++++---------------------------- src/halo.hpp | 10 +- 4 files changed, 183 insertions(+), 356 deletions(-) diff --git a/src/display.cpp b/src/display.cpp index 504361dcb93..71212edbcbb 100644 --- a/src/display.cpp +++ b/src/display.cpp @@ -156,7 +156,7 @@ void display::remove_single_overlay(const map_location& loc, const std::string& display::display(const display_context * dc, std::weak_ptr wb, reports & reports_object, const config& theme_cfg, const config& level, bool auto_join) : video2::draw_layering(auto_join) , dc_(dc) - , halo_man_(new halo::manager(*this)) + , halo_man_(new halo::manager()) , wb_(wb) , exclusive_unit_draw_requests_() , screen_(CVideo::get_singleton()) @@ -506,7 +506,7 @@ void display::change_display_context(const display_context * dc) void display::reset_halo_manager() { - halo_man_.reset(new halo::manager(*this)); + halo_man_.reset(new halo::manager()); } void display::reset_halo_manager(halo::manager & halo_man) diff --git a/src/game_display.cpp b/src/game_display.cpp index 8348e2ba897..0b2379c72bb 100644 --- a/src/game_display.cpp +++ b/src/game_display.cpp @@ -236,7 +236,6 @@ void game_display::draw_invalidated() { return; // DONE - halo_man_->unrender(invalidated_); display::draw_invalidated(); if (fake_unit_man_->empty()) { return; diff --git a/src/halo.cpp b/src/halo.cpp index f742f576f4e..2f393041bd0 100644 --- a/src/halo.cpp +++ b/src/halo.cpp @@ -1,5 +1,5 @@ /* - Copyright (C) 2003 - 2018 by David White + Copyright (C) 2003 - 2017 by David White Part of the Battle for Wesnoth Project http://www.wesnoth.org/ This program is free software; you can redistribute it and/or modify @@ -18,11 +18,11 @@ * Examples: white mage, lighthouse. */ +#include "halo.hpp" #include "animated.hpp" #include "display.hpp" -#include "preferences/game.hpp" -#include "halo.hpp" #include "log.hpp" +#include "preferences/game.hpp" #include "serialization/string_utils.hpp" #include @@ -32,138 +32,122 @@ static lg::log_domain log_display("display"); namespace halo { - class halo_impl { - -class effect -{ public: - effect(display * screen, int xpos, int ypos, const animated::anim_description& img, - const map_location& loc, ORIENTATION, bool infinite); + halo_impl() + : halos() + , halo_id(1) + { + } - void set_location(int x, int y); + int add(int x, + int y, + const std::string& image, + const map_location& loc, + ORIENTATION orientation = NORMAL, + bool infinite = true); - bool render(); - void unrender(); + /** Set the position of an existing haloing effect, according to its handle. */ + void set_location(int handle, int x, int y); - bool expired() const { return !images_.cycles() && images_.animation_finished(); } - bool need_update() const { return images_.need_update(); } - bool does_change() const { return !images_.does_not_change(); } - bool on_location(const std::set& locations) const; - bool location_not_known() const; + /** Remove the halo with the given handle. */ + void remove(int handle); + + /** + * Render halos. + * + * Which halos are rendered is determined by invalidated_locations and the + * internal state in the control sets (in halo.cpp). + */ + void render(); - void add_overlay_location(std::set& locations); private: + /** Encapsulates the drawing of a single halo effect. */ + class effect + { + public: + effect(int xpos, + int ypos, + const animated::anim_description& img, + const map_location& loc, + ORIENTATION, + bool infinite); - const image::locator& current_image() const { return images_.get_current_frame(); } + void set_location(int x, int y); - animated images_; + bool render(); - ORIENTATION orientation_; + bool expired() const + { + return !images_.cycles() && images_.animation_finished(); + } - int x_, y_; - surface surf_, buffer_; - SDL_Rect rect_; + bool need_update() const + { + return images_.need_update(); + } - /** The location of the center of the halo. */ - map_location loc_; + bool does_change() const + { + return !images_.does_not_change(); + } - /** All locations over which the halo lies. */ - std::vector overlayed_hexes_; + private: + const image::locator& current_image() const + { + return images_.get_current_frame(); + } - display * disp; + animated images_; + + ORIENTATION orientation_; + + int x_, y_; + + texture texture_; + + /** The location of the center of the halo. */ + map_location loc_; + + display* disp; + }; + + std::map halos; + + int halo_id; }; -display* disp; - -std::map haloes; -int halo_id; - -/** - * Upon unrendering, an invalidation list is send. All haloes in that area and - * the other invalidated haloes are stored in this set. Then there'll be - * tested which haloes overlap and they're also stored in this set. - */ -std::set invalidated_haloes; - -/** - * Upon deleting, a halo isn't deleted but added to this set, upon unrendering - * the image is unrendered and deleted. - */ -std::set deleted_haloes; - -/** - * Haloes that have an animation or expiration time need to be checked every - * frame and are stored in this set. - */ -std::set changing_haloes; - -public: -/** - * impl's of exposed functions - */ - -explicit halo_impl(display & screen) : - disp(&screen), - haloes(), - halo_id(1), - invalidated_haloes(), - deleted_haloes(), - changing_haloes() -{} - - -int add(int x, int y, const std::string& image, const map_location& loc, - ORIENTATION orientation=NORMAL, bool infinite=true); - -/** Set the position of an existing haloing effect, according to its handle. */ -void set_location(int handle, int x, int y); - -/** Remove the halo with the given handle. */ -void remove(int handle); - -/** - * Render and unrender haloes. - * - * Which haloes are rendered is determined by invalidated_locations and the - * internal state in the control sets (in halo.cpp). - */ -void unrender(std::set invalidated_locations); -void render(); - -}; //end halo_impl - -halo_impl::effect::effect(display * screen, int xpos, int ypos, const animated::anim_description& img, - const map_location& loc, ORIENTATION orientation, bool infinite) : - images_(img), - orientation_(orientation), - x_(0), - y_(0), - surf_(nullptr), - buffer_(nullptr), - rect_(sdl::empty_rect), - loc_(loc), - overlayed_hexes_(), - disp(screen) +halo_impl::effect::effect( + int xpos, + int ypos, + const animated::anim_description& img, + const map_location& loc, + ORIENTATION orientation, + bool infinite) + : images_(img) + , orientation_(orientation) + , x_(0) + , y_(0) + , texture_(nullptr) + , loc_(loc) + , disp(display::get_singleton()) { assert(disp != nullptr); - set_location(xpos,ypos); - - images_.start_animation(0,infinite); + set_location(xpos, ypos); + images_.start_animation(0, infinite); } void halo_impl::effect::set_location(int x, int y) { int new_x = x - disp->get_location_x(map_location::ZERO()); int new_y = y - disp->get_location_y(map_location::ZERO()); - if (new_x != x_ || new_y != y_) { + + if(new_x != x_ || new_y != y_) { x_ = new_x; y_ = new_y; - buffer_.assign(nullptr); - overlayed_hexes_.clear(); } } @@ -179,138 +163,60 @@ bool halo_impl::effect::render() } else { // The location of a halo is an x,y value and not a map location. // This means when a map is zoomed, the halo's won't move, - // This glitch is most visible on [item] haloes. + // This glitch is most visible on [item] halos. // This workaround always recalculates the location of the halo - // (item haloes have a location parameter to hide them under the shroud) + // (item halos have a location parameter to hide them under the shroud) // and reapplies that location. // It might be optimized by storing and comparing the zoom value. set_location( disp->get_location_x(loc_) + disp->hex_size() / 2, - disp->get_location_y(loc_) + disp->hex_size() / 2); + disp->get_location_y(loc_) + disp->hex_size() / 2 + ); } } images_.update_last_draw_time(); - surf_.assign(image::get_image(current_image(),image::SCALED_TO_ZOOM)); - if(surf_ == nullptr) { + + texture_ = image::get_texture(current_image() /*, image::SCALED_TO_ZOOM*/); + if(texture_ == nullptr) { return false; } - if(orientation_ == HREVERSE || orientation_ == HVREVERSE) { - surf_.assign(image::reverse_image(surf_)); - } - if(orientation_ == VREVERSE || orientation_ == HVREVERSE) { - surf_.assign(flop_surface(surf_)); - } const int screenx = disp->get_location_x(map_location::ZERO()); const int screeny = disp->get_location_y(map_location::ZERO()); - const int xpos = x_ + screenx - surf_->w/2; - const int ypos = y_ + screeny - surf_->h/2; + texture::info t_info = texture_.get_info(); - SDL_Rect rect {xpos, ypos, surf_->w, surf_->h}; - rect_ = rect; - SDL_Rect clip_rect = disp->map_outside_area(); + const int xpos = x_ + screenx - t_info.w / 2; + const int ypos = y_ + screeny - t_info.h / 2; - // If rendered the first time, need to determine the area affected. - // If a halo changes size, it is not updated. - if(location_not_known()) { - display::rect_of_hexes hexes = disp->hexes_under_rect(rect); - display::rect_of_hexes::iterator i = hexes.begin(), end = hexes.end(); - for (;i != end; ++i) { - overlayed_hexes_.push_back(*i); - } - } + // TODO: decide if I need this + // SDL_Rect clip_rect = disp->map_outside_area(); + // const clip_rect_setter clip_setter(screen, &clip_rect); - if(sdl::rects_overlap(rect,clip_rect) == false) { - buffer_.assign(nullptr); - return false; - } + const bool h_flip = orientation_ == HREVERSE || orientation_ == HVREVERSE; + const bool v_flip = orientation_ == VREVERSE || orientation_ == HVREVERSE; - surface& screen = disp->get_screen_surface(); - - const clip_rect_setter clip_setter(screen, &clip_rect); - if(buffer_ == nullptr || buffer_->w != rect.w || buffer_->h != rect.h) { - SDL_Rect rect2 = rect_; - buffer_.assign(get_surface_portion(screen,rect2)); - } else { - SDL_Rect rect2 = rect_; - sdl_copy_portion(screen,&rect2,buffer_,nullptr); - } - - sdl_blit(surf_,nullptr,screen,&rect); + disp->render_scaled_to_zoom(texture_, xpos, ypos, h_flip, v_flip); return true; } -void halo_impl::effect::unrender() -{ - if (!surf_ || !buffer_) { - return; - } - // Shrouded haloes are never rendered unless shroud has been re-placed; in - // that case, unrendering causes the hidden terrain (and previous halo - // frame, when dealing with animated halos) to glitch through shroud. We - // don't need to unrender them because shroud paints over the underlying - // area anyway. - if (loc_.x != -1 && loc_.y != -1 && disp->shrouded(loc_)) { - return; - } +// +// HALO IMPL ========================================================================= +// - surface& screen = disp->get_screen_surface(); - - SDL_Rect clip_rect = disp->map_outside_area(); - const clip_rect_setter clip_setter(screen, &clip_rect); - - // Due to scrolling, the location of the rendered halo - // might have changed; recalculate - const int screenx = disp->get_location_x(map_location::ZERO()); - const int screeny = disp->get_location_y(map_location::ZERO()); - - const int xpos = x_ + screenx - surf_->w/2; - const int ypos = y_ + screeny - surf_->h/2; - - SDL_Rect rect {xpos, ypos, surf_->w, surf_->h}; - sdl_blit(buffer_,nullptr,screen,&rect); -} - -bool halo_impl::effect::on_location(const std::set& locations) const -{ - for(std::vector::const_iterator itor = overlayed_hexes_.begin(); - itor != overlayed_hexes_.end(); ++itor) { - if(locations.find(*itor) != locations.end()) { - return true; - } - } - return false; -} - -bool halo_impl::effect::location_not_known() const -{ - return overlayed_hexes_.empty(); -} - -void halo_impl::effect::add_overlay_location(std::set& locations) -{ - for(std::vector::const_iterator itor = overlayed_hexes_.begin(); - itor != overlayed_hexes_.end(); ++itor) { - - locations.insert(*itor); - } -} - -// End halo_impl::effect impl's - -int halo_impl::add(int x, int y, const std::string& image, const map_location& loc, - ORIENTATION orientation, bool infinite) +int halo_impl::add( + int x, int y, const std::string& image, const map_location& loc, ORIENTATION orientation, bool infinite) { const int id = halo_id++; - animated::anim_description image_vector; - std::vector items = utils::square_parenthetical_split(image, ','); - for(const std::string& item : items) { + animated::anim_description image_vector; + + for(const std::string& item : utils::square_parenthetical_split(image, ',')) { const std::vector& sub_items = utils::split(item, ':'); + std::string str = item; int time = 100; @@ -318,203 +224,131 @@ int halo_impl::add(int x, int y, const std::string& image, const map_location& l str = sub_items.front(); try { time = std::stoi(sub_items.back()); - } catch(std::invalid_argument&) { + } catch(std::invalid_argument) { ERR_DP << "Invalid time value found when constructing halo: " << sub_items.back() << "\n"; } } - image_vector.push_back(animated::frame_description(time,image::locator(str))); + image_vector.emplace_back(time, image::locator(str)); } - haloes.emplace(id, effect(disp, x, y, image_vector, loc, orientation, infinite)); - invalidated_haloes.insert(id); - if(haloes.find(id)->second.does_change() || !infinite) { - changing_haloes.insert(id); - } + + halos.emplace(id, effect(x, y, image_vector, loc, orientation, infinite)); + return id; } void halo_impl::set_location(int handle, int x, int y) { - const std::map::iterator itor = haloes.find(handle); - if(itor != haloes.end()) { - itor->second.set_location(x,y); + const auto itor = halos.find(handle); + if(itor != halos.end()) { + itor->second.set_location(x, y); } } void halo_impl::remove(int handle) { - // Silently ignore invalid haloes. - // This happens when Wesnoth is being terminated as well. - if(handle == NO_HALO || haloes.find(handle) == haloes.end()) { + // Silently ignore invalid halos. This happens when Wesnoth is being terminated as well. + if(handle == NO_HALO || halos.find(handle) == halos.end()) { return; } - deleted_haloes.insert(handle); -} - -void halo_impl::unrender(std::set invalidated_locations) -{ - if(preferences::show_haloes() == false || haloes.empty()) { - return; + auto itor = halos.find(handle); + if(itor != halos.end()) { + halos.erase(itor); } - //assert(invalidated_haloes.empty()); - - // Remove expired haloes - std::map::iterator itor = haloes.begin(); - for(; itor != haloes.end(); ++itor ) { - if(itor->second.expired()) { - deleted_haloes.insert(itor->first); - } - } - - // Add the haloes marked for deletion to the invalidation set - std::set::const_iterator set_itor = deleted_haloes.begin(); - for(;set_itor != deleted_haloes.end(); ++set_itor) { - invalidated_haloes.insert(*set_itor); - haloes.find(*set_itor)->second.add_overlay_location(invalidated_locations); - } - - // Test the multi-frame haloes whether they need an update - for(set_itor = changing_haloes.begin(); - set_itor != changing_haloes.end(); ++set_itor) { - if(haloes.find(*set_itor)->second.need_update()) { - invalidated_haloes.insert(*set_itor); - haloes.find(*set_itor)->second.add_overlay_location(invalidated_locations); - } - } - - // Find all halo's in a the invalidated area - size_t halo_count; - - // Repeat until set of haloes in the invalidated area didn't change - // (including none found) or all existing haloes are found. - do { - halo_count = invalidated_haloes.size(); - for(itor = haloes.begin(); itor != haloes.end(); ++itor) { - // Test all haloes not yet in the set - // which match one of the locations - if(invalidated_haloes.find(itor->first) == invalidated_haloes.end() && - (itor->second.location_not_known() || - itor->second.on_location(invalidated_locations))) { - - // If found, add all locations which the halo invalidates, - // and add it to the set - itor->second.add_overlay_location(invalidated_locations); - invalidated_haloes.insert(itor->first); - } - } - } while (halo_count != invalidated_haloes.size() && halo_count != haloes.size()); - - if(halo_count == 0) { - return; - } - - // Render the haloes: - // iterate through all the haloes and invalidate if in set - for(std::map::reverse_iterator ritor = haloes.rbegin(); ritor != haloes.rend(); ++ritor) { - if(invalidated_haloes.find(ritor->first) != invalidated_haloes.end()) { - ritor->second.unrender(); - } - } - - // Really delete the haloes marked for deletion - for(set_itor = deleted_haloes.begin(); set_itor != deleted_haloes.end(); ++set_itor) { - changing_haloes.erase(*set_itor); - invalidated_haloes.erase(*set_itor); - haloes.erase(*set_itor); - } - - deleted_haloes.clear(); } void halo_impl::render() { - if(preferences::show_haloes() == false || haloes.empty() || - invalidated_haloes.empty()) { + if(!preferences::show_haloes() || halos.empty()) { return; } - // Render the haloes: draw all invalidated haloes - for(int id : invalidated_haloes) { - haloes.at(id).render(); + // + // Clean up expired halos. + // + for(auto itor = halos.begin(); itor != halos.end(); /* Handle increment in loop*/) { + if(itor->second.expired()) { + halos.erase(itor++); + } else { + ++itor; + } } - invalidated_haloes.clear(); + // + // Render the halos. + // + for(auto& halo : halos) { + halo.second.render(); + } } -// end halo_impl implementations -// begin halo::manager +// +// HALO MANAGER ========================================================================= +// -manager::manager(display& screen) : impl_(new halo_impl(screen)) -{} - -handle manager::add(int x, int y, const std::string& image, const map_location& loc, - ORIENTATION orientation, bool infinite) +manager::manager() + : impl_(new halo_impl()) { - int new_halo = impl_->add(x,y,image, loc, orientation, infinite); +} + +handle manager::add( + int x, int y, const std::string& image, const map_location& loc, ORIENTATION orientation, bool infinite) +{ + int new_halo = impl_->add(x, y, image, loc, orientation, infinite); return handle(new halo_record(new_halo, impl_)); } -/** Set the position of an existing haloing effect, according to its handle. */ -void manager::set_location(const handle & h, int x, int y) +void manager::set_location(const handle& h, int x, int y) { - impl_->set_location(h->id_,x,y); + impl_->set_location(h->id_, x, y); } -/** Remove the halo with the given handle. */ -void manager::remove(const handle & h) +void manager::remove(const handle& h) { impl_->remove(h->id_); h->id_ = NO_HALO; } -/** - * Render and unrender haloes. - * - * Which haloes are rendered is determined by invalidated_locations and the - * internal state in the control sets (in halo.cpp). - */ -void manager::unrender(std::set invalidated_locations) -{ - impl_->unrender(invalidated_locations); -} - void manager::render() { impl_->render(); } -// end halo::manager implementation +// +// HALO RECORD ========================================================================= +// -/** - * halo::halo_record implementation - */ +halo_record::halo_record() + : id_(NO_HALO) // halo::NO_HALO + , my_manager_() +{ +} -halo_record::halo_record() : - id_(NO_HALO), //halo::NO_HALO - my_manager_() -{} - -halo_record::halo_record(int id, const std::shared_ptr & my_manager) : - id_(id), - my_manager_(my_manager) -{} +halo_record::halo_record(int id, const std::shared_ptr& my_manager) + : id_(id) + , my_manager_(my_manager) +{ +} halo_record::~halo_record() { - if (!valid()) return; + if(!valid()) { + return; + } std::shared_ptr man = my_manager_.lock(); - if (man) { + if(man) { try { man->remove(id_); - } catch (std::exception & e) { + } catch(std::exception& e) { std::cerr << "Caught an exception in halo::halo_record destructor: \n" << e.what() << std::endl; - } catch (...) {} + } catch(...) { + } } } -} //end namespace halo +} // end namespace halo diff --git a/src/halo.hpp b/src/halo.hpp index 0c173db9de3..53d9e1d53da 100644 --- a/src/halo.hpp +++ b/src/halo.hpp @@ -39,7 +39,7 @@ const int NO_HALO = 0; class manager { public: - manager(display& screen); + manager(); /** * Add a haloing effect using 'image centered on (x,y). @@ -59,13 +59,7 @@ public: /** Remove the halo with the given handle. */ void remove(const handle & h); - /** - * Render and unrender haloes. - * - * Which haloes are rendered is determined by invalidated_locations and the - * internal state in the control sets (in halo.cpp). - */ - void unrender(std::set invalidated_locations); + /** Render managed halos. */ void render(); private: