Unity-UI-Extensions / com.unity.uiextensions

https://unity-ui-extensions.github.io/
1.21k stars 129 forks source link

HorizontalScrollSnap size calc error #80

Closed SimonDarksideJ closed 2 years ago

SimonDarksideJ commented 2 years ago

Issue created by 李扬 as Bitbucket Issue #​80 on 2016.06.08 08:40. hi, i have tried HorizontalScrollSnap , it's very nice. a little bug, when i set the anchor presets of the scrollrect gameobject to stretch (horizontal), the size of child object would calculate to a wrong value. it may caused by

Vector2 panelDimensions = gameObject.GetComponent<RectTransform>().sizeDelta;

at line 228, as the sizeDelta would be zero or neg value when the anchor was set to stretch.

i guess

Vector2 panelDimensions = gameObject.GetComponent<RectTransform>().rect.size;

should be the same effect, and the value would be right in the case above.

SimonDarksideJ commented 2 years ago

On 2016.06.12 07:23, 李扬 commented: another question: the position of ScrollRect was calculated using:

_scroll_rect.horizontalNormalizedPosition = (float)(_currentScreen) / (_screens - 1);

but at Start() function, it was wrote

_currentScreen = StartingScreen;
_scroll_rect.horizontalNormalizedPosition = (float)(_currentScreen - 1) / (_screens - 1);

and the StartingScreen was set to 1 by default (cannot change to 0 in editor), this will make a bug (the Pagination flag will focus on the "second" item while the scroll rect show the "first" page).

i doubt if the strange code was to avoid some other problem?

SimonDarksideJ commented 2 years ago

On 2016.07.29 16:17, Utku Sipahioğlu commented: Yeah, in Unity 5.4 RectTransform.sizeDelta returns (0, 0) vector if your RectTransform is stretching. Idk if it's a Unity 5.4 bug, maybe it's a API change. In Unity 5.3 it was returning the sizeDelta vector correctly.

SimonDarksideJ commented 2 years ago

On 2016.07.30 00:47, @SimonDarksideJ commented: Ok, seeing a few changes / fixes going in to 5.4. Will test and update accordingly. Thanks for the pointers

SimonDarksideJ commented 2 years ago

On 2016.11.15 18:10, @SimonDarksideJ commented: Testing in 5.4.2 as not caused any issues. Seems the "bug" was fixed. Can anyone re-test either with updated source, or after the next asset update?

SimonDarksideJ commented 2 years ago

On 2016.12.21 18:35, @SimonDarksideJ commented: @rugbbyli @TeorikDeli do you want to try the latest update in source. The control has almost been completely rewritten now.

SimonDarksideJ commented 2 years ago

On 2016.12.22 07:12, 李扬 commented: @simonjackson thanks, i will try it later.

SimonDarksideJ commented 2 years ago

On 2016.12.29 00:40, Mikko Karvonen commented: I'm using stretched content with HSS (i.e., UI Panels that are stretched to fill the whole screen). All this content is loaded dynamically from different Scenes / Asset Bundles. So at app start the initial number of children for the HSS component is zero. I'm still using a patched (by me) version of a version from September 2016 (I think from this commit), because I've had issues with newer versions.

This is how I managed to get the stretched content to work: by adding the following two lines after child.pivot = ... in DistributePages():

child.offsetMin = new Vector2(child.offsetMin.x, 0f);
child.offsetMax = new Vector2(child.offsetMax.x, 0f);

The other problem is with the inconsistent use of the StartingScreen property. The component default seems to be 1, hinting that it's NOT 0-based. Yet, in OnValidate() it is set to 0 if it's less than 0. In my case, where I don't have any children to begin with, Unity Editor forces the starting page to be 0. Then, in Start(), there's a line:

_currentPage = StartingScreen - 1;

, which will set _currentPage to -1 and cause a crash.

Yet another function I've had to patch in is related to "fake" portrait / landscape support. Our application is actually always in landscape left, but I have a script listening to Input.deviceOrientation changes and rotating the UI Panels accordingly (a bit like the Camera app on iOS; it's in landscape left but the UI rotates). This works pretty well as I have all the UI elements anchored inside a container UI Panel that gets rotated and resized. However, HSS doesn't work with this approach out of the box, as it calculates its size internally in pixels. I've fixed this by adding a function that I call every time the orientation changes:

    public void UpdateLayout()
    {
        _lerp = false;
        PageStep = (int)_scroll_rect.GetComponent<RectTransform>().rect.width;
        _scroll_rect.horizontalNormalizedPosition = 0;

        DistributePages();

        _scroll_rect.horizontalNormalizedPosition = (float)(_currentScreen) / (_screens - 1);
    }

