Re: Review Request: Rewrite kinetic scrolling on ScrollWidget

2010-03-18 Thread Marco Martin


> On 2010-03-17 12:31:22, Marco Martin wrote:
> > just by a quick test, the physics seems to work indeed better, it just 
> > still has some quirks:
> > 
> > - scrollbars aren't syncronized when dragging (yeah i know it needs a mode 
> > without scrollbars but i would keep them on the desktop)
> > 
> > - event stealing doesn't seem to work (even with setFiltersChildEvents): if 
> > registerasdraghandle is used clicks don't pass anymore to childrens, if 
> > it's not used is the parent to not receive events)
> > 
> > - the old behaviour of scrolwidget was to center the inner widget into the 
> > parent if it's smaller, now seems to always go to 0,0
> > 
> > 
> > about scrollStateChanged uhm.. permits external things to know the scrlling 
> > state, to for instance show/hide things depending if the user is 
> > dragging... not terribly useful indeed..
> 
> Zack Rusin wrote:
> The scrollbars issue is fixed.
> The event stealing works for sure, since that was the first thing I got 
> working. I don't really understand what draghandle's are supposed to be doing 
> or why there are event filters for them so I'm not sure what exactly is 
> expected to happen there. Is there an example that uses that code so that I 
> can figure out what's the expected behavior for those?
> I don't see the centering behavior with the old code (especially since 
> the setWidget has been calling an unconditional setPos(0,0)) , is there an 
> example that shows the behavior you mention?
> 
> Marco Martin wrote:
> in plasma some places that are using the scrollwidget and have lots of 
> child items (that are supposed to manage clicks too) is the microblog 
> plasmoid or the netbook search and launch interface (just plasmoidviewer sal) 
> where all the icons can be clicked or it should be possible to drag the view 
> around also by draging the icons, here the need of stealing  events when 
> needed.
> before it was working wit the registerasdraghandle function: it installed 
> an eventfilter and bounced the event back and forth between the child and the 
> scroll widget.
> this would hopefully become deprecated, since now it's using 
> setFiltersChildEvents.
> Icons in search and launch had this eventfilters in place, by killing the 
> implementation of registerasdraghandle we are sure there shouldn't 
> eventfilters around that do strange things, but it's still impossible to 
> scroll by dragging over the icons.
> 
> about the centering thing, always with search and launch, usually when it 
> starts the contents of the scrollwidget are just some icons, so  content it's 
> smaller than the scrollwidget and is centered.
> If the inner widget was placed centered by hand, it stayed there, it 
> simply couldn't scroll horizontally, so no problems. now as soon as is 
> touched by the mouse it animates and it goes to 0,0.
> I know is not a prety thing to move the inner widget by code, but 
> probably the scrollwidget will need a Qt::Alignment flag in the api to decide 
> how to align the inner widget if it's smaller than the scroll widget...
> 
> Zack Rusin wrote:
> Yes, the "lots of child items that accept clicks on top of a scrollview" 
> is exactly the case that works here. 
> Here "sal" seems to be working ok. BTW, setting the position of a widget 
> managed by another widget from somewhere else is pretty evil =) I did add 
> setAlignment code to the ScrollWidget, i think it makes a lot more sense than 
> allowing others to position the items that it's supposed to be managing :)
> 
> With the patch underneath sal has items centered correctly and I can 
> flick the view by click&dragging items without them executing. BTW, sal could 
> probably actually use the scrollStateChanged signal to not call 
> ensureItemVisible when another animation is in progress (when that happens 
> ScrollView has to immediately stop the currently running animation and invoke 
> another one, since otherwise we could have two different animations pulling 
> in opposite directions, which causes a small jump on hover events during the 
> flick animation)
> 
> Index: containments/sal/itemcontainer.cpp
> ===
> --- containments/sal/itemcontainer.cpp  (revision 1104506)
> +++ containments/sal/itemcontainer.cpp  (working copy)
> @@ -150,7 +150,6 @@
>  m_usedItems.remove(key, item);
>  } else {
>  item = new ResultWidget(this);
> -m_itemView->registerAsDragHandle(item);
>  item->hide();
>  item->setPos(boundingRect().center().x(), size().height());
>  }
> Index: containments/sal/itemview.cpp
> ===
> --- containments/sal/itemview.cpp   (revision 1104506)
> +++ containments/sal/itemview.cpp   (working copy)
> @@ -32,6 +32,7 @@
>  setFocusPolicy(Qt::Str

Re: Review Request: Rewrite kinetic scrolling on ScrollWidget

2010-03-18 Thread Marco Martin

---
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3312/#review4546
---

Ship it!


now it works prefect(tm)!
(well, it's still missing the support for the mouse wheel but will be quite 
easy to add it afterwards)
but it's reeeally about time to ship it :p

- Marco


On 2010-03-17 22:20:25, Zack Rusin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3312/
> ---
> 
> (Updated 2010-03-17 22:20:25)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> ---
> 
> As previously discussed with this approach we can actually properly intercept 
> events from child items. Furthermore now we properly "steal" events which 
> cause a flick (it's important for child items to properly act on 
> mouseReleaseEvents and not on mousePressEvents as some like to do, since it's 
> the release events that cause a flick) so items don't get clicks when 
> flicked. The physics and especially the overshoot behavior is a lot better in 
> this code as well.
> I tried to preserve the old behavior and emit the scrollStateChanged when 
> needed, but I'm not quite sure what that signal was meant to be good for.
> 
> 
> Diffs
> -
> 
>   trunk/KDE/kdelibs/plasma/widgets/scrollwidget.h 1102878 
>   trunk/KDE/kdelibs/plasma/widgets/scrollwidget.cpp 1102878 
> 
> Diff: http://reviewboard.kde.org/r/3312/diff
> 
> 
> Testing
> ---
> 
> Done in a custom app. Would be nice if someone double checked it with other 
> things that use ScrollWidget though (e.g. the notebook shell :) ).
> 
> 
> Thanks,
> 
> Zack
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: Rewrite kinetic scrolling on ScrollWidget

