Skip to content

Instantly share code, notes, and snippets.

@Pokechu22 Pokechu22/!MC-123773.md
Last active Oct 18, 2018

Embed
What would you like to do?

Like most of my posts, this ended up being long and full of details that usually do not matter but may be useful as reference. Feel free to jump to the conclusion and/or to skip information on how I gathered data.

The change that caused this

18w01a fixed MC-122053, which was an issue where the amount that the mouse scrolled by was ignored. This meant that for some mouses, on windows, scrolling would scroll extremely fast and far; note that this issue was not reported for 1.13 but rather 1.12, and actually existed far before then.

Here are the changes relevant to that fix (MCP names):

--- a/src/main/java/net/minecraft/client/gui/GuiSlot.java
+++ b/src/main/java/net/minecraft/client/gui/GuiSlot.java
@@ -381,21 +381,12 @@ public abstract class GuiSlot extends GuiEventHandler
     public boolean mouseScrolled(double p_mouseScrolled_1_)
     {
         if (!this.isVisible())
         {
             return false;
         }
         else
         {
-            if (p_mouseScrolled_1_ > 0.0D)
-            {
-                p_mouseScrolled_1_ = -1.0D;
-            }
-            else if (p_mouseScrolled_1_ < 0.0D)
-            {
-                p_mouseScrolled_1_ = 1.0D;
-            }
-
-            this.amountScrolled += p_mouseScrolled_1_ * (double)this.slotHeight / 2.0D;
+            this.amountScrolled -= p_mouseScrolled_1_ * (double)this.slotHeight / 2.0D;
             return true;
         }
     }
--- a/src/main/java/net/minecraft/client/gui/inventory/GuiContainerCreative.java
+++ b/src/main/java/net/minecraft/client/gui/inventory/GuiContainerCreative.java
@@ -635,26 +635,15 @@ public class GuiContainerCreative extends InventoryEffectRenderer
     public boolean mouseScrolled(double p_mouseScrolled_1_)
     {
         if (!this.needsScrollBars())
         {
             return false;
         }
         else
         {
             int i = (((GuiContainerCreative.ContainerCreative)this.inventorySlots).itemList.size() + 9 - 1) / 9 - 5;
-
-            if (p_mouseScrolled_1_ > 0.0D)
-            {
-                p_mouseScrolled_1_ = 1.0D;
-            }
-
-            if (p_mouseScrolled_1_ < 0.0D)
-            {
-                p_mouseScrolled_1_ = -1.0D;
-            }
-
             this.currentScroll = (float)((double)this.currentScroll - p_mouseScrolled_1_ / (double)i);
             this.currentScroll = MathHelper.clamp(this.currentScroll, 0.0F, 1.0F);
             ((GuiContainerCreative.ContainerCreative)this.inventorySlots).scrollTo(this.currentScroll);
             return true;
         }
     }
--- a/src/main/java/net/minecraft/util/MouseHelper.java
+++ b/src/main/java/net/minecraft/util/MouseHelper.java
@@ -20,16 +20,17 @@ public class MouseHelper
     private final Minecraft minecraft;
     private boolean leftDown;
     private boolean middleDown;
     private boolean rightDown;
     private double mouseX;
     private double mouseY;
     private int activeButton = -1;
     private boolean ignoreFirstMove = true;
     private int touchScreenCounter;
     private double eventTime;
     private final MouseSmoother xSmoother = new MouseSmoother();
     private final MouseSmoother ySmoother = new MouseSmoother();
     private double xVelocity;
     private double yVelocity;
+    private double accumulatedScrollDelta;
     private double field_198050_o = Double.MIN_VALUE;
     private boolean mouseGrabbed;
