[Haiku-commits] r27258 - haiku/trunk/src/apps/deskbar

[Alexandre Deckner]

Michael Pfeiffer wrote:
> Am 31.08.2008 um 21:27 schrieb Alexandre Deckner:
>   
>>>> * Rewrote long click mouse tracking. This avoids busy looping in
>>>> TimeView::MouseDown and fixes
>>>> #591. The helper class could be reused for similar situations (ex:
>>>> Tracker context menu and
>>>> drag detection). See the brief doc in the code. Comments welcomed!
>>>>
>>>>         
>>> Hmm. This problem could be solved more elegantly by using a
>>> BMessageRunner. This way the polling thread could be avoided
>>> completely. IIRC src/apps/pairs/PairsWindow.cpp uses BMessageRunner
>>> for a similar problem.
>>>       
>> Thanks for having a look, i talked about that with René and he told me
>> the same, but i didn't find my solution that ugly :) and still more
>> elegant than the previous impl!
>>     
>
> I did not want to say it in my first my mail, but IMHO it is ugly ;)
> 1) You need to poll, although that is not necessary.
>   
Ah? How can you know the duration of a mouse down otherwise?
> 2) You access the window and view from another thread, than the window  
> thread,
>     which should be avoided to prevent dead locks and avoid problems
>     with thread safety (Can you guarantee that the view is not
>     going to be deleted while the polling thread sill accesses it?).
>   
Yep, the object is always deleted before the view as it is a member.
>     Your implementation has the potential for a dead lock too,
>     for example if the destructor is called while the window lock is  
> hold,
>     it will dead lock if the polling thread tries to lock the window
>     after snooze, since the destructor will wait for the termination
>     of the polling thread, the lock will never be released.
>   
Hmm, possible, i didn't think about this scenario yet.
> 3) You start a thread for each instance of LongClickTracker, that's
>     a waste of resources, just imagine it was used for every replicant
>     in the Deskbar or it would be used for each button in a tool bar.
>   
For now there's only one instance, i didn't have time to generalize to 
every scenario you imagined. I've seen lots of small threading solutions 
like mine in the codebase, so i thought it would be such a big deal.

>  From your other mail.
> [Modularity]
> Agreed.
> However see my post to openbeos ML "How to open Calendar from  
> Deskbar?" Which I had already sent yesterday, from a not subscribed  
> mail address so it did not hit the ML. I have now resent it.
>   
Well i had a fix ready for 9 days already (#591), i had to finish and 
commit it, i don't like letting fixes die. But there's no problem if you 
change the behaviour so that it's not needed anymore, i have other uses 
for it :)
> [Performance]
> I disagree, the user is the slowest part when it comes to UI  
> interaction.
>   
I was talking about the cpu usage of the threaded impl. vs 
BMessageRunner impl.
> If you don't care, I can create an alternative implementation.
>
>   
Yep, sincerely, go for it, i'm already on something else.

Best regards,
Alex