Skip to content

Commit

Permalink
GUI2/Styled Widget: use move schematics to greatly reduce the amount …
Browse files Browse the repository at this point in the history
…of canvas copying

I added some debugging aids, and found that prior to this change, a total of *807* canvas objects
were created by the time the titlescreen was reached, and a new one almost (I think) every time a
tooltip was created.

With this change, the number of new canvas objects created as fallen to *476* by the same point
(a *331* decrease), and no new canvases are created when triggering a tooltip.

I'm not exactly sure how expensive creating a canvas is, though again, it's not a trivial object.
Avoiding hundreds of extra ones should result in some performance boost, at least.
  • Loading branch information
Vultraz committed May 27, 2017
1 parent 646bedb commit 515f450
Showing 1 changed file with 4 additions and 5 deletions.
9 changes: 4 additions & 5 deletions src/gui/widgets/styled_widget.cpp
Expand Up @@ -464,14 +464,13 @@ void styled_widget::definition_load_configuration(const std::string& control_typ
set_config(get_control(control_type, definition_));

/**
* Fill in each canvas from the widget state definitons.
*
* Most widgets have a single canvas. However, some widgets such as toggle_panel
* and toggle_button have a variable canvas count determined by their definitions.
*/
canvas_.resize(config_->state.size());

// Fill in each canvas.
for(size_t i = 0; i < canvas_.size(); ++i) {
canvas_[i] = config_->state[i].canvas_;
for(const auto& widget_state : config_->state) {
canvas_.emplace_back(std::move(widget_state.canvas_));
}

update_canvas();
Expand Down

3 comments on commit 515f450

@CelticMinstrel
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not how you optimize. Benchmark first, then optimize. Then you know you're increasing performance, rather than just thinking your work will increase performance without proof.

@GregoryLundberg
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes but if you want to take shots in the dark, using move instead of copy semantics is a safe bet .. just so you're SURE the moved-from instance is never again used.

@CelticMinstrel
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, that's true enough, yeah.

Please sign in to comment.