I'm using PageStep 1 at startup so this seems to work, with other values it needs some work.

I hope some of these ideas are of use. In the meantime, I'm gonna stick with the older version for now.

SimonDarksideJ commented 2 years ago

On 2016.12.29 10:40, @SimonDarksideJ commented: Is this with the new version of the control in source @mikkoka ? Do you have a simple scene with your setup I can test with?

SimonDarksideJ commented 2 years ago

On 2016.12.29 10:43, Mikko Karvonen commented: I tried with the new version too. I don't have a test scene setup but I can try to make one today.

SimonDarksideJ commented 2 years ago

On 2016.12.29 14:47, Mikko Karvonen commented: @ddreaper here's a test project for you: https://dl.dropboxusercontent.com/u/14100324/HSS_Test.zip (Unity 5.4.3p4)

Using the latest HSS sources + some patches (search for "ADDED BY MIKKO" in both HorizontalScrollSnap.cs and ScrollSnapBase.cs). My orientation change fix doesn't seem to work too well with this version, just tested on my iPhone... (see UpdateLayout() function)

SimonDarksideJ commented 2 years ago

On 2016.12.29 18:04, @SimonDarksideJ commented: Thanks @mikkoka that sample helped a lot to understand your use case.

To solve the size issue, this was caused by setting the Anchor of your panels to Stretch, where in fact when placed in the HSS, the panel anchor position needs to be set to middle-left to control the layout. Doesn't affect anything within your container. Tested by soft setting (simply setting anchor, not using the ALT key) each UIPanel in their respective scene's. Then then they are moved / initialised, their anchor is correct.

I've updated the control to now soft-force the anchor point when the control is laying out to stop this behaviour.

Your fix to force the offsets creates issues if any other type of anchor is used because Unity is trying to compensate. Generally the offset is only used to override Unity's default behaviour instead of working with it.

Just looking at your resize suggestion now and I'll get it working. Although previously, just calling distributepages() should have been enough. but I will get an answer for you.

SimonDarksideJ commented 2 years ago

On 2016.12.29 18:05, @SimonDarksideJ commented: Oh and your comment @mikkoka on the Starting Page was spot on. I've changed the default to 0 and fixed all references that use it. It is a zero index point, especially now that the arrays have gone in the update.

SimonDarksideJ commented 2 years ago

On 2016.12.29 20:22, Mikko Karvonen commented: Thanks, the middle-left anchor preset seems to do the job. Btw, did you notice that the OnPageChanged event wasn't called anywhere? I added it as the first line in ChangeBulletsInfo(), seems to work there nicely. In our app we actually have two HSS instances, and they're controlling each other (and should always be in sync), so after receiving an OnPageChanged event from either one of them I can call GoToScreen() on the other component.

Looking forward to your fixes!

SimonDarksideJ commented 2 years ago

On 2016.12.29 22:04, @SimonDarksideJ commented: I've updated the control with the fixes and also resolved the UpdateLayout problems Let me know how you get on.

Will double check on the OnPageChanged Event. Should be in the "CurrentPage" property, updated when the page setting is updated. *edit, nope, lol. Seems I got half way through implementing that and got distracted. Will sort it out first thing tomorrow and then update the docs to the new style. Will take a bit of writing that one now :S

SimonDarksideJ commented 2 years ago

On 2016.12.30 15:46, @SimonDarksideJ commented: Patched OnPageChanged, also did some testing in worldspace with the HSS / VSS. Fixes uploaded.

Might do one more change, to update the OnChangeEnd event to also output the new page number.

SimonDarksideJ commented 2 years ago

On 2017.01.02 15:01, Mikko Karvonen commented: Hey,

still found a rather annoying problem with OnPageChanged... When I go to the next page, either by clicking a button or dragging, the event fires 3 times -- first with the correct value (e.g., 1 if going to the next page from the first page), after that with the original value (0) and finally again with the correct value. I think the problem is in HorizontalScrollSnap's Update(), where the CurrentPage setter is invoked.

SimonDarksideJ commented 2 years ago

On 2017.01.02 15:15, @SimonDarksideJ commented: Thanks for the update, I'll investigate.

SimonDarksideJ commented 2 years ago

On 2017.01.04 12:36, @SimonDarksideJ commented: Commit done to add a fix in to source. Tested with new scene on drag and button :D

SimonDarksideJ commented 2 years ago

On 2017.01.04 19:57, @SimonDarksideJ commented: Fix delivered in 1.2.0.2

SimonDarksideJ commented 2 years ago

On 2017.01.04 19:57 @SimonDarksideJ modified issue: status changed newresolved