Fix issues with TermuxActivityRootView margin adjustment

Margin adjustment was causing screen flickering due to invalid values being calculated in landscape and split screen mode.

Attempts to fix issue #2127

app/src/main/java/com/termux/app/TermuxActivity.java

@@ -203,6 +203,7 @@ public final class TermuxActivity extends Activity implements ServiceConnection
         mTermuxActivityRootView = findViewById(R.id.activity_termux_root_view);
         mTermuxActivityRootView.setActivity(this);
         mTermuxActivityBottomSpaceView = findViewById(R.id.activity_termux_bottom_space_view);
+        mTermuxActivityRootView.setOnApplyWindowInsetsListener(new TermuxActivityRootView.WindowInsetsListener());
 
         View content = findViewById(android.R.id.content);
         content.setOnApplyWindowInsetsListener((v, insets) -> {

app/src/main/java/com/termux/app/terminal/TermuxActivityRootView.java

@@ -7,11 +7,13 @@ import android.util.AttributeSet;
 import android.view.View;
 import android.view.ViewGroup;
 import android.view.ViewTreeObserver;
+import android.view.WindowInsets;
 import android.view.inputmethod.EditorInfo;
 import android.widget.FrameLayout;
 import android.widget.LinearLayout;
 
 import androidx.annotation.Nullable;
+import androidx.core.view.WindowInsetsCompat;
 
 import com.termux.app.TermuxActivity;
 import com.termux.shared.logger.Logger;
@@ -64,15 +66,18 @@ public class TermuxActivityRootView extends LinearLayout implements ViewTreeObse
     public TermuxActivity mActivity;
     public Integer marginBottom;
     public Integer lastMarginBottom;
+    public long lastMarginBottomTime;
+    public long lastMarginBottomExtraTime;
 
     /** Log root view events. */
     private boolean ROOT_VIEW_LOGGING_ENABLED = false;
 
     private static final String LOG_TAG = "TermuxActivityRootView";
 
+    private static int mStatusBarHeight;
+
     public TermuxActivityRootView(Context context) {
         super(context);
-
     }
 
     public TermuxActivityRootView(Context context, @Nullable AttributeSet attrs) {
@@ -118,10 +123,15 @@ public class TermuxActivityRootView extends LinearLayout implements ViewTreeObse
         View bottomSpaceView = mActivity.getTermuxActivityBottomSpaceView();
         if (bottomSpaceView == null) return;
 
+        boolean root_view_logging_enabled = ROOT_VIEW_LOGGING_ENABLED;
+
+        if (root_view_logging_enabled)
+            Logger.logVerbose(LOG_TAG, ":\nonGlobalLayout:");
+
         FrameLayout.LayoutParams params = (FrameLayout.LayoutParams) getLayoutParams();
 
         // Get the position Rects of the bottom space view and the main window holding it
-        Rect[] windowAndViewRects = ViewUtils.getWindowAndViewRects(bottomSpaceView);
+        Rect[] windowAndViewRects = ViewUtils.getWindowAndViewRects(bottomSpaceView, mStatusBarHeight);
         if (windowAndViewRects == null)
             return;
 
@@ -129,12 +139,19 @@ public class TermuxActivityRootView extends LinearLayout implements ViewTreeObse
         Rect bottomSpaceViewRect = windowAndViewRects[1];
 
         // If the bottomSpaceViewRect is inside the windowAvailableRect, then it must be completely visible
-        boolean isVisible = windowAvailableRect.contains(bottomSpaceViewRect);
+        //boolean isVisible = windowAvailableRect.contains(bottomSpaceViewRect); // rect.right comparison often fails in landscape
+        boolean isVisible = ViewUtils.isRectAbove(windowAvailableRect, bottomSpaceViewRect);
         boolean isVisibleBecauseMargin = (windowAvailableRect.bottom == bottomSpaceViewRect.bottom) && params.bottomMargin > 0;
-        boolean isVisibleBecauseExtraMargin = (bottomSpaceViewRect.bottom - windowAvailableRect.bottom) < 0;
+        boolean isVisibleBecauseExtraMargin = ((bottomSpaceViewRect.bottom - windowAvailableRect.bottom) < 0);
 
-        if (ROOT_VIEW_LOGGING_ENABLED)
+        if (root_view_logging_enabled) {
-            Logger.logVerbose(LOG_TAG, "onGlobalLayout: windowAvailableRect " + windowAvailableRect.bottom + ", bottomSpaceViewRect " + bottomSpaceViewRect.bottom + ", diff " + (bottomSpaceViewRect.bottom - windowAvailableRect.bottom) + ", bottom " + params.bottomMargin + ", isVisible " + isVisible + ", isVisibleBecauseMargin " + isVisibleBecauseMargin + ", isVisibleBecauseExtraMargin " + isVisibleBecauseExtraMargin);
+            Logger.logVerbose(LOG_TAG, "windowAvailableRect " + ViewUtils.toRectString(windowAvailableRect) + ", bottomSpaceViewRect " + ViewUtils.toRectString(bottomSpaceViewRect));
+            Logger.logVerbose(LOG_TAG, "windowAvailableRect.bottom " + windowAvailableRect.bottom +
+                ", bottomSpaceViewRect.bottom " +bottomSpaceViewRect.bottom +
+                ", diff " + (bottomSpaceViewRect.bottom - windowAvailableRect.bottom) + ", bottom " + params.bottomMargin +
+                ", isVisible " + windowAvailableRect.contains(bottomSpaceViewRect) + ", isRectAbove " + ViewUtils.isRectAbove(windowAvailableRect, bottomSpaceViewRect) +
+                ", isVisibleBecauseMargin " + isVisibleBecauseMargin + ", isVisibleBecauseExtraMargin " + isVisibleBecauseExtraMargin);
+        }
 
         // If the bottomSpaceViewRect is visible, then remove the margin if needed
         if (isVisible) {
@@ -148,15 +165,25 @@ public class TermuxActivityRootView extends LinearLayout implements ViewTreeObse
             // set appropriate margins when views are changed quickly since some changes
             // may be missed.
             if (isVisibleBecauseMargin) {
-                if (ROOT_VIEW_LOGGING_ENABLED)
+                if (root_view_logging_enabled)
-                    Logger.logVerbose(LOG_TAG, "onGlobalLayout: Visible due to margin");
+                    Logger.logVerbose(LOG_TAG, "Visible due to margin");
+
                 // Once the view has been redrawn with new margin, we set margin back to 0 so that
                 // when next time onMeasure() is called, margin 0 is used. This is necessary for
                 // cases when view has been redrawn with new margin because bottom space view was
                 // hidden by keyboard and then view was redrawn again due to layout change (like
                 // keyboard symbol view is switched to), android will add margin below its new position
                 // if its greater than 0, which was already above the keyboard creating x2x margin.
-                marginBottom = 0;
+                // Adding time check since moving split screen divider in landscape causes jitter
+                // and prevents some infinite loops
+                if ((System.currentTimeMillis() - lastMarginBottomTime) > 40) {
+                    lastMarginBottomTime = System.currentTimeMillis();
+                    marginBottom = 0;
+                } else {
+                    if (root_view_logging_enabled)
+                        Logger.logVerbose(LOG_TAG, "Ignoring restoring marginBottom to 0 since called to quickly");
+                }
+
                 return;
             }
 
@@ -166,21 +193,28 @@ public class TermuxActivityRootView extends LinearLayout implements ViewTreeObse
             // onGlobalLayout: windowAvailableRect 1408, bottomSpaceViewRect 1232, diff -176, bottom 0, isVisible true, isVisibleBecauseMargin false, isVisibleBecauseExtraMargin false
             // onGlobalLayout: Bottom margin already equals 0
             if (isVisibleBecauseExtraMargin) {
-                if (ROOT_VIEW_LOGGING_ENABLED)
+                // Adding time check since prevents infinite loops, like in landscape mode in freeform mode in Taskbar
-                    Logger.logVerbose(LOG_TAG, "onGlobalLayout: Resetting margin since visible due to extra margin");
+                if ((System.currentTimeMillis() - lastMarginBottomExtraTime) > 40) {
-                setMargin = true;
+                    if (root_view_logging_enabled)
-                // lastMarginBottom must be invalid. May also happen when keyboards are changed.
+                        Logger.logVerbose(LOG_TAG, "Resetting margin since visible due to extra margin");
-                lastMarginBottom = null;
+                    lastMarginBottomExtraTime = System.currentTimeMillis();
+                    // lastMarginBottom must be invalid. May also happen when keyboards are changed.
+                    lastMarginBottom = null;
+                    setMargin = true;
+                } else {
+                    if (root_view_logging_enabled)
+                        Logger.logVerbose(LOG_TAG, "Ignoring resetting margin since visible due to extra margin since called to quickly");
+                }
             }
 
             if (setMargin) {
-                if (ROOT_VIEW_LOGGING_ENABLED)
+                if (root_view_logging_enabled)
-                    Logger.logVerbose(LOG_TAG, "onGlobalLayout: Setting bottom margin to 0");
+                    Logger.logVerbose(LOG_TAG, "Setting bottom margin to 0");
                 params.setMargins(0, 0, 0, 0);
                 setLayoutParams(params);
             } else {
-                if (ROOT_VIEW_LOGGING_ENABLED)
+                if (root_view_logging_enabled)
-                    Logger.logVerbose(LOG_TAG, "onGlobalLayout: Bottom margin already equals 0");
+                    Logger.logVerbose(LOG_TAG, "Bottom margin already equals 0");
                 // This is done so that when next time onMeasure() is called, lastMarginBottom is used.
                 // This is done since we **expect** the keyboard to have same dimensions next time layout
                 // changes, so best set margin while view is drawn the first time, otherwise it will
@@ -193,8 +227,9 @@ public class TermuxActivityRootView extends LinearLayout implements ViewTreeObse
         // ELse find the part of the extra keys/terminal that is hidden and add a margin accordingly
         else {
             int pxHidden = bottomSpaceViewRect.bottom - windowAvailableRect.bottom;
-            if (ROOT_VIEW_LOGGING_ENABLED)
+
-                Logger.logVerbose(LOG_TAG, "onGlobalLayout: pxHidden " + pxHidden + ", bottom " + params.bottomMargin);
+            if (root_view_logging_enabled)
+                Logger.logVerbose(LOG_TAG, "pxHidden " + pxHidden + ", bottom " + params.bottomMargin);
 
             boolean setMargin = params.bottomMargin != pxHidden;
 
@@ -205,23 +240,45 @@ public class TermuxActivityRootView extends LinearLayout implements ViewTreeObse
             // onMeasure: Setting bottom margin to 176
             // onGlobalLayout: windowAvailableRect 1232, bottomSpaceViewRect 1408, diff 176, bottom 176, isVisible false, isVisibleBecauseMargin false, isVisibleBecauseExtraMargin false
             // onGlobalLayout: Bottom margin already equals 176
-            if ((bottomSpaceViewRect.bottom - windowAvailableRect.bottom) > 0) {
+            if (pxHidden > 0 && params.bottomMargin > 0) {
-                if (ROOT_VIEW_LOGGING_ENABLED)
+                if (pxHidden != params.bottomMargin) {
-                    Logger.logVerbose(LOG_TAG, "onGlobalLayout: Force setting margin since not visible despite margin");
+                    if (root_view_logging_enabled)
+                        Logger.logVerbose(LOG_TAG, "Force setting margin to 0 since not visible due to wrong margin");
+                    pxHidden = 0;
+                } else {
+                    if (root_view_logging_enabled)
+                        Logger.logVerbose(LOG_TAG, "Force setting margin since not visible despite required margin");
+                }
                 setMargin = true;
             }
 
+            if (pxHidden  < 0) {
+                if (root_view_logging_enabled)
+                    Logger.logVerbose(LOG_TAG, "Force setting margin to 0 since new margin is negative");
+                pxHidden = 0;
+            }
+
+
             if (setMargin) {
-                if (ROOT_VIEW_LOGGING_ENABLED)
+                if (root_view_logging_enabled)
-                    Logger.logVerbose(LOG_TAG, "onGlobalLayout: Setting bottom margin to " + pxHidden);
+                    Logger.logVerbose(LOG_TAG, "Setting bottom margin to " + pxHidden);
                 params.setMargins(0, 0, 0, pxHidden);
                 setLayoutParams(params);
                 lastMarginBottom = pxHidden;
             } else {
-                if (ROOT_VIEW_LOGGING_ENABLED)
+                if (root_view_logging_enabled)
-                    Logger.logVerbose(LOG_TAG, "onGlobalLayout: Bottom margin already equals " + pxHidden);
+                    Logger.logVerbose(LOG_TAG, "Bottom margin already equals " + pxHidden);
             }
         }
     }
 
+    public static class WindowInsetsListener implements View.OnApplyWindowInsetsListener {
+        @Override
+        public WindowInsets onApplyWindowInsets(View v, WindowInsets insets) {
+            mStatusBarHeight =  WindowInsetsCompat.toWindowInsetsCompat(insets).getInsets(WindowInsetsCompat.Type.statusBars()).top;
+            // Let view window handle insets however it wants
+            return v.onApplyWindowInsets(insets);
+        }
+    }
+
 }

app/src/main/java/com/termux/app/terminal/TermuxTerminalViewClient.java

@@ -35,6 +35,7 @@ import com.termux.shared.logger.Logger;
 import com.termux.shared.markdown.MarkdownUtils;
 import com.termux.shared.termux.TermuxUtils;
 import com.termux.shared.view.KeyboardUtils;
+import com.termux.shared.view.ViewUtils;
 import com.termux.terminal.KeyHandler;
 import com.termux.terminal.TerminalEmulator;
 import com.termux.terminal.TerminalSession;
@@ -88,6 +89,7 @@ public class TermuxTerminalViewClient extends TermuxTerminalViewClientBase {
 
         // Piggyback on the terminal view key logging toggle for now, should add a separate toggle in future
         mActivity.getTermuxActivityRootView().setIsRootViewLoggingEnabled(isTerminalViewKeyLoggingEnabled);
+        ViewUtils.setIsViewUtilsLoggingEnabled(isTerminalViewKeyLoggingEnabled);
     }
 
     /**

termux-shared/build.gradle

@@ -8,6 +8,7 @@ android {
         implementation 'androidx.appcompat:appcompat:1.2.0'
         implementation "androidx.annotation:annotation:1.2.0"
         implementation "androidx.core:core:1.5.0-rc01"
+        implementation "androidx.window:window:1.0.0-alpha08"
         implementation "com.google.guava:guava:24.1-jre"
         implementation "io.noties.markwon:core:$markwonVersion"
         implementation "io.noties.markwon:ext-strikethrough:$markwonVersion"

termux-shared/src/main/java/com/termux/shared/view/ViewUtils.java

@@ -3,25 +3,45 @@ package com.termux.shared.view;
 import android.app.Activity;
 import android.content.Context;
 import android.content.ContextWrapper;
+import android.content.res.Configuration;
+import android.graphics.Point;
 import android.graphics.Rect;
 import android.util.TypedValue;
 import android.view.View;
 
+import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.appcompat.app.AppCompatActivity;
 
+import com.termux.shared.logger.Logger;
+
 public class ViewUtils {
 
+    /** Log root view events. */
+    public static boolean VIEW_UTILS_LOGGING_ENABLED = false;
+
+    private static final String LOG_TAG = "ViewUtils";
+
+    /**
+     * Sets whether view utils logging is enabled or not.
+     *
+     * @param value The boolean value that defines the state.
+     */
+    public static void setIsViewUtilsLoggingEnabled(boolean value) {
+        VIEW_UTILS_LOGGING_ENABLED = value;
+    }
+
     /**
      * Check if a {@link View} is fully visible and not hidden or partially covered by another view.
      *
      * https://stackoverflow.com/a/51078418/14686958
      *
      * @param view The {@link View} to check.
+     * @param statusBarHeight The status bar height received by {@link View.OnApplyWindowInsetsListener}.
      * @return Returns {@code true} if view is fully visible.
      */
-    public static boolean isViewFullyVisible(View view) {
+    public static boolean isViewFullyVisible(View view, int statusBarHeight) {
-        Rect[] windowAndViewRects = getWindowAndViewRects(view);
+        Rect[] windowAndViewRects = getWindowAndViewRects(view, statusBarHeight);
         if (windowAndViewRects == null)
             return false;
         return windowAndViewRects[0].contains(windowAndViewRects[1]);
@@ -34,15 +54,18 @@ public class ViewUtils {
      * https://stackoverflow.com/a/51078418/14686958
      *
      * @param view The {@link View} inside the window whose {@link Rect} to get.
+     * @param statusBarHeight The status bar height received by {@link View.OnApplyWindowInsetsListener}.
      * @return Returns {@link Rect[]} if view is visible where Rect[0] will contain window
      * {@link Rect} and Rect[1] will contain view {@link Rect}. This will be {@code null}
      * if view is not visible.
      */
     @Nullable
-    public static Rect[] getWindowAndViewRects(View view) {
+    public static Rect[] getWindowAndViewRects(View view, int statusBarHeight) {
         if (view == null || !view.isShown())
             return null;
 
+        boolean view_utils_logging_enabled = VIEW_UTILS_LOGGING_ENABLED;
+
         // windowRect - will hold available area where content remain visible to users
         // Takes into account screen decorations (e.g. statusbar)
         Rect windowRect = new Rect();
@@ -50,15 +73,20 @@ public class ViewUtils {
 
         // If there is actionbar, get his height
         int actionBarHeight = 0;
+        boolean isInMultiWindowMode = false;
         Context context = view.getContext();
         if (context instanceof AppCompatActivity) {
             androidx.appcompat.app.ActionBar actionBar = ((AppCompatActivity) context).getSupportActionBar();
             if (actionBar != null) actionBarHeight = actionBar.getHeight();
+            isInMultiWindowMode = ((AppCompatActivity) context).isInMultiWindowMode();
         } else if (context instanceof Activity) {
             android.app.ActionBar actionBar = ((Activity) context).getActionBar();
             if (actionBar != null) actionBarHeight = actionBar.getHeight();
+            isInMultiWindowMode = ((Activity) context).isInMultiWindowMode();
         }
 
+        int displayOrientation = getDisplayOrientation(context);
+
         // windowAvailableRect - takes into account actionbar and statusbar height
         Rect windowAvailableRect;
         windowAvailableRect = new Rect(windowRect.left, windowRect.top + actionBarHeight, windowRect.right, windowRect.bottom);
@@ -71,13 +99,108 @@ public class ViewUtils {
         view.getLocationInWindow(viewsLocationInWindow);
         int viewLeft = viewsLocationInWindow[0];
         int viewTop = viewsLocationInWindow[1];
+
+        if (view_utils_logging_enabled) {
+            Logger.logVerbose(LOG_TAG, "getWindowAndViewRects:");
+            Logger.logVerbose(LOG_TAG, "windowRect: " + toRectString(windowRect) + ", windowAvailableRect: " + toRectString(windowAvailableRect));
+            Logger.logVerbose(LOG_TAG, "viewsLocationInWindow: " + toPointString(new Point(viewLeft, viewTop)));
+            Logger.logVerbose(LOG_TAG, "activitySize: " + toPointString(getDisplaySize(context, true)) +
+                ", displaySize: " + toPointString(getDisplaySize(context, false)) +
+                ", displayOrientation=" + displayOrientation);
+        }
+
+        if (isInMultiWindowMode) {
+            if (displayOrientation == Configuration.ORIENTATION_PORTRAIT) {
+                // The windowRect.top of the window at the of split screen mode should start right
+                // below the status bar
+                if (statusBarHeight != windowRect.top) {
+                    if (view_utils_logging_enabled)
+                        Logger.logVerbose(LOG_TAG, "Window top does not equal statusBarHeight " + statusBarHeight + " in multi-window portrait mode. Window is possibly bottom app in split screen mode. Adding windowRect.top " + windowRect.top + " to viewTop.");
+                    viewTop += windowRect.top;
+                } else {
+                    if (view_utils_logging_enabled)
+                        Logger.logVerbose(LOG_TAG, "windowRect.top equals statusBarHeight " + statusBarHeight + " in multi-window portrait mode. Window is possibly top app in split screen mode.");
+                }
+
+            } else if (displayOrientation == Configuration.ORIENTATION_LANDSCAPE) {
+                // If window is on the right in landscape mode of split screen, the viewLeft actually
+                // starts at windowRect.left instead of 0 returned by getLocationInWindow
+                viewLeft += windowRect.left;
+            }
+        }
+
         int viewRight = viewLeft + view.getWidth();
         int viewBottom = viewTop + view.getHeight();
         viewRect = new Rect(viewLeft, viewTop, viewRight, viewBottom);
 
+        if (displayOrientation == Configuration.ORIENTATION_LANDSCAPE && viewRight > windowAvailableRect.right) {
+            if (view_utils_logging_enabled)
+                Logger.logVerbose(LOG_TAG, "viewRight " + viewRight + " is greater than windowAvailableRect.right " + windowAvailableRect.right + " in landscape mode. Setting windowAvailableRect.right to viewRight since it may not include navbar height.");
+            windowAvailableRect.right = viewRight;
+        }
+
         return new Rect[]{windowAvailableRect, viewRect};
     }
 
+    /**
+     * Check if {@link Rect} r2 is above r2. An empty rectangle never contains another rectangle.
+     *
+     * @param r1 The base rectangle.
+     * @param r2 The rectangle being tested that should be above.
+     * @return Returns {@code true} if r2 is above r1.
+     */
+    public static boolean isRectAbove(@NonNull Rect r1, @NonNull Rect r2) {
+        // check for empty first
+        return r1.left < r1.right && r1.top < r1.bottom
+            // now check if above
+            && r1.left <= r2.left && r1.bottom >= r2.bottom;
+    }
+
+    /**
+     * Get device orientation.
+     *
+     * Related: https://stackoverflow.com/a/29392593/14686958
+     *
+     * @param context The {@link Context} to check with.
+     * @return {@link Configuration#ORIENTATION_PORTRAIT} or {@link Configuration#ORIENTATION_LANDSCAPE}.
+     */
+    public static int getDisplayOrientation(@NonNull Context context) {
+        Point size = getDisplaySize(context, false);
+        return (size.x < size.y) ? Configuration.ORIENTATION_PORTRAIT : Configuration.ORIENTATION_LANDSCAPE;
+    }
+
+    /**
+     * Get device display size.
+     *
+     * @param context The {@link Context} to check with.
+     * @param activitySize The set to {@link true}, then size returned will be that of the activity
+     *                     and can be smaller than physical display size in multi-window mode.
+     * @return Returns the display size as {@link Point}.
+     */
+    public static Point getDisplaySize( @NonNull Context context, boolean activitySize) {
+        // android.view.WindowManager.getDefaultDisplay() and Display.getSize() are deprecated in
+        // API 30 and give wrong values in API 30 for activitySize=false in multi-window
+        androidx.window.WindowManager windowManager = new androidx.window.WindowManager(context);
+        androidx.window.WindowMetrics windowMetrics;
+        if (activitySize)
+            windowMetrics = windowManager.getCurrentWindowMetrics();
+        else
+            windowMetrics = windowManager.getMaximumWindowMetrics();
+        return new Point(windowMetrics.getBounds().width(), windowMetrics.getBounds().height());
+    }
+
+    /** Convert {@link Rect} to {@link String}. */
+    public static String toRectString(Rect rect) {
+        if (rect == null) return "null";
+        return "(" + rect.left + "," + rect.top + "), (" + rect.right + "," + rect.bottom + ")";
+    }
+
+    /** Convert {@link Point} to {@link String}. */
+    public static String toPointString(Point point) {
+        if (point == null) return "null";
+        return "(" + point.x + "," + point.y + ")";
+    }
+
     /** Get the {@link Activity} associated with the {@link Context} if available. */
     @Nullable
     public static Activity getActivity(Context context) {