Fix issues with TermuxActivityRootView margin adjustment

agnostic-apollo · agnostic-apollo · commit e5a9b99afe02 · 2021-06-30T02:31:47.000+05:00
Margin adjustment was causing screen flickering due to invalid values being calculated in landscape and split screen mode. Attempts to fix issue #2127
diff --git a/app/src/main/java/com/termux/app/TermuxActivity.java b/app/src/main/java/com/termux/app/TermuxActivity.java
@@ -203,6 +203,7 @@ public void onCreate(Bundle savedInstanceState) {
         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) -> {
diff --git a/app/src/main/java/com/termux/app/terminal/TermuxActivityRootView.java b/app/src/main/java/com/termux/app/terminal/TermuxActivityRootView.java
@@ -7,11 +7,13 @@
 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,23 +123,35 @@ public void onGlobalLayout() {
         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;
 
         Rect windowAvailableRect = windowAndViewRects[0];
         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)
-            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);
+        if (root_view_logging_enabled) {
+            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 void onGlobalLayout() {
             // set appropriate margins when views are changed quickly since some changes
             // may be missed.
             if (isVisibleBecauseMargin) {
-                if (ROOT_VIEW_LOGGING_ENABLED)
-                    Logger.logVerbose(LOG_TAG, "onGlobalLayout: Visible due to margin");
+                if (root_view_logging_enabled)
+                    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 void onGlobalLayout() {
             // 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)
-                    Logger.logVerbose(LOG_TAG, "onGlobalLayout: Resetting margin since visible due to extra margin");
-                setMargin = true;
-                // lastMarginBottom must be invalid. May also happen when keyboards are changed.
-                lastMarginBottom = null;
+                // Adding time check since prevents infinite loops, like in landscape mode in freeform mode in Taskbar
+                if ((System.currentTimeMillis() - lastMarginBottomExtraTime) > 40) {
+                    if (root_view_logging_enabled)
+                        Logger.logVerbose(LOG_TAG, "Resetting margin since visible due to extra margin");
+                    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)
-                    Logger.logVerbose(LOG_TAG, "onGlobalLayout: Setting bottom margin to 0");
+                if (root_view_logging_enabled)
+                    Logger.logVerbose(LOG_TAG, "Setting bottom margin to 0");
                 params.setMargins(0, 0, 0, 0);
                 setLayoutParams(params);
             } else {
-                if (ROOT_VIEW_LOGGING_ENABLED)
-                    Logger.logVerbose(LOG_TAG, "onGlobalLayout: Bottom margin already equals 0");
+                if (root_view_logging_enabled)
+                    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 void onGlobalLayout() {
         // 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 void onGlobalLayout() {
             // 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 (ROOT_VIEW_LOGGING_ENABLED)
-                    Logger.logVerbose(LOG_TAG, "onGlobalLayout: Force setting margin since not visible despite margin");
+            if (pxHidden > 0 && params.bottomMargin > 0) {
+                if (pxHidden != params.bottomMargin) {
+                    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)
-                    Logger.logVerbose(LOG_TAG, "onGlobalLayout: Setting bottom margin to " + pxHidden);
+                if (root_view_logging_enabled)
+                    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)
-                    Logger.logVerbose(LOG_TAG, "onGlobalLayout: Bottom margin already equals " + pxHidden);
+                if (root_view_logging_enabled)
+                    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);
+        }
+    }
+
 }
diff --git a/app/src/main/java/com/termux/app/terminal/TermuxTerminalViewClient.java b/app/src/main/java/com/termux/app/terminal/TermuxTerminalViewClient.java
@@ -35,6 +35,7 @@
 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 void onStart() {
 
         // Piggyback on the terminal view key logging toggle for now, should add a separate toggle in future
         mActivity.getTermuxActivityRootView().setIsRootViewLoggingEnabled(isTerminalViewKeyLoggingEnabled);
+        ViewUtils.setIsViewUtilsLoggingEnabled(isTerminalViewKeyLoggingEnabled);
     }
 
     /**
diff --git a/termux-shared/build.gradle b/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"
diff --git a/termux-shared/src/main/java/com/termux/shared/view/ViewUtils.java b/termux-shared/src/main/java/com/termux/shared/view/ViewUtils.java