2010-03-17 Thread Zack Rusin


> On 2010-03-17 12:31:22, Marco Martin wrote:
> > just by a quick test, the physics seems to work indeed better, it just 
> > still has some quirks:
> > 
> > - scrollbars aren't syncronized when dragging (yeah i know it needs a mode 
> > without scrollbars but i would keep them on the desktop)
> > 
> > - event stealing doesn't seem to work (even with setFiltersChildEvents): if 
> > registerasdraghandle is used clicks don't pass anymore to childrens, if 
> > it's not used is the parent to not receive events)
> > 
> > - the old behaviour of scrolwidget was to center the inner widget into the 
> > parent if it's smaller, now seems to always go to 0,0
> > 
> > 
> > about scrollStateChanged uhm.. permits external things to know the scrlling 
> > state, to for instance show/hide things depending if the user is 
> > dragging... not terribly useful indeed..
> 
> Zack Rusin wrote:
> The scrollbars issue is fixed.
> The event stealing works for sure, since that was the first thing I got 
> working. I don't really understand what draghandle's are supposed to be doing 
> or why there are event filters for them so I'm not sure what exactly is 
> expected to happen there. Is there an example that uses that code so that I 
> can figure out what's the expected behavior for those?
> I don't see the centering behavior with the old code (especially since 
> the setWidget has been calling an unconditional setPos(0,0)) , is there an 
> example that shows the behavior you mention?
> 
> Marco Martin wrote:
> in plasma some places that are using the scrollwidget and have lots of 
> child items (that are supposed to manage clicks too) is the microblog 
> plasmoid or the netbook search and launch interface (just plasmoidviewer sal) 
> where all the icons can be clicked or it should be possible to drag the view 
> around also by draging the icons, here the need of stealing  events when 
> needed.
> before it was working wit the registerasdraghandle function: it installed 
> an eventfilter and bounced the event back and forth between the child and the 
> scroll widget.
> this would hopefully become deprecated, since now it's using 
> setFiltersChildEvents.
> Icons in search and launch had this eventfilters in place, by killing the 
> implementation of registerasdraghandle we are sure there shouldn't 
> eventfilters around that do strange things, but it's still impossible to 
> scroll by dragging over the icons.
> 
> about the centering thing, always with search and launch, usually when it 
> starts the contents of the scrollwidget are just some icons, so  content it's 
> smaller than the scrollwidget and is centered.
> If the inner widget was placed centered by hand, it stayed there, it 
> simply couldn't scroll horizontally, so no problems. now as soon as is 
> touched by the mouse it animates and it goes to 0,0.
> I know is not a prety thing to move the inner widget by code, but 
> probably the scrollwidget will need a Qt::Alignment flag in the api to decide 
> how to align the inner widget if it's smaller than the scroll widget...

