Opened 8 years ago

Closed 7 years ago

#3571 closed defect (fixed)

[PATCH] GUI Dropdown list breaks on screen resize with scrollbars=false

Reported by: elexis Owned by:
Priority: Should Have Milestone: Alpha 22
Component: Core engine Keywords: patch
Cc: Patch:

Description (last modified by elexis)

When you resize the window while being in gamesetup, the color picker dropdown element from #1580 will be messed up sometimes.

In this screenshot I resized from 1280x1024 to something else and back to 1280x1024.

Notice the last three elements are missing.

http://trac.wildfiregames.com/raw-attachment/ticket/3415/without_patch_after_resizing.jpg

This seems to be caused by the scrollbar="false" property of that playerColorPicker[n] element.

Reported initially in the second half of comment:10:ticket:3415

Attachments (5)

CDropDown.cpp.patch (617 bytes ) - added by Imarok 7 years ago.
proper patch
CDropDown.cpp.2.patch (624 bytes ) - added by Imarok 7 years ago.
Remove a comment
CDropDown.cpp_v2.patch (1.2 KB ) - added by Imarok 7 years ago.
removed button_width
bug.gif (312.9 KB ) - added by elexis 7 years ago.
Remaining bug when resizing the window horizontally. Not that problematic and not related to the proposed code apparently.
bug.jpg (8.5 KB ) - added by elexis 7 years ago.

Download all attachments as: .zip

Change History (23)

comment:1 by Imarok, 7 years ago

http://pastebin.com/fytRJjCr seems to fix this.

comment:2 by elexis, 7 years ago

So it looks like the scrollbar size is taken into account, even if there is no scollbar. In that case there should be a conditional.

comment:3 by mimo, 7 years ago

When no resize, everything is fine, and even after resizing (see #4304), if we go back and then reenter the gamesetup window, everything is fine. So the code itself, and the way it deals with scrollbars should be ok. And my interpretation is that something is not reset correctly when resizing (possibly the value of the scrollbar property).

And sorry for #4304, i forgot about this one already 1 year old :-)

comment:4 by elexis, 7 years ago

Keywords: simple added

comment:5 by Imarok, 7 years ago

The patch I posted seems to have no side effects.

It also fixes, that the dropdown list without scrollbar is mostly too small:

without patch: http://i.imgur.com/fNlVjto.pngwith patch: http://i.imgur.com/t3le4pN.png

Edit: Added images I forgot

Last edited 7 years ago by Imarok (previous) (diff)

comment:6 by mimo, 7 years ago

I agree that the patch seems to be the right thing to do: there is no reason to substract the scrollbar size if we don't have one. But what I do not understand is why it worked without the patch when we do not resize? I would have expected that without the fix, the colorPicker would be always truncated. Maybe it was just by chance that it worked, but nonetheless there is something strange here.

comment:7 by Imarok, 7 years ago

Maybe it worked without resize, because it somehow considers the textsize on init but not on resize. (just guessing ;))

by Imarok, 7 years ago

Attachment: CDropDown.cpp.patch added

proper patch

by Imarok, 7 years ago

Attachment: CDropDown.cpp.2.patch added

Remove a comment

comment:8 by Imarok, 7 years ago

Keywords: patch rfc added
Milestone: BacklogAlpha 22
Summary: GUI Dropdown list breaks on screen resize with scrollbars=false[PATCH] GUI Dropdown list breaks on screen resize with scrollbars=false

comment:9 by mimo, 7 years ago

while working at it, you could also remove button_width from lines 385 and 388 as not used in that method.

by Imarok, 7 years ago

Attachment: CDropDown.cpp_v2.patch added

removed button_width

comment:10 by elexis, 7 years ago

There is another bug. When resizing the dropdown (especially visible when resizing while the dropdown is opened), it sometimes needs uses twice the height per element, as if it would do some linebreak. It only shows the first 5 colors up to teal. This is the case with and without the patch (but the patch certainly improves the situation a lot).

by elexis, 7 years ago

Attachment: bug.gif added

Remaining bug when resizing the window horizontally. Not that problematic and not related to the proposed code apparently.

by elexis, 7 years ago

Attachment: bug.jpg added

comment:11 by elexis, 7 years ago

Keywords: simple rfc removed
Milestone: Alpha 22Backlog

Thanks for the patch and review!

http://trac.wildfiregames.com/raw-attachment/ticket/3571/bug.gif http://trac.wildfiregames.com/raw-attachment/ticket/3571/bug.jpg

Thats the remaining bug I experience when resizing horizontally. The still shows the state when stopping at the right width.

If it's indeed about linebreaks, it might be similar to #3351.

comment:12 by elexis, 7 years ago

In 18984:

Don't subtract the scrollbar size from GUI dropdowns when there is no scrollbar. Patch by Imarok, reviewed by mimo, refs #3571.
Remove unused button_width.

comment:13 by elexis, 7 years ago

The remaining bug seems to only occur with window managers that refresh the window while resizing.

comment:14 by Imarok, 7 years ago

The remaining bug can be reproduced with a window manager that doesn'`t refresh while resizing. The bug just occurs on specific window widths.

comment:15 by elexis, 7 years ago

Description: modified (diff)

temple found a way to reproduce it reliably on r19839 (almost a22):

  1. Start multiplayergame and type a long name that would get a linebreak in the middle, for example (ffffffff gggggggg hhhhhhhh)
  2. Switch between two resolutions, for example with the fullscreen/windowed mode toggle hotkey alt+enter and 1024x768 and 1920x1280

Images at http://imgur.com/a/1Jge8

When setting the size of the observermode playerselection to a relative width, the bug occurs in that dropdown too (see other images in that gallery).

Potential solution:

temple noticed that calling getComputedSize onTick fixes the issue. getComputedSize calls UpdateCachedSize.

The GUI objects/manager should be able to subscribe to SDL_WINDOWEVENT_RESIZED and call UpdateCachedSize and if the size actually changed, SendEvent(GUIM_RESIZED, "resized");

comment:16 by elexis, 7 years ago

Milestone: BacklogWork In Progress
Patch: Phab:D689

Ok the potential solution is already implemented since r77 UpdateResolution :P (and obviously isn't a solution therefore)

The most recent issue described by temple in Phab:D689 seems actually unrelated to the original bug with a similar outcome and is proposed to be fixed at Phab:D692.

comment:17 by elexis, 7 years ago

In 19845:

Fix dropdown list height after resizing the window (in case there is an item that used wordwrapping before but not after the resize or vice versa).

Differential Revision: https://code.wildfiregames.com/D692
Tested By: temple
Proofread by: Vladislav
Refs #3571

comment:18 by elexis, 7 years ago

Milestone: Work In ProgressAlpha 22
Patch: Phab:D689
Resolution: fixed
Status: newclosed

Reported issues fixed by r18984 and r19845.

Note: See TracTickets for help on using tickets.