Code looks good to me. Three small comments are in the diff, feel free to
ignore them.
The speed problem is not so great. Is this a general problem of the new
renderer? Do you have an idea why it is so much slower?
On a side note: I don't like the "speed-hack" in revision 8540, but when it
helps...
As far as I am concerned, this can be merged. But the speed problem should be
looked into at some point.
Diff comments:
>
> === modified file 'data/scripting/richtext.lua'
> --- data/scripting/richtext.lua 2017-07-03 10:16:59 +0000
> +++ data/scripting/richtext.lua 2017-12-19 18:07:21 +0000
> @@ -327,21 +327,24 @@
> end
>
> -- RST
> --- .. function li_image(imagepath, text)
> +-- .. function li_image(imagepath, text[, vsp])
> --
> -- Places a paragraph of text to the right of an image
>
> -- :arg imagepath: the full path to the image file
> -- :arg text_width_percent: the percentatge of space that the text will
> occupy
This parameter does not seem to exists any more.
> -- :arg text: the text to be placed next to the image
> +-- :arg vsp: Vertical spacing. Default is 6.
> --
> --- :returns: the text wrapped in a paragraph and placed next to the
> image, The outer tag is a div.
> -function li_image(imagepath, text_width_percent, text)
> - return p("<br>") .. div("width=100%", "") ..
> - div(p(img(imagepath))) ..
> +-- :returns: the text wrapped in a paragraph and placed next to the
> image, the outer tag is a div.
> +function li_image(imagepath, text, vsp)
> + if vsp == nil then vsp = 6 end
> + return
> + div("width=100%",
> + div(p(vspace(vsp) .. img(imagepath) .. space(6))) ..
> div(p(space(6))) ..
> - div("width="..text_width_percent.."%", p(text)) ..
> - div("width=100%", "")
> + div("width=*", p(vspace(vsp) .. text .. vspace(2 * vsp)))
> + )
> end
>
> -- RST
>
> === modified file 'data/txts/AUTHORS.lua'
> --- data/txts/AUTHORS.lua 2016-03-14 11:37:49 +0000
> +++ data/txts/AUTHORS.lua 2017-12-19 18:07:21 +0000
> @@ -26,7 +26,7 @@
> return {
> title = _"Developers",
> text = rt(
> - title(_"Widelands Development Team") ..
> + p_font("align=center", "size=28 color=2F9131", _"Widelands Development
> Team") ..
Why no h1() or so for this? Also below.
> list_authors()
> )
> }
>
> === modified file 'src/ui_basic/fileview_panel.cc'
> --- src/ui_basic/fileview_panel.cc 2017-08-08 17:39:40 +0000
> +++ src/ui_basic/fileview_panel.cc 2017-12-19 18:07:21 +0000
> @@ -51,8 +51,18 @@
> std::unique_ptr<UI::Box>(new UI::Box(this, 0, 0, UI::Box::Vertical,
> 0, 0, padding_)));
> size_t index = boxes_.size() - 1;
>
> - textviews_.push_back(std::unique_ptr<UI::MultilineTextarea>(
> - new UI::MultilineTextarea(boxes_.at(index).get(), 0, 0,
> Scrollbar::kSize, 0, content)));
> + UI::MultilineTextarea* textarea =
> + new UI::MultilineTextarea(boxes_.at(index).get(), 0, 0,
> Scrollbar::kSize, 0);
> + try {
> + textarea->force_new_renderer();
> + textarea->set_text(content);
> + } catch (const std::exception& e) {
> + log("Fileview: falling back to OLD font renderer: %s\n",
> e.what());
> + textarea->force_new_renderer(false);
> + textarea->set_text(content);
> + }
> +
> +
> textviews_.push_back(std::unique_ptr<UI::MultilineTextarea>(std::move(textarea)));
Is there a reason you are using std::move() here instead of just passing the
pointer to the constructor?
> add((boost::format("about_%lu") % index).str(), title,
> boxes_.at(index).get(), "");
>
> assert(boxes_.size() == textviews_.size());
--
https://code.launchpad.net/~widelands-dev/widelands/fh1-editorhelp/+merge/335283
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/fh1-editorhelp.
_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