Yes, the "lots of child items that accept clicks on top of a scrollview" is 
exactly the case that works here. 
Here "sal" seems to be working ok. BTW, setting the position of a widget 
managed by another widget from somewhere else is pretty evil =) I did add 
setAlignment code to the ScrollWidget, i think it makes a lot more sense than 
allowing others to position the items that it's supposed to be managing :)

With the patch underneath sal has items centered correctly and I can flick the 
view by click&dragging items without them executing. BTW, sal could probably 
actually use the scrollStateChanged signal to not call ensureItemVisible when 
another animation is in progress (when that happens ScrollView has to 
immediately stop the currently running animation and invoke another one, since 
otherwise we could have two different animations pulling in opposite 
directions, which causes a small jump on hover events during the flick 
animation)

Index: containments/sal/itemcontainer.cpp
===
--- containments/sal/itemcontainer.cpp  (revision 1104506)
+++ containments/sal/itemcontainer.cpp  (working copy)
@@ -150,7 +150,6 @@
 m_usedItems.remove(key, item);
 } else {
 item = new ResultWidget(this);
-m_itemView->registerAsDragHandle(item);
 item->hide();
 item->setPos(boundingRect().center().x(), size().height());
 }
Index: containments/sal/itemview.cpp
===
--- containments/sal/itemview.cpp   (revision 1104506)
+++ containments/sal/itemview.cpp   (working copy)
@@ -32,6 +32,7 @@
 setFocusPolicy(Qt::StrongFocus);
 setSizePolicy(QSizePolicy::Expanding, QSizePolicy::Expanding);
 m_itemContainer = new ItemContainer(this);
+setAlignment(Qt::AlignCenter);
 setWidget(m_it

Re: Review Request: Rewrite kinetic scrolling on ScrollWidget

2010-03-17 Thread Zack Rusin

---
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3312/
---

(Updated 2010-03-17 22:20:25.287357)


Review request for Plasma.


Changes
---

Adds the ability to set the alignment on the inner widget.


Summary
---

As previously discussed with this approach we can actually properly intercept 
events from child items. Furthermore now we properly "steal" events which cause 
a flick (it's important for child items to properly act on mouseReleaseEvents 
and not on mousePressEvents as some like to do, since it's the release events 
that cause a flick) so items don't get clicks when flicked. The physics and 
especially the overshoot behavior is a lot better in this code as well.
I tried to preserve the old behavior and emit the scrollStateChanged when 
needed, but I'm not quite sure what that signal was meant to be good for.


Diffs (updated)
-

  trunk/KDE/kdelibs/plasma/widgets/scrollwidget.h 1102878 
  trunk/KDE/kdelibs/plasma/widgets/scrollwidget.cpp 1102878 

Diff: http://reviewboard.kde.org/r/3312/diff


Testing
---

Done in a custom app. Would be nice if someone double checked it with other 
things that use ScrollWidget though (e.g. the notebook shell :) ).


Thanks,

Zack

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: Rewrite kinetic scrolling on ScrollWidget

2010-03-17 Thread Marco Martin


> On 2010-03-17 12:31:22, Marco Martin wrote:
> > just by a quick test, the physics seems to work indeed better, it just 
> > still has some quirks:
> > 
> > - scrollbars aren't syncronized when dragging (yeah i know it needs a mode 
> > without scrollbars but i would keep them on the desktop)
> > 
> > - event stealing doesn't seem to work (even with setFiltersChildEvents): if 
> > registerasdraghandle is used clicks don't pass anymore to childrens, if 
> > it's not used is the parent to not receive events)
> > 
> > - the old behaviour of scrolwidget was to center the inner widget into the 
> > parent if it's smaller, now seems to always go to 0,0
> > 
> > 
> > about scrollStateChanged uhm.. permits external things to know the scrlling 
> > state, to for instance show/hide things depending if the user is 
> > dragging... not terribly useful indeed..
> 
> Zack Rusin wrote:
> The scrollbars issue is fixed.
> The event stealing works for sure, since that was the first thing I got 
> working. I don't really understand what draghandle's are supposed to be doing 
> or why there are event filters for them so I'm not sure what exactly is 
> expected to happen there. Is there an example that uses that code so that I 
> can figure out what's the expected behavior for those?
> I don't see the centering behavior with the old code (especially since 
> the setWidget has been calling an unconditional setPos(0,0)) , is there an 
> example that shows the behavior you mention?

