Re: [opensource-dev] Review Request: STORM-1713: Mouse pointer flickers when hovering over any active/clickable UI item

2011-12-08 Thread Lance Corrimal 


> On Dec. 8, 2011, 1:09 p.m., Lance Corrimal wrote:
> > I'm finding that in a viewer with the r2 version of this patch the mouse 
> > pointer flickers through at least three different states in rapid 
> > succeession when you hover it over your opened inventory and your inventory 
> > is still loading.
> > I'm pretty sure it didn't do that with the previous version.

on a viewer with the older patch the mouse pointer flickers too in this 
situation, but by far not as pronounced.


- Lance


---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/521/#review1116
---


On Dec. 5, 2011, 2:43 p.m., Ansariel Hiller wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/521/
> ---
> 
> (Updated Dec. 5, 2011, 2:43 p.m.)
> 
> 
> Review request for Viewer and Richard Nelson.
> 
> 
> Description
> ---
> 
> The fix for the flickering mouse cursor consists of 2 parts:
> 
> The first part changes LLView::childrenHandleHover() so that the cursor is 
> only set if the control itself is blocking the mouse event and not at every 
> hierarchy level in the control hierarchy. If the cursor would be set at each 
> level, it would cause a flicker in case the different controls set a 
> different cursor. This change actually resembles the algorithm used prior the 
> start of the flickering.
> 
> The second part changes LLToolTip::handleHover() and specifically handles 
> flickering of the cursor while hovering over the "i"-button of a hovertip. 
> The general call to LLPanel::handleHover() was changed to be only called if 
> the tooltip itself does not set the cursor itself. Logging the calls to 
> LLWindowWin32::setCursor() revealed that if the "i"-button on a hovertop is 
> hovered with the cursor said method is called twice with different cursors 
> alternatively. Checking the call stack further revealed that one call is 
> coming from LLToolTip::handleHover() and the other one from 
> LLButton::handleHover(). Latter gets invoked if LLPanel::handleHover() is 
> called. Since nothing is really done here except setting the cursor to 
> UI_CURSOR_ARROW only ti get then set to UI_CURSOR_HAND if 
> LLPanel::handleHover() returns, it doesn't make sense to do invoke that 
> method unless the cursor is not changed in the tooltip itself. So 
> LLPanel::handleHover() is only invoked if the tooltip does not set the cursor 
> itself, so that child controls should take care.
> 
> 
> This addresses bug STORM-1713.
> http://jira.secondlife.com/browse/STORM-1713
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt a984f7ffeb4b 
>   indra/llwindow/llwindow.h a984f7ffeb4b 
>   indra/llwindow/llwindow.cpp a984f7ffeb4b 
>   indra/llwindow/llwindowheadless.h a984f7ffeb4b 
>   indra/llwindow/llwindowmacosx.h a984f7ffeb4b 
>   indra/llwindow/llwindowmacosx.cpp a984f7ffeb4b 
>   indra/llwindow/llwindowmesaheadless.h a984f7ffeb4b 
>   indra/llwindow/llwindowsdl.h a984f7ffeb4b 
>   indra/llwindow/llwindowsdl.cpp a984f7ffeb4b 
>   indra/llwindow/llwindowwin32.h a984f7ffeb4b 
>   indra/llwindow/llwindowwin32.cpp a984f7ffeb4b 
> 
> Diff: http://codereview.secondlife.com/r/521/diff/diff
> 
> 
> Testing
> ---
> 
> Testing was done by myself on Firestorm rev. 24073 
> (http://hg.phoenixviewer.com/phoenix-firestorm-lgpl/rev/bcc95de39ca9) on 
> Windows 7 and Lance Corrimal on Dolphin Viewer. Apparently the fix works 
> without any side-effects
> 
> 
> Thanks,
> 
> Ansariel Hiller
> 
>

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: STORM-1713: Mouse pointer flickers when hovering over any active/clickable UI item

2011-12-08 Thread Lance Corrimal 

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/521/#review1116
---


I'm finding that in a viewer with the r2 version of this patch the mouse 
pointer flickers through at least three different states in rapid succeession 
when you hover it over your opened inventory and your inventory is still 
loading.
I'm pretty sure it didn't do that with the previous version.

- Lance Corrimal


On Dec. 5, 2011, 2:43 p.m., Ansariel Hiller wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/521/
> ---
> 
> (Updated Dec. 5, 2011, 2:43 p.m.)
> 
> 
> Review request for Viewer and Richard Nelson.
> 
> 
> Description
> ---
> 
> The fix for the flickering mouse cursor consists of 2 parts:
> 
> The first part changes LLView::childrenHandleHover() so that the cursor is 
> only set if the control itself is blocking the mouse event and not at every 
> hierarchy level in the control hierarchy. If the cursor would be set at each 
> level, it would cause a flicker in case the different controls set a 
> different cursor. This change actually resembles the algorithm used prior the 
> start of the flickering.
> 
> The second part changes LLToolTip::handleHover() and specifically handles 
> flickering of the cursor while hovering over the "i"-button of a hovertip. 
> The general call to LLPanel::handleHover() was changed to be only called if 
> the tooltip itself does not set the cursor itself. Logging the calls to 
> LLWindowWin32::setCursor() revealed that if the "i"-button on a hovertop is 
> hovered with the cursor said method is called twice with different cursors 
> alternatively. Checking the call stack further revealed that one call is 
> coming from LLToolTip::handleHover() and the other one from 
> LLButton::handleHover(). Latter gets invoked if LLPanel::handleHover() is 
> called. Since nothing is really done here except setting the cursor to 
> UI_CURSOR_ARROW only ti get then set to UI_CURSOR_HAND if 
> LLPanel::handleHover() returns, it doesn't make sense to do invoke that 
> method unless the cursor is not changed in the tooltip itself. So 
> LLPanel::handleHover() is only invoked if the tooltip does not set the cursor 
> itself, so that child controls should take care.
> 
> 
> This addresses bug STORM-1713.
> http://jira.secondlife.com/browse/STORM-1713
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt a984f7ffeb4b 
>   indra/llwindow/llwindow.h a984f7ffeb4b 
>   indra/llwindow/llwindow.cpp a984f7ffeb4b 
>   indra/llwindow/llwindowheadless.h a984f7ffeb4b 
>   indra/llwindow/llwindowmacosx.h a984f7ffeb4b 
>   indra/llwindow/llwindowmacosx.cpp a984f7ffeb4b 
>   indra/llwindow/llwindowmesaheadless.h a984f7ffeb4b 
>   indra/llwindow/llwindowsdl.h a984f7ffeb4b 
>   indra/llwindow/llwindowsdl.cpp a984f7ffeb4b 
>   indra/llwindow/llwindowwin32.h a984f7ffeb4b 
>   indra/llwindow/llwindowwin32.cpp a984f7ffeb4b 
> 
> Diff: http://codereview.secondlife.com/r/521/diff/diff
> 
> 
> Testing
> ---
> 
> Testing was done by myself on Firestorm rev. 24073 
> (http://hg.phoenixviewer.com/phoenix-firestorm-lgpl/rev/bcc95de39ca9) on 
> Windows 7 and Lance Corrimal on Dolphin Viewer. Apparently the fix works 
> without any side-effects
> 
> 
> Thanks,
> 
> Ansariel Hiller
> 
>

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] codereview site upgraded

2011-12-08 Thread Oz Linden (Scott Lawrence) 

  
  
On 2011-12-01 11:53, Oz Linden (Scott Lawrence) wrote:
I've
  upgraded codereview.secondlife.com to version 1.6.3 of
  ReviewBoard.  Let me know if this causes any problems...
  
  


Incidentally, this version adds a nice feature - issue tracking.  
When commenting on an issue, the reviewer has the option of checking
an 'Open an issue' checkbox:





The review display then provides a list of the open issues, and
quick buttons for the author to indicate that they've been dealt
with (either 'Fixed' or 'Drop' - using the later should be
accompanied by a comment about why it was not fixed).

By default, every comment creates an issue, so if as a reviewer you
are just adding commentary but don't think that any action is
required, then uncheck that box.

For the complete docs on this feature see
http://www.reviewboard.org/docs/manual/1.6/users/reviews/issue-tracking/#issue-tracking
  

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: Allow scripts to be saved/loaded to/from files.

2011-12-08 Thread Oz Linden 

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/516/#review1115
---


Just a few minor comments - looks very good.


indra/newview/llfloaternamedesc.cpp


for future reference, I'd prefer to see this as if/then/else with only one 
return, but this instance is so small that I don't feel too strongly about it.



indra/newview/llpreviewscript.cpp


Don't leave commented-out code in.



indra/newview/llpreviewscript.cpp


a comment here about why would be good ("user chose Cancel" ?)



indra/newview/llpreviewscript.cpp


user chose cancel?



indra/newview/skins/default/xui/en/strings.xml


fix indentation


- Oz Linden


On Dec. 8, 2011, 2:45 a.m., Ima Mechanique wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/516/
> ---
> 
> (Updated Dec. 8, 2011, 2:45 a.m.)
> 
> 
> Review request for Viewer.
> 
> 
> Description
> ---
> 
> Changes to allow opened script window to save/load to/from files on the users 
> computer.
> 
> 
> This addresses bug https://jira.secondlife.com/browse/storm-1708.
> 
> http://jira.secondlife.com/browse/https://jira.secondlife.com/browse/storm-1708
> 
> 
> Diffs
> -
> 
>   doc/contributions.txt a1319d553db9 
>   indra/llui/lltexteditor.h a1319d553db9 
>   indra/llui/lltexteditor.cpp a1319d553db9 
>   indra/newview/llfilepicker.h a1319d553db9 
>   indra/newview/llfilepicker.cpp a1319d553db9 
>   indra/newview/llfloaternamedesc.h a1319d553db9 
>   indra/newview/llfloaternamedesc.cpp a1319d553db9 
>   indra/newview/llpreviewscript.h a1319d553db9 
>   indra/newview/llpreviewscript.cpp a1319d553db9 
>   indra/newview/llviewerfloaterreg.cpp a1319d553db9 
>   indra/newview/skins/default/xui/en/panel_script_ed.xml a1319d553db9 
>   indra/newview/skins/default/xui/en/strings.xml a1319d553db9 
> 
> Diff: http://codereview.secondlife.com/r/516/diff/diff
> 
> 
> Testing
> ---
> 
> Successfully opened and saved scripts from/to local files.
> 
> 
> Thanks,
> 
> Ima Mechanique
> 
>

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges

Re: [opensource-dev] Review Request: Allow scripts to be saved/loaded to/from files.

2011-12-08 Thread Ima Mechanique 

---
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/516/
---

(Updated Dec. 8, 2011, 2:45 a.m.)


Review request for Viewer.


Changes
---

This is, hopefully, the final diff. It brings together the UI additions for 
Linux, Mac, and Windows.

As discussed in IRC and here I've removed the TAB replacement code, as this is 
wider problem which needs to be addressed separately.


Description
---

Changes to allow opened script window to save/load to/from files on the users 
computer.


This addresses bug https://jira.secondlife.com/browse/storm-1708.

http://jira.secondlife.com/browse/https://jira.secondlife.com/browse/storm-1708


Diffs (updated)
-

  doc/contributions.txt a1319d553db9 
  indra/llui/lltexteditor.h a1319d553db9 
  indra/llui/lltexteditor.cpp a1319d553db9 
  indra/newview/llfilepicker.h a1319d553db9 
  indra/newview/llfilepicker.cpp a1319d553db9 
  indra/newview/llfloaternamedesc.h a1319d553db9 
  indra/newview/llfloaternamedesc.cpp a1319d553db9 
  indra/newview/llpreviewscript.h a1319d553db9 
  indra/newview/llpreviewscript.cpp a1319d553db9 
  indra/newview/llviewerfloaterreg.cpp a1319d553db9 
  indra/newview/skins/default/xui/en/panel_script_ed.xml a1319d553db9 
  indra/newview/skins/default/xui/en/strings.xml a1319d553db9 

Diff: http://codereview.secondlife.com/r/516/diff/diff


Testing
---

Successfully opened and saved scripts from/to local files.


Thanks,

Ima Mechanique

___
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges