Okay, thanks. I think I finally understand what is happening on my system.
Regarding the primary bug, I am unfortunately unable to reproduce it on my
system. But the code is looking okay and if it fixes the bug for you, that is
enough testing for me.
Are you by chance testing in release mode? Or has your system only limited
texture memory? My RT::TextureTooBig exception is not triggered in release mode
since another restriction is used there. My exception is triggered due to
graphic/text/rt_render.cc:266 where different values for debug and release are
used. When using the release version your restrictions work fine. The problem
in debug mode is that the exception uses the smaller (hardcoded) value while
your checks are using the real (hardware) value which is seemingly bigger on my
system. Maybe we should move the restriction from rt_render.cc:266 into
graphic/graphic.cc:91 ? (If you do: render_text.cc:183 can then be cleaned up,
too.) Except for the text rendering no-one seems to care about the maximum
texture size so moving this should be safe.
I added some questions regarding int-casting in the diff, but they haven't been
introduced by your changes and someone probably had a reasons for the casts.
Diff comments:
>
> === modified file 'src/ui_basic/editbox.cc'
> --- src/ui_basic/editbox.cc 2018-04-27 06:11:05 +0000
> +++ src/ui_basic/editbox.cc 2018-05-23 07:31:09 +0000
> @@ -97,8 +97,7 @@
> m_->caret = 0;
> m_->scrolloffset = 0;
> // yes, use *signed* max as maximum length; just a small safe-guard.
Does this safe-guard make sense? set_max_length() takes an uint. Is the idea
that we end up with (Uint::max() / 2) ?
> - m_->maxLength =
> - std::min(g_gr->max_texture_size() / UI_FONT_SIZE_SMALL,
> std::numeric_limits<int32_t>::max());
> + set_max_length(std::numeric_limits<int32_t>::max());
>
> set_handle_mouse(true);
> set_can_focus(true);
> @@ -146,7 +145,7 @@
> * its end is cut off to fit into the maximum length.
> */
> void EditBox::set_max_length(uint32_t const n) {
> - m_->maxLength = std::min(g_gr->max_texture_size() / UI_FONT_SIZE_SMALL,
> static_cast<int>(n));
> + m_->maxLength = std::min(g_gr->max_texture_size() / text_height(),
> static_cast<int>(n));
Why casting here? n could theoretically be too big for int.
Maybe cast the first part to uint? (And possibly add an assertion that the
first part is >= 0?)
>
> if (m_->text.size() > m_->maxLength) {
> m_->text.erase(m_->text.begin() + m_->maxLength,
> m_->text.end());
>
> === modified file 'src/ui_basic/multilineeditbox.cc'
> --- src/ui_basic/multilineeditbox.cc 2018-05-13 07:15:39 +0000
> +++ src/ui_basic/multilineeditbox.cc 2018-05-23 07:31:09 +0000
> @@ -97,7 +97,7 @@
> background_style(style),
> cursor_pos(0),
> lineheight(text_height()),
> - maxbytes(std::min(g_gr->max_texture_size() / UI_FONT_SIZE_SMALL,
> 0xffff)),
> + maxbytes(std::min(g_gr->max_texture_size() * g_gr->max_texture_size() /
> (text_height() * text_height()), std::numeric_limits<int32_t>::max())),
Same here.
> ww_valid(false),
> owner(o) {
>
> scrollbar.moved.connect(boost::bind(&MultilineEditbox::scrollpos_changed, &o,
> _1));
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1741779-map-description-edit-cutoff/+merge/345495
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-1741779-map-description-edit-cutoff into
lp:widelands.
_______________________________________________
Mailing list: https://launchpad.net/~widelands-dev
Post to : [email protected]
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