in plasma some places that are using the scrollwidget and have lots of child 
items (that are supposed to manage clicks too) is the microblog plasmoid or the 
netbook search and launch interface (just plasmoidviewer sal) where all the 
icons can be clicked or it should be possible to drag the view around also by 
draging the icons, here the need of stealing  events when needed.
before it was working wit the registerasdraghandle function: it installed an 
eventfilter and bounced the event back and forth between the child and the 
scroll widget.
this would hopefully become deprecated, since now it's using 
setFiltersChildEvents.
Icons in search and launch had this eventfilters in place, by killing the 
implementation of registerasdraghandle we are sure there shouldn't eventfilters 
around that do strange things, but it's still impossible to scroll by dragging 
over the icons.

about the centering thing, always with search and launch, usually when it 
starts the contents of the scrollwidget are just some icons, so  content it's 
smaller than the scrollwidget and is centered.
If the inner widget was placed centered by hand, it stayed there, it simply 
couldn't scroll horizontally, so no problems. now as soon as is touched by the 
mouse it animates and it goes to 0,0.
I know is not a prety thing to move the inner widget by code, but probably the 
scrollwidget will need a Qt::Alignment flag in the api to decide how to align 
the inner widget if it's smaller than the scroll widget...


> On 2010-03-17 12:31:22, Marco Martin wrote:
> > trunk/KDE/kdelibs/plasma/widgets/scrollwidget.cpp, line 63
> > 
> >
> > is it safe to assume is it's an huge move it was done 
> > programmatically?, so 
> > a) use a fixed speed for the first
> > b) use immediate move for moves repeated very fast
> 
> Zack Rusin wrote:
> I'm having a bit of trouble parsing this paragraph. 
> IGNORE_SUSPICIOUS_MOVES isn't for moves of widgets it's for suspicious 
> mouse move events that scene is sending us. Suspicious in the sense that 
> they're coming in too close to each other. They are generated by a user 
> moving the mouse.
> Scrolling a widget from api is done through ensureItemVisible, which does 
> it with a constant speed and imho if someone would try to do it by sending 
> synthetic mousePress/mouseMove/mouseRelease event that'd be a bug in their 
> code.

ehm, yeah, of course you're right :p


- Marco


---
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3312/#review4532
---


On 2010-03-17 14:31:13, Zack Rusin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3312/
> ---
> 
> (Updated 2010-03-17 14:31:13)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> ---
> 
> As previously discussed with this approach we can actually properly intercept 
> events from child items. Furthermore now we properly "steal" events which 
> cause a flick (it's important for child items to properly act on 
> mouseReleaseEvents and not on mousePressEvents as some like to do, since it's 
> the release events that cause a flick) so items don't get clicks when 
> flicked. The 

Re: Review Request: Rewrite kinetic scrolling on ScrollWidget

2010-03-17 Thread Zack Rusin


> On 2010-03-17 12:31:22, Marco Martin wrote:
> > just by a quick test, the physics seems to work indeed better, it just 
> > still has some quirks:
> > 
> > - scrollbars aren't syncronized when dragging (yeah i know it needs a mode 
> > without scrollbars but i would keep them on the desktop)
> > 
> > - event stealing doesn't seem to work (even with setFiltersChildEvents): if 
> > registerasdraghandle is used clicks don't pass anymore to childrens, if 
> > it's not used is the parent to not receive events)
> > 
> > - the old behaviour of scrolwidget was to center the inner widget into the 
> > parent if it's smaller, now seems to always go to 0,0
> > 
> > 
> > about scrollStateChanged uhm.. permits external things to know the scrlling 
> > state, to for instance show/hide things depending if the user is 
> > dragging... not terribly useful indeed..

