Fix render of overlapped UI elements

[Districh-ru]

This PR fixes the render of overlapped UI elements (buttons and scrollbar) in the Hide Interface mode.

In master build:

https://github.com/user-attachments/assets/a5beb04a-ca5a-4c0b-881e-f1691ffabd58

This PR also fixes a case when while moving the scrollbar the pointer is moved outside of the scrollbar move area and the scrollbar is drawn (this moveable sprite is hidden/shown) on the every mouse pointer move, but luckily without screen redraw.

This PR affects all scrollbars and buttons rendering.

[oleg-derevenetz]

Hi @Districh-ru after some thought, I decided to try to implement this in a more canonical way, if I may say so - in terms of minimizing the changeset that is needed to implement the ad-hoc logic that is in fact not widely used.

https://github.com/user-attachments/assets/fd256618-790d-49d4-9a6e-975dfaec71fb

Also I fixed a minor issue with button rendering in the "no interface" when buttons panel captures the mouse cursor, but it becomes hidden before the mouse cursor was released. In the master branch:

https://github.com/user-attachments/assets/9c0e41c4-9d02-4abe-828c-1ef87ae1e546

With this PR:

https://github.com/user-attachments/assets/8b5cc986-0f13-43a1-8ad3-0a04658b733b

I have removed the changes related to the scroll buttons of the icons panel so far:

https://github.com/user-attachments/assets/889bf0bd-8b7e-4c54-8465-da8f1531b43c

I suggest we do without them in this PR for now, this is a relatively minor issue. We can think a bit more about how to make it in a more elegant way. But, if nothing better is thought up, then we can always implement this particular part the way it was originally implemented here - in a separate PR. In any case, there is no rush with this - as I said, this issue is a relatively minor one.

What do you think about these changes?

[Districh-ru]

Hi, @oleg-derevenetz, I agree with the changes you made. Thanks for cleaning the code and fixing another bug!

But I did one commit to do the new Buttons Panel buttons rendering only if the button is actually overlapped. If you think it is a complex solution we can revert it, but do fheroes2::Display::instance().render( buttonRect ); to render not the whole screen, but only the changed part.

Currently I see no other way than to use bool ButtonBase::drawOnPress( const bool doRenderToDisplay /* = true */ ) approach. Some time before while fixing some rendering bugs I was thinking to do such change. Currently we always force screen render to display when rendering buttons' state change , but it is not needed in all the cases. In example, when we hit the CANCEL button we render it and then at the same time restore (by ImageRestorer destructor) the image under the closed dialog, thus we do the redundant (and often not seen) render of the button, which some times leads to UI bugs. But, you are right - lets do such change in a new PR. Also in the next PR I'll remove the never used non-default Display & output = Display::instance() argument from the drawOnPress() and drawOnRelease methods.

[oleg-derevenetz]

Hi @Districh-ru

But I did one commit to do the new Buttons Panel buttons rendering only if the button is actually overlapped. If you think it is a complex solution we can revert it, but do fheroes2::Display::instance().render( buttonRect ); to render not the whole screen, but only the changed part.

Well, IMHO this looks like an unnesessary complication if we take into account that these buttons are not intended to be pressed with 1000 PPS (presses per second :) ), so we in fact win nothing at the cost of such a complication of the code. We can do render only of the button panel itself, however, and that's it.

Also could you please explain the purpose of passing the REDRAW_STATUS flag to the _interface.redraw()? In the "no interface mode", the rendering process takes into account possibly-overlapped windows without the need to pass these flags manually.

In example, when we hit the CANCEL button we render it and then at the same time restore (by ImageRestorer destructor) the image under the closed dialog, thus we do the redundant (and often not seen) render of the button, which some times leads to UI bugs. But, you are right - lets do such change in a new PR. Also in the next PR I'll remove the never used non-default Display & output = Display::instance() argument from the drawOnPress() and drawOnRelease methods.

OK, let's do it this way. But if you can somehow simplify all this for the future readers and maintainers of the code, somehow wrap it in something and so on, then it's better to think about how to do it.

[Districh-ru]

Well, IMHO this looks like an unnesessary complication if we take into account that these buttons are not intended to be pressed with 1000 PPS (presses per second :) ), so we in fact win nothing at the cost of such a complication of the code. We can do render only of the button panel itself, however, and that's it.

Yeah, looking this way we have no benefit of this complication. :) I've reverted the change, but made fheroes2::Display::instance().render( buttonRect ); not to render the whole (currently unchanged) screen.

Also could you please explain the purpose of passing the REDRAW_STATUS flag to the _interface.redraw()? In the "no interface mode", the rendering process takes into account possibly-overlapped windows without the need to pass these flags manually.

I knew about it, but put REDRAW_STATUS to show that we actually want to redraw it even if we change the _interface.redraw() code for some reason.

[Districh-ru]

OK, let's do it this way. But if you can somehow simplify all this for the future readers and maintainers of the code, somehow wrap it in something and so on, then it's better to think about how to do it.

I'll take some time to find a better way to fix these forced rendering issues.

[oleg-derevenetz]

I knew about it, but put REDRAW_STATUS to show that we actually want to redraw it even if we change the _interface.redraw() code for some reason.

If we will change the redraw() code, then it IMHO still shouldn't force the "end-user"/window code to think about all these overlapping, because today these windows are overlapping in this order, and tomorrow they may overlap in different order or there may be new window(s), etc. The "window manager" (i.e. _interface in our case) should track all this, each window should ideally know as little about other windows as absolutely necessary.

[Districh-ru]

OK, let's do it this way. But if you can somehow simplify all this for the future readers and maintainers of the code, somehow wrap it in something and so on, then it's better to think about how to do it.

I'll take some time to find a better way to fix these forced rendering issues.

I'll do it in a new PR, this one has enough changes. :)

If we will change the redraw() code, then it IMHO still shouldn't force the "end-user"/window code to think about all these overlapping, because today these windows are overlapping in this order, and tomorrow they may overlap in different order or there may be new window(s), etc. The "window manager" (i.e. _interface in our case) should track all this, each window should ideally know as little about other windows as absolutely necessary.

Should we then try to make the last clicked window be on top? ( not in this PR :) ) In this case the behavior will be close to OS behavior and there will be no overlap of the clicked items (buttons, scrollbars).

[oleg-derevenetz]

Should we then try to make the last clicked window be on top? ( not in this PR :) ) In this case the behavior will be close to OS behavior and there will be no overlap of the clicked items (buttons, scrollbars).

Sure, why not. It will look more natural than it does now.

[ihhub]

@Districh-ru , many thanks for these updates!