UIComponent & ScrollComponent: Code review fixes & improved scrolling in ScrollComponent

Linear: EM-3575

Author: RedEpicness

api/Elementa.api

@@ -274,6 +274,7 @@ public abstract class gg/essential/elementa/WindowScreen : gg/essential/universa
 	public fun onKeyPressed (ICLgg/essential/universal/UKeyboard$Modifiers;)V
 	public fun onMouseClicked (DDI)V
 	public fun onMouseReleased (DDI)V
+	public fun onMouseScrolled (D)V
 	public fun onMouseScrolled (DDDD)V
 	public fun onScreenClose ()V
 	public final fun stopAnimating (Lkotlin/reflect/KMutableProperty0;)V
@@ -2750,6 +2751,7 @@ public abstract class gg/essential/elementa/events/UIEvent {
 }
 
 public final class gg/essential/elementa/events/UIScrollEvent : gg/essential/elementa/events/UIEvent {
+	public fun <init> (DDLgg/essential/elementa/UIComponent;Lgg/essential/elementa/UIComponent;)V
 	public fun <init> (DLgg/essential/elementa/UIComponent;Lgg/essential/elementa/UIComponent;)V
 	public fun <init> (DLgg/essential/elementa/UIComponent;Lgg/essential/elementa/UIComponent;D)V
 	public final fun component1 ()D
@@ -2763,8 +2765,8 @@ public final class gg/essential/elementa/events/UIScrollEvent : gg/essential/ele
 	public fun equals (Ljava/lang/Object;)Z
 	public final fun getCurrentTarget ()Lgg/essential/elementa/UIComponent;
 	public final fun getDelta ()D
-	public final fun getDeltaHorizontal ()D
-	public final fun getDeltaVertical ()D
+	public final fun getScrollX ()D
+	public final fun getScrollY ()D
 	public final fun getTarget ()Lgg/essential/elementa/UIComponent;
 	public fun hashCode ()I
 	public fun toString ()Ljava/lang/String;

src/main/kotlin/gg/essential/elementa/ElementaVersion.kt

@@ -196,10 +196,11 @@ enum class ElementaVersion {
      * Improvements for vertical scrolling and support for horizontal scrolling.
      *
      * Adds new mouseScroll() function to UIComponent that also supports horizontal scrolling.
-     * The old function is no longer called by WindowScreen.
-     * Vertical deltas passed to the new function are now passed along unchanged.
-     * Previous they were coerced between -1.0 and 1.0 in WindowScreen.
+     * The old function is no longer supported and will no longer be called.
+     * Previously, deltas were always coerced between -1.0 and 1.0 in WindowScreen.
+     * This was wrong, and is no longer the case for the new function.
      * UIScrollEvent now includes also the horizontal delta.
+     * ScrollComponent now also properly supports new horizontal scrolling.
      *
      * See [UniversalCraft#128](https://github.com/SparkUniverse/UniversalCraft/pull/128) for the underlying changes.
      */

src/main/kotlin/gg/essential/elementa/UIComponent.kt

@@ -751,18 +751,18 @@ abstract class UIComponent : Observable(), ReferenceHolder {
      * Use this in the proper mouse scroll event to cascade all component's mouse scroll events.
      * Most common use is on the [Window] object.
      */
-    open fun mouseScroll(deltaHorizontal: Double, deltaVertical: Double) {
-        if (deltaHorizontal == 0.0 && deltaVertical == 0.0) return
+    open fun mouseScroll(scrollX: Double, scrollY: Double) {
+        if (scrollX == 0.0 && scrollY == 0.0) return
 
         for (i in children.lastIndex downTo 0) {
             val child = children[i]
 
             if (child.isHovered()) {
-                return child.mouseScroll(deltaHorizontal, deltaVertical)
+                return child.mouseScroll(scrollX, scrollY)
             }
         }
 
-        fireScrollEvent(UIScrollEvent(deltaVertical, this, this, deltaHorizontal))
+        fireScrollEvent(UIScrollEvent(scrollX, scrollY, this, this))
     }
 
     open fun onWindowResize() {

src/main/kotlin/gg/essential/elementa/WindowScreen.kt

@@ -86,14 +86,25 @@ abstract class WindowScreen @JvmOverloads constructor(
         window.mouseRelease()
     }
 
+    @Suppress("DEPRECATION")
+    @Deprecated(
+        "Provided `delta` values have different units depending on Minecraft versions. See ElementaVersion.V11 for details.",
+        replaceWith = ReplaceWith("onMouseScrolled(mouseX, mouseY, deltaHorizontal, deltaVertical)")
+    )
+    override fun onMouseScrolled(delta: Double) {
+        super.onMouseScrolled(delta)
+
+        if (version < ElementaVersion.v11) {
+            // We also need to pass along scrolling
+            window.mouseScroll(delta.coerceIn(-1.0, 1.0))
+        }
+    }
+
     override fun onMouseScrolled(mouseX: Double, mouseY: Double, deltaHorizontal: Double, deltaVertical: Double) {
         super.onMouseScrolled(mouseX, mouseY, deltaHorizontal, deltaVertical)
 
         if (version >= ElementaVersion.v11) {
             window.mouseScroll(deltaHorizontal, deltaVertical)
-        } else {
-            @Suppress("DEPRECATION")
-            window.mouseScroll(deltaVertical.coerceIn(-1.0, 1.0))
         }
     }
 

src/main/kotlin/gg/essential/elementa/components/ScrollComponent.kt

@@ -2,7 +2,6 @@ package gg.essential.elementa.components
 
 import gg.essential.elementa.ElementaVersion
 import gg.essential.elementa.UIComponent
-import gg.essential.elementa.components.UpdateFunc
 import gg.essential.elementa.constraints.*
 import gg.essential.elementa.constraints.animation.Animations
 import gg.essential.elementa.constraints.resolution.ConstraintVisitor
@@ -143,7 +142,29 @@ class ScrollComponent constructor(
 
 
     private val mouseScrollLambda: UIComponent.(UIScrollEvent) -> Unit = {
-        if (Window.of(this).version >= ElementaVersion.v5) {
+        if (Window.of(this).version >= ElementaVersion.v11) {
+            // For easier understanding we reframe the provided scroll in terms of primary and secondary directions
+            // We used to only get vertical scrolls, so that remains as the primary direction
+            val scrollPrimary = it.scrollY.toFloat()
+            val scrollSecondary = it.scrollX.toFloat()
+
+            // We map the primary and secondary scroll values based on the scroll direction
+            // `Vertical` and `Horizontal` directions disable the other direction, so we use 0f
+            val (providedX, providedY) = when (scrollDirection) {
+                Direction.Vertical -> 0f to scrollPrimary
+                Direction.Horizontal -> scrollPrimary to 0f
+                Direction.PreferVertical -> scrollSecondary to scrollPrimary
+                Direction.PreferHorizontal -> scrollPrimary to scrollSecondary
+            }
+
+            // We swap directions if shift is pressed
+            val (actualX, actualY) = if (UKeyboard.isShiftKeyDown()) providedY to providedX else providedX to providedY
+
+            // Finally, process the scroll with computed values
+            if (onScroll(actualX, actualY) || !passthroughScroll) {
+                it.stopPropagation()
+            }
+        } else if (Window.of(this).version >= ElementaVersion.v5) {
             // new behavior
             val scrollDirection = if (!UKeyboard.isShiftKeyDown()) primaryScrollDirection else secondaryScrollDirection
             if (scrollDirection != null) {
@@ -207,8 +228,7 @@ class ScrollComponent constructor(
 
         if (needsUpdate) {
             needsUpdate = false
-            val horizontalRange = calculateOffsetRange(isHorizontal = true)
-            val verticalRange = calculateOffsetRange(isHorizontal = false)
+            val (horizontalRange, verticalRange) = calculateOffsetRanges()
 
             // Recalculate our scroll box and move the content inside if needed.
             actualHolder.animate {
@@ -317,8 +337,7 @@ class ScrollComponent constructor(
         verticalOffset: Float = this.verticalOffset,
         smoothScroll: Boolean = true
     ) {
-        val horizontalRange = calculateOffsetRange(isHorizontal = true)
-        val verticalRange = calculateOffsetRange(isHorizontal = false)
+        val (horizontalRange, verticalRange) = calculateOffsetRanges()
         this.horizontalOffset =
             if (horizontalRange.isEmpty()) innerPadding else horizontalOffset.coerceIn(horizontalRange)
         this.verticalOffset = if (verticalRange.isEmpty()) {
@@ -425,13 +444,26 @@ class ScrollComponent constructor(
      * @return whether the offset changed
      */
     private fun onScroll(delta: Float, isHorizontal: Boolean): Boolean {
+        return if (isHorizontal) onScroll(delta, 0f) else onScroll(0f, delta)
+    }
+
+    /**
+     * @return whether either offset changed
+     */
+    private fun onScroll(scrollX: Float, scrollY: Float): Boolean {
         var changed = false
-        val offset = if (isHorizontal) ::horizontalOffset else ::verticalOffset
-        val range = calculateOffsetRange(isHorizontal)
-        val newOffset = if(range.isEmpty()) innerPadding else (offset.get() + delta * pixelsPerScroll * currentScrollAcceleration).coerceIn(range)
-        if (newOffset != offset.get()) {
+        val offsetX = ::horizontalOffset
+        val offsetY = ::verticalOffset
+        val (rangeX, rangeY) = calculateOffsetRanges()
+        val newOffsetX = if (rangeX.isEmpty()) innerPadding else (offsetX.get() + scrollX * pixelsPerScroll * currentScrollAcceleration).coerceIn(rangeX)
+        if (newOffsetX != offsetX.get()) {
             changed = true
-            offset.set(newOffset)
+            offsetX.set(newOffsetX)
+        }
+        val newOffsetY = if (rangeY.isEmpty()) innerPadding else (offsetY.get() + scrollY * pixelsPerScroll * currentScrollAcceleration).coerceIn(rangeY)
+        if (newOffsetY != offsetY.get()) {
+            changed = true
+            offsetY.set(newOffsetY)
         }
 
         currentScrollAcceleration =
@@ -511,16 +543,16 @@ class ScrollComponent constructor(
         }
     }
 
-    private fun calculateOffsetRange(isHorizontal: Boolean): ClosedFloatingPointRange<Float> {
-        return if (isHorizontal) {
-            val actualWidth = calculateActualWidth()
-            val maxNegative = this.getWidth() - actualWidth - innerPadding
-            if (horizontalScrollOpposite) (-innerPadding)..-maxNegative else maxNegative..(innerPadding)
-        } else {
-            val actualHeight = calculateActualHeight()
-            val maxNegative = this.getHeight() - actualHeight - innerPadding
-            if (verticalScrollOpposite) (-innerPadding)..-maxNegative else maxNegative..(innerPadding)
-        }
+    private fun calculateOffsetRanges(): Pair<ClosedFloatingPointRange<Float>, ClosedFloatingPointRange<Float>> {
+        val actualWidth = calculateActualWidth()
+        val maxNegativeWidth = this.getWidth() - actualWidth - innerPadding
+        val rangeX = if (horizontalScrollOpposite) (-innerPadding)..-maxNegativeWidth else maxNegativeWidth..(innerPadding)
+
+        val actualHeight = calculateActualHeight()
+        val maxNegativeHeight = this.getHeight() - actualHeight - innerPadding
+        val rangeY = if (verticalScrollOpposite) (-innerPadding)..-maxNegativeHeight else maxNegativeHeight..(innerPadding)
+
+        return rangeX to rangeY
     }
 
     private fun onClick(mouseX: Float, mouseY: Float, mouseButton: Int) {

src/main/kotlin/gg/essential/elementa/components/Window.kt

@@ -244,7 +244,7 @@ class Window @JvmOverloads constructor(
         super.mouseScroll(delta)
     }
 
-    override fun mouseScroll(deltaHorizontal: Double, deltaVertical: Double) {
+    override fun mouseScroll(scrollX: Double, scrollY: Double) {
         if (hasErrored && version >= ElementaVersion.v7) {
             return
         }
@@ -254,12 +254,12 @@ class Window @JvmOverloads constructor(
         val (mouseX, mouseY) = getMousePosition()
         for (floatingComponent in allFloatingComponentsInReverseOrder()) {
             if (floatingComponent.isPointInside(mouseX, mouseY)) {
-                floatingComponent.mouseScroll(deltaHorizontal, deltaVertical)
+                floatingComponent.mouseScroll(scrollX, scrollY)
                 return
             }
         }
 
-        super.mouseScroll(deltaHorizontal, deltaVertical)
+        super.mouseScroll(scrollX, scrollY)
     }
 
     override fun mouseClick(mouseX: Double, mouseY: Double, button: Int) {

src/main/kotlin/gg/essential/elementa/events/UIMouseEvents.kt

@@ -15,22 +15,25 @@ data class UIClickEvent(
 }
 
 data class UIScrollEvent(
-    val deltaVertical: Double,
+    val scrollY: Double,
     val target: UIComponent,
     val currentTarget: UIComponent,
-    val deltaHorizontal: Double,
+    val scrollX: Double, // only on Minecraft 1.20.2+ and ElementaVersion.V11+
 ) : UIEvent() {
+    constructor(scrollX: Double, scrollY: Double, target: UIComponent, currentTarget: UIComponent)
+            : this(scrollY, target, currentTarget, scrollX)
+
     // Added to ensure backwards binary compatibility
     constructor(delta: Double, target: UIComponent, currentTarget: UIComponent) : this(delta, target, currentTarget, 0.0)
 
     // Added to ensure backwards binary compatibility
     fun copy(
-        deltaVertical: Double = this.deltaVertical,
+        delta: Double = this.scrollY,
         target: UIComponent = this.target,
         currentTarget: UIComponent = this.currentTarget,
-    ) = copy(deltaVertical = deltaVertical, target = target, currentTarget = currentTarget, deltaHorizontal = deltaHorizontal)
+    ) = copy(scrollY = delta, target = target, currentTarget = currentTarget, scrollX = scrollX)
 
     // Added to ensure backwards binary compatibility
     val delta: Double
-        get() = deltaVertical
+        get() = scrollY
 }