The scrollbars issue is fixed.
The event stealing works for sure, since that was the first thing I got 
working. I don't really understand what draghandle's are supposed to be doing 
or why there are event filters for them so I'm not sure what exactly is 
expected to happen there. Is there an example that uses that code so that I can 
figure out what's the expected behavior for those?
I don't see the centering behavior with the old code (especially since the 
setWidget has been calling an unconditional setPos(0,0)) , is there an example 
that shows the behavior you mention?


> On 2010-03-17 12:31:22, Marco Martin wrote:
> > trunk/KDE/kdelibs/plasma/widgets/scrollwidget.cpp, line 63
> > 
> >
> > is it safe to assume is it's an huge move it was done 
> > programmatically?, so 
> > a) use a fixed speed for the first
> > b) use immediate move for moves repeated very fast

I'm having a bit of trouble parsing this paragraph. 
IGNORE_SUSPICIOUS_MOVES isn't for moves of widgets it's for suspicious mouse 
move events that scene is sending us. Suspicious in the sense that they're 
coming in too close to each other. They are generated by a user moving the 
mouse.
Scrolling a widget from api is done through ensureItemVisible, which does it 
with a constant speed and imho if someone would try to do it by sending 
synthetic mousePress/mouseMove/mouseRelease event that'd be a bug in their code.


- Zack


---
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3312/#review4532
---


On 2010-03-17 14:31:13, Zack Rusin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3312/
> ---
> 
> (Updated 2010-03-17 14:31:13)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> ---
> 
> As previously discussed with this approach we can actually properly intercept 
> events from child items. Furthermore now we properly "steal" events which 
> cause a flick (it's important for child items to properly act on 
> mouseReleaseEvents and not on mousePressEvents as some like to do, since it's 
> the release events that cause a flick) so items don't get clicks when 
> flicked. The physics and especially the overshoot behavior is a lot better in 
> this code as well.
> I tried to preserve the old behavior and emit the scrollStateChanged when 
> needed, but I'm not quite sure what that signal was meant to be good for.
> 
> 
> Diffs
> -
> 
>   trunk/KDE/kdelibs/plasma/widgets/scrollwidget.h 1102878 
>   trunk/KDE/kdelibs/plasma/widgets/scrollwidget.cpp 1102878 
> 
> Diff: http://reviewboard.kde.org/r/3312/diff
> 
> 
> Testing
> ---
> 
> Done in a custom app. Would be nice if someone double checked it with other 
> things that use ScrollWidget though (e.g. the notebook shell :) ).
> 
> 
> Thanks,
> 
> Zack
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: Rewrite kinetic scrolling on ScrollWidget

2010-03-17 Thread Zack Rusin

---
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3312/
---

(Updated 2010-03-17 14:31:13.742901)


Review request for Plasma.


Changes
---

Fixes scrollbars and stops stealing events from the scrollbars and drag handles.


Summary
---

As previously discussed with this approach we can actually properly intercept 
events from child items. Furthermore now we properly "steal" events which cause 
a flick (it's important for child items to properly act on mouseReleaseEvents 
and not on mousePressEvents as some like to do, since it's the release events 
that cause a flick) so items don't get clicks when flicked. The physics and 
especially the overshoot behavior is a lot better in this code as well.
I tried to preserve the old behavior and emit the scrollStateChanged when 
needed, but I'm not quite sure what that signal was meant to be good for.


Diffs (updated)
-

  trunk/KDE/kdelibs/plasma/widgets/scrollwidget.h 1102878 
  trunk/KDE/kdelibs/plasma/widgets/scrollwidget.cpp 1102878 

Diff: http://reviewboard.kde.org/r/3312/diff


Testing
---

Done in a custom app. Would be nice if someone double checked it with other 
things that use ScrollWidget though (e.g. the notebook shell :) ).


Thanks,

Zack

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: Rewrite kinetic scrolling on ScrollWidget

2010-03-17 Thread Artur Souza (MoRpHeUz)
On Wednesday 17 March 2010, 09:20 igorto wrote:
> Instead of change the ScrollWidget code you should have a look in
> plasma/private/kineticscroll.* because there is the old kinetic scroll
> implementation and it is used by others classes(like plasma/webview) and
> would be difficult maintain two different kinetic scrolling
> implementations.

