Review Request: Rewrite kinetic scrolling on ScrollWidget

Zack Rusin zack at kde.org
Wed Mar 17 20:54:45 CET 2010



> 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
> > <http://reviewboard.kde.org/r/3312/diff/2/?file=20889#file20889line63>
> >
> >     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
> 
>



More information about the Plasma-devel mailing list