@@ -130,31 +131,44 @@ public class MouseHelper
     private void scrollCallback(long handle, double xoffset, double yoffset)
     {
         if (handle == Minecraft.getMinecraft().mainWindow.getHandle())
         {
             if (this.minecraft.currentScreen != null)
             {
                 this.minecraft.currentScreen.mouseScrolled(yoffset);
             }
             else if (this.minecraft.player != null)
             {
+                if (this.accumulatedScrollDelta != 0.0D && Math.signum(yoffset) != Math.signum(this.accumulatedScrollDelta))
+                {
+                    this.accumulatedScrollDelta = 0.0D;
+                }
+
+                this.accumulatedScrollDelta += yoffset;
+                double d0 = (double)((int)this.accumulatedScrollDelta);
+
+                if (d0 == 0.0D)
+                {
+                    return;
+                }
+
+                this.accumulatedScrollDelta -= d0;

                 if (this.minecraft.player.isSpectator())
                 {
-                    double lvt_7_1_ = yoffset < 0.0D ? -1.0D : 1.0D;
                     if (this.minecraft.ingameGUI.getSpectatorGui().isMenuActive())
                     {
-                        this.minecraft.ingameGUI.getSpectatorGui().onMouseScroll(-lvt_7_1_);
+                        this.minecraft.ingameGUI.getSpectatorGui().onMouseScroll(-d0);
                     }
                     else
                     {
-                        double d1 = MathHelper.clamp((double)this.minecraft.player.capabilities.getFlySpeed() + lvt_7_1_ * (double)0.005F, 0.0D, (double)0.2F);
+                        double d1 = MathHelper.clamp((double)this.minecraft.player.capabilities.getFlySpeed() + d0 * (double)0.005F, 0.0D, (double)0.2F);
                         this.minecraft.player.capabilities.setFlySpeed(d1);
                     }
                 }
                 else
                 {
-                    this.minecraft.player.inventory.changeCurrentItem(yoffset);
+                    this.minecraft.player.inventory.changeCurrentItem(d0);
                 }
             }
         }
     }

The three different files do different things. The first one is the one responsible for fixing scrolling on lists. The second one is responsible for being able to move the creative scroll bar without changing items (which is interesting, but not a real issue/the actual cause of this).

The third one is the one responsible for the actual issue. accumulatedScrollDelta is a new thing, and the code keeps on changing it until it is at least 1 (or -1), and then uses the integer value of that for actually doing other stuff. There's also code to reset it to 0 if the scroll direction is changed, which makes sense. The weird thing here is that it changes to integers (despite the other methods actually taking doubles and processing them there), and it ignores any value smaller than an integer (which gives weird behavior for scrolling). This is different from the behavior in previous versions, where (at least for changeCurrentItem), any negative value is treated as -1 and any positive value is treated as +1, even for smaller values. (This code is not present in the above diff.)

Understanding the parameters as seen on Windows

Just to give some context before I show some data, here's an explanation of how scrolling on windows works.

WM_MOUSEWHEEL is used to get scroll input. The documentation states, among other things:

wParam

The high-order word indicates the distance the wheel is rotated, expressed in multiples or divisions of WHEEL_DELTA, which is 120. A positive value indicates that the wheel was rotated forward, away from the user; a negative value indicates that the wheel was rotated backward, toward the user.

-snip-

Remarks

The wheel rotation will be a multiple of WHEEL_DELTA, which is set at 120. This is the threshold for action to be taken, and one such action (for example, scrolling one increment) should occur for each delta.

The delta was set to 120 to allow Microsoft or other vendors to build finer-resolution wheels (a freely-rotating wheel with no notches) to send more messages per rotation, but with a smaller value in each message. To use this feature, you can either add the incoming delta values until WHEEL_DELTA is reached (so for a delta-rotation you get the same response), or scroll partial lines in response to the more frequent messages. You can also choose your scroll granularity and accumulate deltas until it is reached.

My mouse actually scrolls in increments of 30, and sometimes events are bunched into larger values of 60, 90, 120; I've even managed 360 when scrolling vigorously.

Understanding the parameters as seen on Mac.

Apple's documentation is less useful. The various possible fields for a scroll event are deltaX, deltaY, deltaZ, hasPreciseScrollingDeltas, scrollingDeltaX, and scrollingDeltaY. Here are some various quotes:

deltaX and deltaY

(snapshots: 1 2)

It's not clear what these do for scroll events, if anything... but they are present for it, and were used in a few cases...

Discussion

This property is only valid for scroll wheel, mouse-move, mouse-drag, and swipe events. For swipe events, a nonzero value represents a vertical swipe: -1 for swipe down and 1 for swipe up.

For scroll wheel events, use scrollingDeltaY instead.

deltaZ

(snapshot)