Some threads below you'll find the discussion that the ScrollWidget is the 
right place to have this code. Take a look at the other thread where Zack and 
Marco were discussing this subject :)

Cheers,

--
Artur Duque de Souza
openBossa
INdT - Instituto Nokia de Tecnologia
--
Blog: http://blog.morpheuz.cc
PGP: 0xDBEEAAC3 @ wwwkeys.pgp.net
--


signature.asc
Description: This is a digitally signed message part.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: Rewrite kinetic scrolling on ScrollWidget

2010-03-17 Thread Marco Martin


> On 2010-03-17 12:20:55, igorto wrote:
> > Instead of change the ScrollWidget code you should have a look in 
> > plasma/private/kineticscroll.* because there is the old kinetic scroll 
> > implementation and it is used by others classes(like plasma/webview) and 
> > would be difficult maintain two different kinetic scrolling implementations.

as discussed in the previous patch on this issue, with kineticscroll you can't 
use setFiltersChildEvents (tat sill doesn't seem to actually work anyways but 
that's another issue :p)


- Marco


---
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3312/#review4534
---


On 2010-03-17 04:59:11, Zack Rusin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3312/
> ---
> 
> (Updated 2010-03-17 04:59:11)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> ---
> 
> As previously discussed with this approach we can actually properly intercept 
> events from child items. Furthermore now we properly "steal" events which 
> cause a flick (it's important for child items to properly act on 
> mouseReleaseEvents and not on mousePressEvents as some like to do, since it's 
> the release events that cause a flick) so items don't get clicks when 
> flicked. The physics and especially the overshoot behavior is a lot better in 
> this code as well.
> I tried to preserve the old behavior and emit the scrollStateChanged when 
> needed, but I'm not quite sure what that signal was meant to be good for.
> 
> 
> Diffs
> -
> 
>   trunk/KDE/kdelibs/plasma/widgets/scrollwidget.h 1102878 
>   trunk/KDE/kdelibs/plasma/widgets/scrollwidget.cpp 1102878 
> 
> Diff: http://reviewboard.kde.org/r/3312/diff
> 
> 
> Testing
> ---
> 
> Done in a custom app. Would be nice if someone double checked it with other 
> things that use ScrollWidget though (e.g. the notebook shell :) ).
> 
> 
> Thanks,
> 
> Zack
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: Rewrite kinetic scrolling on ScrollWidget

2010-03-17 Thread Marco Martin

---
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3312/#review4532
---


just by a quick test, the physics seems to work indeed better, it just still 
has some quirks:

- scrollbars aren't syncronized when dragging (yeah i know it needs a mode 
without scrollbars but i would keep them on the desktop)

- event stealing doesn't seem to work (even with setFiltersChildEvents): if 
registerasdraghandle is used clicks don't pass anymore to childrens, if it's 
not used is the parent to not receive events)

- the old behaviour of scrolwidget was to center the inner widget into the 
parent if it's smaller, now seems to always go to 0,0


about scrollStateChanged uhm.. permits external things to know the scrlling 
state, to for instance show/hide things depending if the user is dragging... 
not terribly useful indeed..


trunk/KDE/kdelibs/plasma/widgets/scrollwidget.cpp


is it safe to assume is it's an huge move it was done programmatically?, so 
a) use a fixed speed for the first
b) use immediate move for moves repeated very fast 


- Marco


On 2010-03-17 04:59:11, Zack Rusin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3312/
> ---
> 
> (Updated 2010-03-17 04:59:11)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> ---
> 
> As previously discussed with this approach we can actually properly intercept 
> events from child items. Furthermore now we properly "steal" events which 
> cause a flick (it's important for child items to properly act on 
> mouseReleaseEvents and not on mousePressEvents as some like to do, since it's 
> the release events that cause a flick) so items don't get clicks when 
> flicked. The physics and especially the overshoot behavior is a lot better in 
> this code as well.
> I tried to preserve the old behavior and emit the scrollStateChanged when 
> needed, but I'm not quite sure what that signal was meant to be good for.
> 
> 
> Diffs
> -
> 
>   trunk/KDE/kdelibs/plasma/widgets/scrollwidget.h 1102878 
>   trunk/KDE/kdelibs/plasma/widgets/scrollwidget.cpp 1102878 
> 
> Diff: http://reviewboard.kde.org/r/3312/diff
> 
> 
> Testing
> ---
> 
> Done in a custom app. Would be nice if someone double checked it with other 
> things that use ScrollWidget though (e.g. the notebook shell :) ).
> 
> 
> Thanks,
> 
> Zack
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: Rewrite kinetic scrolling on ScrollWidget

