Page MenuHomePhabricator

efl.ui.scroll_manager
Open, TODOPublic

Description

A private class that also needs at least some sort of stable internal semantics.

bu5hm4n created this task.EditedMay 8 2019, 12:00 PM
bu5hm4n triaged this task as TODO priority.

A problem of this class:

List of problems:

  • On each animator tick, we are emitting 3 events, which means. We are walking a big list of event subscriptions up to 3 times. This costs a lot of time when scrolling for example on a raspberry pi.
  • pan_position: According to the API of pan, we have a minimum and maximum position of the pan. However, When doing bouncing, we set ourself out of this.
  • The whole interaction with mouse wheels depends on evas_post_event_callback_push, which appears to not be available in out current aimed API form
  • x-axis vs. y-axis scrolling is completely copy and pasted...
  • The code for animation etc. is not really abstracted. animations are connected at random points. different animations do manipulate the scroller position at certain times.
  • Events are emitted on the parent that has been there at construction time. No further updates are considered.

A list of features that the scroller stuff needs to achive:

  • Normal mouse wheel Bouncing
  • Thumb scrolling (press, move, unpress)
  • Bouncing (bounce when you are below minimum or above maximum
  • Looping (when you are at the bottom, start again at the top, and visa verca)
zmike added a comment.May 8 2019, 12:43 PM

@woohyun @Jaehyun_Cho What's going on with this? I think scroll manager was supposed to be the scroller interface for 2.0, but it seems unfinished, has no real testing, and is still used by a number of "high priority" widgets.

After some initial review, it's my opinion that there is no way to stabilize any widgets which use on scroll manager at this time. It seems like "completing" this widget to the point where it could be considered reliable/functional enough to stabilize classes which use it would take a while too, so I am wondering if anyone there is actively working on this, or if there are internal patches waiting to be upstreamed?

woohyun added a comment.EditedJun 18 2019, 3:52 AM

@zmike

Sorry for missing this - I'll talk with my team member and will assign this one to proper person SOON.
@eagleeye Let's check this together ~

woohyun added a comment.EditedAug 14 2019, 4:15 AM

@eagleeye

Let's make a list of work items here (and add them in the description comment)
I think below things need to be considered together as work items.

[problems]

  • On each animator tick, we are emitting 3 events, which means. We are walking a big list of event subscriptions up to 3 times. This costs a lot of time when scrolling for example on a raspberry pi.
  • pan_position: According to the API of pan, we have a minimum and maximum position of the pan. However, When doing bouncing, we set ourself out of this.
  • The whole interaction with mouse wheels depends on evas_post_event_callback_push, which appears to not be available in out current aimed API form
  • x-axis vs. y-axis scrolling is completely copy and pasted...
  • The code for animation etc. is not really abstracted. animations are connected at random points. different animations do manipulate the scroller position at certain times.
  • Events are emitted on the parent that has been there at construction time. No further updates are considered.

[Features]

  • Normal mouse wheel Bouncing
  • Thumb scrolling (press, move, unpress)
  • Bouncing (bounce when you are below minimum or above maximum
  • Looping (when you are at the bottom, start again at the top, and visa verca)
eagleeye added a comment.EditedThu, Aug 29, 3:18 AM

@bu5hm4n

List of problems:

On each animator tick, we are emitting 3 events, which means. We are walking a big list of event subscriptions up to 3 times. This costs a lot of time when scrolling for example on a raspberry pi.

After working to interface, I will check for refactoring.

pan_position: According to the API of pan, we have a minimum and maximum position of the pan. However, When doing bouncing, we set ourself out of this.

It is necessary for bouncing behavior, but do you have some idea?

The whole interaction with mouse wheels depends on evas_post_event_callback_push, which appears to not be available in out current aimed API form

Post event is need to child scroller process wheel event. what is aimed API?

x-axis vs. y-axis scrolling is completely copy and pasted...

After working to interface, I will check for refactoring.

The code for animation etc. is not really abstracted. animations are connected at random points. different animations do manipulate the scroller position at certain times.

I wanna more information about this comment.

Events are emitted on the parent that has been there at construction time. No further updates are considered.

Which events do you mean?

A list of features that the scroller stuff needs to achive:

Normal mouse wheel Bouncing

I think it is working now.

Thumb scrolling (press, move, unpress)

I understood that we change evas event callback to efl interface(such as efl_input_clickable), it is right?

Bouncing (bounce when you are below minimum or above maximum

As mentioned above

Looping (when you are at the bottom, start again at the top, and visa verca)

After working to interface, I will implement it

zmike moved this task from Backlog to Evaluating on the efl: api board.Fri, Aug 30, 11:07 AM

@bu5hm4n

List of problems:

On each animator tick, we are emitting 3 events, which means. We are walking a big list of event subscriptions up to 3 times. This costs a lot of time when scrolling for example on a raspberry pi.

After working to interface, I will check for refactoring.

I've split this a little in a recent series, but I think we may be able to do more analysis here. This is more of an internals issue.

pan_position: According to the API of pan, we have a minimum and maximum position of the pan. However, When doing bouncing, we set ourself out of this.

It is necessary for bouncing behavior, but do you have some idea?

This is actually an issue in CollectionView, as @SanghyeonLee can confirm; the widget breaks completely when bounce is triggered.

The whole interaction with mouse wheels depends on evas_post_event_callback_push, which appears to not be available in out current aimed API form

Post event is need to child scroller process wheel event. what is aimed API?

This is an internals issue. The post event mechanic must be used in order to avoid intercepting events for widgets unless we devise a new strategy for mimicking that behavior.

x-axis vs. y-axis scrolling is completely copy and pasted...

After working to interface, I will check for refactoring.

This could maybe do like box/table layouts and use int* pointers for the axis, or an array index? Would be nice to consolidate this code.

The code for animation etc. is not really abstracted. animations are connected at random points. different animations do manipulate the scroller position at certain times.

I wanna more information about this comment.

I agree that this could use some work. Having a single entry point for animation start/stop would be ideal to improve readability. This does function fine at present, however, so I'm not sure it's a priority.

Events are emitted on the parent that has been there at construction time. No further updates are considered.

Which events do you mean?

If this is in reference to not handling things during construction, that issue should now be resolved.

A list of features that the scroller stuff needs to achive:

Normal mouse wheel Bouncing

I think it is working now.

This works in efl.ui.scroller, but it does not work as well in other widgets (like CollectionView) for whatever reason.

Thumb scrolling (press, move, unpress)

I understood that we change evas event callback to efl interface(such as efl_input_clickable), it is right?

Yes, we've generally moved towards that.

Bouncing (bounce when you are below minimum or above maximum

As mentioned above

Looping (when you are at the bottom, start again at the top, and visa verca)

After working to interface, I will implement it

In general I think this class mostly just needs refactoring to consolidate duplicated code and optimization to improve performance in some cases. It works fairly well.

@zmike

You are right. If this task is only for "definition of class" - then we can move this to stabilized step.
But features that @bu5hm4n said above are necessary for this task - then, I'll ask @eagleeye to make a plan for each action item.

@woohyun sure, though we will need to check with @stefan_schmidt for this if we want to add features as we are technically in the feature freeze.

@zmike

Ok.

I hope -

  1. refactoring would be handled later after this release
  2. make tasks for serious problems on this widget, and handle them immediately.
zmike added a comment.Thu, Sep 5, 7:59 PM

@woohyun I'm not aware of any "serious" problems with the class. There's only some missing features, which you've said @eagleeye should be handling if I understood correctly.

@zmike

Ok. Then, we don't need to consider 1.23 release for those features. Am I right ?
I'm asking this because I want @eagleeye do review works for other scroll relating "interfaces + classes" tasks first.

I am wondering if we need to stabelize this class at all, it is only implementing interfaces, and is used via composition, do we *need* it to be stable?

zmike added a comment.Fri, Sep 6, 6:02 AM

@woohyun the only reason we would consider it would be if Tizen urgently needs them. Otherwise there doesn't seem to be a rush.

From my perspective, the purpose of this ticket currently is to ensure that scroll manager provides all the functionality needed for the current release. I don't think it needs to be stable right now?

@zmike

I agree with you,
and @eagleeye also talked to me that there would be no urgent needs from Tizen yet.

Okay, let's get this out of the pending column then.

zmike moved this task from Evaluating to Backlog on the efl: api board.Fri, Sep 13, 11:31 AM