A quote for this is sufficient...

deltaZ

The z-coordinate change for a scroll wheel, mouse-move, or mouse-drag event.

Discussion

This value is typically 0.0.

hasPreciseScrollingDeltas

(snapshot)

This is a flag of some sort, which clearly does something important...

hasPreciseScrollingDeltas

A Boolean value that indicates whether precise scrolling deltas are available.

Discussion

This property is set to true if precise scrolling deltas are available; otherwise, false.

This property is valid for NSScrollWheel events. A generic scroll wheel issues rather coarse scroll deltas. Some mice and trackpads provide much more precise delta. This method determines how the values of the scrollingDeltaX and scrollingDeltaY should be interpreted.

scrollingDeltaX and scrollingDeltaY

(snapshots: 1 2)

These seem to be the actual properties that apple wants you to use. Unfortunately there isn't a clear explanation of what the values will be, still...

scrollingDeltaY

The scroll wheel’s vertical delta.

Discussion

This is the preferred property for accessing NSScrollWheel delta values. When hasPreciseScrollingDeltas is false, multiply the value returned by this method by the line or row height. Otherwise scroll by the returned amount.

(scrollingDeltaX is similar, but it says "When hasPreciseScrollingDeltas is false, your application may need to modify the raw value before using it." without actually specifying what that means)

Of course, this isn't super helpful for games, since what the line height is is not clear, either.

What about linux?

I'm not covering linux here since that's a third systemto test for, and I don't have a good setup for testing on it either. However, from my recollection and what others (mrgrim if I recall correctly) have told me, scroll input on linux basically acts as two additional buttons, one pressed when the wheel makes a rotation up and one when it makes a rotation down, and there is no mechanism for partial rotations to be reported. Assuming all that is the case, this bug is unlikely to have any effects.

Interpretation of scroll values and how I gathered them

So, knowing that, here's some more info on LWJGL 2, LWJGL 3, and Firefox (as a control group) handle scroll input. Also provided are the various programs I used to gather my input, though these are very unstable and I don't recommend using them for much.

LWJGL 2

On windows, LWJGL 2 supplies these values to the game directly, where 120 is a normal rotation (and my machine reports 30, 60, 90, 120, etc). Code source.

On mac, it does a similar thing of directly passing values (code source), but multiplies values by 120 (WHEEL_SCALE) to maintain consistency (code source). Interestingly, an older version of the code also multiplies by 12 "or so" when hasPreciseScrollingDeltas is false on the event.