2010-03-17 Thread igorto

---
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3312/#review4534
---


Instead of change the ScrollWidget code you should have a look in 
plasma/private/kineticscroll.* because there is the old kinetic scroll 
implementation and it is used by others classes(like plasma/webview) and would 
be difficult maintain two different kinetic scrolling implementations.

- igorto


On 2010-03-17 04:59:11, Zack Rusin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3312/
> ---
> 
> (Updated 2010-03-17 04:59:11)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> ---
> 
> As previously discussed with this approach we can actually properly intercept 
> events from child items. Furthermore now we properly "steal" events which 
> cause a flick (it's important for child items to properly act on 
> mouseReleaseEvents and not on mousePressEvents as some like to do, since it's 
> the release events that cause a flick) so items don't get clicks when 
> flicked. The physics and especially the overshoot behavior is a lot better in 
> this code as well.
> I tried to preserve the old behavior and emit the scrollStateChanged when 
> needed, but I'm not quite sure what that signal was meant to be good for.
> 
> 
> Diffs
> -
> 
>   trunk/KDE/kdelibs/plasma/widgets/scrollwidget.h 1102878 
>   trunk/KDE/kdelibs/plasma/widgets/scrollwidget.cpp 1102878 
> 
> Diff: http://reviewboard.kde.org/r/3312/diff
> 
> 
> Testing
> ---
> 
> Done in a custom app. Would be nice if someone double checked it with other 
> things that use ScrollWidget though (e.g. the notebook shell :) ).
> 
> 
> Thanks,
> 
> Zack
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: Rewrite kinetic scrolling on ScrollWidget

2010-03-16 Thread Zack Rusin

---
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3312/
---

(Updated 2010-03-17 04:59:11.654236)


Review request for Plasma.


Changes
---

Fixes the scrollStateChanged.


Summary
---

As previously discussed with this approach we can actually properly intercept 
events from child items. Furthermore now we properly "steal" events which cause 
a flick (it's important for child items to properly act on mouseReleaseEvents 
and not on mousePressEvents as some like to do, since it's the release events 
that cause a flick) so items don't get clicks when flicked. The physics and 
especially the overshoot behavior is a lot better in this code as well.
I tried to preserve the old behavior and emit the scrollStateChanged when 
needed, but I'm not quite sure what that signal was meant to be good for.


Diffs (updated)
-

  trunk/KDE/kdelibs/plasma/widgets/scrollwidget.h 1102878 
  trunk/KDE/kdelibs/plasma/widgets/scrollwidget.cpp 1102878 

Diff: http://reviewboard.kde.org/r/3312/diff


Testing
---

Done in a custom app. Would be nice if someone double checked it with other 
things that use ScrollWidget though (e.g. the notebook shell :) ).


Thanks,

Zack

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Review Request: Rewrite kinetic scrolling on ScrollWidget

2010-03-16 Thread Zack Rusin

---
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3312/
---

Review request for Plasma.


Summary
---

As previously discussed with this approach we can actually properly intercept 
events from child items. Furthermore now we properly "steal" events which cause 
a flick (it's important for child items to properly act on mouseReleaseEvents 
and not on mousePressEvents as some like to do, since it's the release events 
that cause a flick) so items don't get clicks when flicked. The physics and 
especially the overshoot behavior is a lot better in this code as well.
I tried to preserve the old behavior and emit the scrollStateChanged when 
needed, but I'm not quite sure what that signal was meant to be good for.


Diffs
-

  trunk/KDE/kdelibs/plasma/widgets/scrollwidget.h 1102878 
  trunk/KDE/kdelibs/plasma/widgets/scrollwidget.cpp 1102878 

Diff: http://reviewboard.kde.org/r/3312/diff


Testing
---

Done in a custom app. Would be nice if someone double checked it with other 
things that use ScrollWidget though (e.g. the notebook shell :) ).


Thanks,

Zack

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel