adventuregamestudio / ags

AGS editor and engine source code
Other
708 stars 159 forks source link

Improve Room Editor's navigation bar's scrolling #2422

Closed ivan-mogilko closed 6 months ago

ivan-mogilko commented 6 months ago

Fix #2415

Wrote a ToolStripDropDownMenuEx class which extends ToolStripDropDownMenu and amends or overrides its behavior. Unfortunately had to use various hacks, including getting internal base methods using reflection.

WHAT IS DONE:

~* Don't close the open dropdown menu when hovering over other items in the Address Bar.~ EDIT: this fix is temporarily removed, because it turned to be not reliable and causing new problems.

ericoporto commented 6 months ago

Could the arrow button height just be slightly bigger, like 24? 20 is in the almost sweet spot to be able to move the mouse without much effort to the place but I think this little pixels would make it just right.

ivan-mogilko commented 6 months ago

Rewrote, adding a new ToolStripDropDownMenuEx class to AGS.Controls, that handles most things.

ericoporto commented 6 months ago

I have no idea why it works but I tried this diff below and the artifacts when scrolling went away

diff --git a/Editor/AGS.Controls/Controls/ToolStripDropDownMenuEx.cs b/Editor/AGS.Controls/Controls/ToolStripDropDownMenuEx.cs
index 4afe1ef25..695d975f8 100644
--- a/Editor/AGS.Controls/Controls/ToolStripDropDownMenuEx.cs
+++ b/Editor/AGS.Controls/Controls/ToolStripDropDownMenuEx.cs
@@ -101,6 +101,16 @@ namespace AGS.Controls
             MinDisplayedItems = 0;
         }

+        protected override CreateParams CreateParams
+        {
+            get
+            {
+                CreateParams cp = base.CreateParams;
+                cp.ExStyle |= 0x02000000;  // Turn on WS_EX_COMPOSITED
+                return cp;
+            }
+        }
+
         /// <summary>
         /// Raises the System.Windows.Forms.ToolStripDropDown.Closing event.
         /// Here we intercept certain scenarios where we do not want our dropdown to close.

From https://stackoverflow.com/a/3718648, I had tried the code in the question which is what we use for our buffered panel in the room editor, but for some reason it doesn't work in this control specifically. More info in the MS docs here.

ivan-mogilko commented 6 months ago

So, unfortunately, my fix to "Don't close the open dropdown menu when hovering over other items" is not reliable, and has 2 unexpected consequences:

  1. You have to click twice on another "..." before it displays its dropdown menu
  2. Sometimes the previous dropdown is not closed even when a new one is opened.
ericoporto commented 6 months ago

There is a comment here in the docs.

Be sure to handle the special case that occurs if the "One screen at a time" mouse option is selected. In this case, the MouseWheelScrollLines property has a value of -1.

Perhaps if bigger than 16 or lower than 0 it's simply set to 16.

Edit: found examples in new Winforms source here and here. The only difference is an internal wheel scroll accumulated value.

ivan-mogilko commented 6 months ago

@Crystal-Shard, I tried to make scrolling speed relative to the system settings, using a formula found elsewhere, but I am not certain if did this right, because I don't know what is considered to be a "line" in there. Is it a line of pixels, or a line of text? And if it's a line of text, then should this equal to a single item?

That's how I calculate it now:

int linesPerWheelDelta = SystemInformation.MouseWheelScrollLines;
int heightOfLine = ItemHeight;
int scrollAmount = (linesPerWheelDelta * heightOfLine * e.Delta / SystemInformation.MouseWheelScrollDelta);

On my PC this results in approximately 3 items long scroll at a single wheel tick.

ivan-mogilko commented 6 months ago

I had to calculate number of displayed items, in order to properly support "One screen at a time" .

16 items is not a hardcoded value, but an optional property in this class, so I cannot rely on that.

ericoporto commented 6 months ago

This works fine here too. I don't think many people set this (one screen) specific setting on windows other than for accessibility reasons. I put a reference to two examples in Winforms source code and it pretty much matches your code - it's written in a much more convoluted way though, but I think it looks fine as is now and it's made in a proper way that minor tweaks are easier to do on top of your code if needed in the future before a new control arises.