I initally tried to test this by a custom program that would be ran by the Minecraft launcher (just because I didn't want to deal with native setups). Unfortunately this program was unreliable, so I ended up also writing a litemod for 1.12.2 that did the same thing ingame when F12 is pressed.

LWJGL 3

LWJGL 3 on windows supplies the values such that 1.000 is a normal rotation (and my machine reports 0.250, 0.500, etc), which it does by dividing by WHEEL_DELTA. Code source.

On mac, it directly reports the values. When hasPreciseScrollingDeltas is true, the values are divided by 10 (producing a ration similar to what LWJGL 2 did in that early code). Code source.

I used a modified LWJGL 3 demo to test this, which is essentially a rewrite of the events demo program included with GLFW, but in java. (NOTE: there is a 3rd version of this demo, but it should not be used; it was a failed experiment to see whether the bug could be fixed a certain way and is not based on the regular libraries).

Firefox

Firefox reports scroll values in different ways on different OSs. The WheelEvent.deltaMode property specifies these. With the data below, on my machine a scroll is given in DOM_DELTA_LINE (where one complete scroll is actually 3 lines, and I get .75, 1.5, etc.), and on mac a windows mouse is DOM_DELTA_LINE and the touchpad and magic mouse are DOM_DELTA_PIXEL.

My reasoning for including firefox was that I can be reasonably certain that firefox behaves as a reasonable app would with regards to scroll data, and it is also open-source so I could check how it actually is handling stuff if so. More on this further down.

To gather this data, I made a page that just records wheel and scroll events (the original data only has wheel events; scroll events were added for later comparison).

Now look at all of these amazing graphs

So, given all that, the next thing to do is to collect data and then make graphs. Most of the mac data here was gathered by Z750; the windows data was gathered by me. This data is also slightly cherry-picked for the nicer-looking graphs; all of the data can be found in this zip file. The data I excluded is mostly because Z750 scrolled extremely vigorously instead of starting slow; while this data is interesting it doesn't show how big the difference is between speeds.

For all of these graphs, the first graph is individual scroll events; the second graph is each event added together. So, multiple events at the same position on the first grpah would result in a linear change on the second graph.

LWJGL 3 on my machine

3-self.png 3-self-i.png

On windows, at a constant scroll speed, events of the same value are continuously produced, producing a logical curve.

LWJGL 3 on a mac with a windows mouse

3-windows-mouse.png 3-windows-mouse-i.png

... but, on a mac, scrolling is VERY different. Values start at .100, but quickly ramp up to 10 -- a factor of 100. This produces CRAZY swings and is the general cause of the bug itself. The value starting at .100 is what requires turning the wheel several times when moving slowly; and the higher values are what makes it a lot harder to control.

LWJGL 3 on a mac with a apple touch mouse

3-apple-touch-mouse.png 3-apple-touch-mouse-i.png

This has acceleration and deceleration, but still behaves weirdly. The starting value is also .100, so the same issue happens here.

LWJGL 3 with the MBP touchpad

3-mbp-touchpad.png 3-mbp-touchpad-i.png

Not much different from the touch mouse. Note that the much lower bits are from horizontal scrolling.

Firefox with the MBP touchpad

web-mbp-touchpad.png web-mbp-touchpad-i.png

A cleaner example of the curve with the touch pad, and also that this isn't exclusive to LWJGL 3. This is a rather weird-looking curve, but I think that's just because it's rounding to the nearest integer.

Firefox on a mac with a windows mouse

web-windows-mouse.png web-windows-mouse-i.png

A cleaner example of the windows mouse on a mac and the steep acceleration, also showing that this affects other platforms.

LWJGL 2 on a mac with a windows mouse

2-windows-mouse.png 2-windows-mouse-i.png

Another interesting example of extreme acceleration.

Is there a way to normalize this?

Needless to say, the mac data confused me. There's some extremely steep acceleration there. Is it actually like this on other apps?

I tested this using firefox, by manually summing the wheel events and comparing that to the actual scroll events. While there was not a 1 to 1 correspondence -- a small wheel event did generate several scroll events to interpolate -- things still look pretty similar and it is clear that firefox is not reversing this acceleration, which indicates that there probably is not a good way of doing so.

Here's a graph of both on windows; they both have the same shape:

winwheel.png winscroll.png

... and here's a graph of them on mac; it also is the same shape (note: I gathered this data myself, and this was with a windows mouse):

macwheel.png macscroll.png

So, basically, there isn't a good way to recover useful scroll data from this.

Why mouseWheelSensitivity almost helps, but causes problems

A common recommendation is to set mouseWheelSensitivity to 10. Looking at the previous data, it's clear why that works; it means that a scroll of .100 is equivalent to 1 and thus a normal mouse wheel turn changes a hotbar slot.

But also looking at the data, it's also pretty obvious looking at the graphs that a higher mouseWheelSensitivity will cause major issues if you keep scrolling, due to the crazy amount of acceleration. This means that actual lists behave strangely in exchange for that.

Proposed fix

I think that the actual fix to MC-122053 in the List class is good. The creative inventory change probably doesn't need to be kept, but it's not too big of a deal so it could be either way.

However, the accumulatedScrollDelta code should be removed, as it produces bad results on devices where scrolling one click produces only a value of .100 and is pointless when the other methods already make use of just the sign of the value (changeCurrentItem most notably). Instead of waiting until a rotation greater than 1 is accumulated, the partial rotations should just be used as-is, since the user did rotate the scroll wheel and that should be enough to change hotbar slots.

A compromise would be to add a second option that controls whether or not the mouse wheel value is actually used, with options for always using it, using it only on lists, and never using it (i.e. the behavior in 1.12.2). On mac, this would default to either never using it or using it only on lists, and on other platforms it would default to always using it. This could combine well with mouseWheelSensitivity on some machines, though it might be overkill. If such a route is taken, both options should be present in the GUI, and not just left as a hidden setting in options.txt.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.