chrisbanes / insetter

Insetter is a library to help apps handle WindowInsets more easily
https://chrisbanes.github.io/insetter
Apache License 2.0
1.13k stars 42 forks source link

Missing requestLayout() after setting new margin #42

Closed hrach closed 1 month ago

hrach commented 4 years ago

As specified in doc, setMargins() requires to call requestLayout after that.

Currently, it works "almost" without it, just on older LG devices (Android 5.1 and 6) views are not updated until I call this manually.

hrach commented 4 years ago

I've tried to add requestLayout into lib, but id didn't help. The issue is the setting margin/request layout happends onMeasure/onApplyInsets callbacks. When I requestLayout later (e.g. in button click listener), everything works. Reproducible on my LGE devices, it works elsewhere.

killvetrov commented 4 years ago

I ran into this issue too on pre-Android 8 devices. In my case workaround was to use padding insets instead of margin insets. You are completely right about requestLayout, I think the following may work:

view.post(new Runnable() {
            @Override
            public void run() {
                view.requestLayout();
            }
        });

This way requestLayout is delayed until the next frame, guaranteed to be after onMeasure/onApplyInsets calls return.

chrisbanes commented 4 years ago

Interesting. Can you provide an example of a layout which didn't work?

Insets dispatch happens before onMeasure and onLayout (at least it should), therefore an explicit call to requestLayout should work. Having to post() shouldn't be needed unless something weird is happening.

hrach commented 4 years ago

Ok, here is a testing code, which works on my OnePlus 7 (A10) and doesn't on LG V10 (A6):

Source code MainActivity.kt ```kotlin package test.app.insetactivity import android.os.Bundle import androidx.appcompat.app.AppCompatActivity import dev.chrisbanes.insetter.setEdgeToEdgeSystemUiFlags import kotlinx.android.synthetic.main.activity_main.* class MainActivity : AppCompatActivity() { override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) setContentView(R.layout.activity_main) root.setEdgeToEdgeSystemUiFlags(true) btn_layout.setOnClickListener { tv.requestLayout() } } } ``` activity_main.xml ```

Maybe the important aspect is adjustResize (🤷‍♂️).

The actuall call stack is:

image

chrisbanes commented 4 years ago

This looks to be due to the weird way in which ActionBarOverlayLayout is implemented. It waits for it's measure before re-dispatching insets, but the child has already been measured. child.requestLayout() at this point won't do anything since we're already in a layout pass.

What ActionBarOverlayLayout should really do is re-measure in case the child has changed. The window decor action bar is mostly deprecated at this stage though, so the easiest fix is to move to using Toolbar.

hrach commented 4 years ago

Well, sorry, this was not a "minimal" repro, here is another trial: the same code just with

<item name="windowActionBar">false</item>
<item name="windowNoTitle">true</item>

still does not work on older LG devices:

image

mercuriy94 commented 4 years ago

Hello! Appcompat

Fixed an issue where ActionBarOverlayLayout (window decor action) was not dispatching WindowInsets correctly.

Check, please! Maybe the fix will be useful to you!

killvetrov commented 4 years ago

Hopefully PR #75 fixes this issue. Can someone with reproducible case test it?

ChairfaceChippendale commented 4 years ago

it still doesn't work correctly. Here is an example in my test project: https://github.com/ChairfaceChippendale/EasyEngMaterial/blob/dev-kotlin/flow/catalog/src/main/java/com/ujujzk/ee/catalog/CatalogFragment.kt both rtToolbar.applySystemWindowInsetsToMargin(top = true) and addBtn.applySystemWindowInsetsToMargin(bottom = true) have no effect until you interact with the UI.

killvetrov commented 4 years ago

@ChairfaceChippendale you are depending on v0.3.1, while this fix is not yet released. Quick option to test it is by using JitPack dependency implementation 'com.github.chrisbanes:insetter:main-SNAPSHOT'

killvetrov commented 4 years ago

@ChairfaceChippendale I just tested your kindly provided example with current snapshot on SDK 23 device. I opened Catalog screen repeatedly, and it's getting better, but it's still kind of hit and miss. Seems like there's race condition with layout of view and parent, because sometimes layout is correct and other times not. I think parent requestLayout call should be postponed by View.post. I'll test it and report.

ChairfaceChippendale commented 4 years ago

Yes, you might see that there is commented code with postUpdateLayoutParams and it really works. I've tried requestLayout() it doesn't help much

ChairfaceChippendale commented 4 years ago

BTW, I test it on the emulator with SDK 29

killvetrov commented 4 years ago

It's unfortunate, earlier I believed Insetter was reliable on newer Android versions. In fact, it's always reliable with padding insets, but not with margins. Indeed your example is reproducible on SDK 29. I've spent a few hours debugging this issue and I still can't figure the clear cause of this problem. Layout inspector shows updated margin on the view, but view position is somehow unaffected until next layout pass. I've encountered this in my projects too.

I can think of some possible causes like special treatment of margins in ConstraintLayout and presence of FragmentContainerView, which has additional code for dispatching insets. I feel that Insetter ends up updating layout params somewhere in between of layout pass, which leads to this problem.

Anyway, as the result of my tests I've found the following workaround:

View v = view;
while (v.getParent() instanceof View) {
  v = (View) v.getParent();
  v.forceLayout();
}

I added this code after setLayoutParams call https://github.com/chrisbanes/insetter/blob/7a3f0700b1b030c5d47054c833c07755cbb6ccff/library/src/main/java/dev/chrisbanes/insetter/Insetter.java#L444

This forces layout pass of entire view hierarchy to happen on the next frame, when all margins are already updated by Insetter. In theory, requestLayout() has the code to do the same, traversing hierarchy with getParent().requestLayout(), but at the time Insetter calls it getParent().isLayoutRequested() returns true, and condition is not satisfied.

@ChairfaceChippendale the code you commented out with post does mitigate this issue, but produces glitched display. At first views are displayed with incorrect margin, and then jump to correct position after a delay.

You can test my workaround (which doesn't produce jumps) with JitPack implementation 'com.github.killvetrov:insetter:force-layout-SNAPSHOT'

@chrisbanes I'm sure that greater expertise in view measuring/layout and debugging of ConstraintLayout's internals should lead to the real cause and a more efficient fix.

killvetrov commented 4 years ago

As specified in doc, setMargins() requires to call requestLayout after that.

@hrach by the way, Insetter does not use setMargins. It updates margins in LayoutParams directly and follows with setLayoutParams, which calls requestLayout.

killvetrov commented 4 years ago

UPDATE: ConstraintLayout 2.0.3 just released https://androidstudio.googleblog.com/2020/10/constraintlayout-203.html It has fixes related to insets handling: https://issuetracker.google.com/issues?q=169262776

Great news is that this release seems to fix this issue! I've tried it with example provided by @ChairfaceChippendale and Insetter v0.3.1 and it works flawlessly now.

ChairfaceChippendale commented 4 years ago

ConstraintLayout 2.0.3 works great! And it somehow fixes this issue also for the RelativeLayout

SamYStudiO commented 3 years ago

Hi ! There is still something wrong with layout pass and it doesn't seem to be margin specific as i get it with padding as well.

Here is a screenshot of layout inspector and emulator

insets_problem

As you can see title is not inseted as expected and i'm using applySystemWindowInsetsToPadding. Once i make later a requestLayout() on parent everything is displayed as normal like on layout inspector

I'm using ConstraintLayout 2.0.4, pb is the same by using insetter library or doing it manually.