date:20110727


 [ 
https://issues.apache.org/jira/browse/PIVOT-778?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Sandro Martini updated PIVOT-778:
-

Fix Version/s: 2.0.1

Comments ?

 Optimise DisplayHost.paintBuffered and DisplayHost.paintVolatileBuffered
 

 Key: PIVOT-778
 URL: https://issues.apache.org/jira/browse/PIVOT-778
 Project: Pivot
  Issue Type: Improvement
  Components: wtk
Affects Versions: 2.0
Reporter: Piotr Kołaczkowski
Priority: Minor
  Labels: DisplayHost, caching, gc, paint, performance, repaint
 Fix For: 2.0.1


 We are writing sort of a game, which continually calls Component.repaint 
 method, at 60 FPS. We noticed excessive CPU usage, although the actual amount 
 of painting done by our component (actually in an overriden Panel.paint) is 
 ridiculously small. The profiler pointed us to the paintVolatileBuffered 
 method in the DisplayHost. What you are doing there is:
 1. obtain a new, fresh BufferedImage of size equal to the actual clip region, 
 let's say for a full screen game it can be about 1280x1024. This is 1.3 Mpix 
 x 4 bytes/pixel = 5.2 MB of raw data, allocated from a probably cold memory 
 region (not in the L2 cache)
 2. then you call actual paint on that buffered image (this is touching at 
 least 5.2 MB again)
 3. then you copy that to the onscreen buffer (which means copying 5.2 MB for 
 another time)
 4. in case GC kicks in after 1 and 3. it has to move the BufferedImage in 
 memory to compact young generation (= touching 5.2 MB fourth time)
 The whole process means allocating from cold memory 5.2 MB per each frame and 
 touching about 20 MB per frame.
 For 60 FPS it makes up ~300 MB/s allocation rate and 1.2GB memory throughput. 
 It also makes the GC go crazy.
 We have found that caching the buffer between the subsequent paint calls 
 improves performance a lot:
 code
 /** Stores the prepared offscreen buffer */
 private BufferedImage bufferedImage;
 /**
  * Attempts to paint the display using an offscreen buffer.
  *
  * @param graphics
  * The source graphics context.
  *
  * @return
  * tttrue/tt if the display was painted using the offscreen
  * buffer; ttfalse/tt, otherwise.
  */
 private boolean paintBuffered(Graphics2D graphics) {
 boolean painted = false;
 // Paint the display into an offscreen buffer
 GraphicsConfiguration gc = graphics.getDeviceConfiguration();
 java.awt.Rectangle clipBounds = graphics.getClipBounds();
 if (bufferedImage == null ||
 bufferedImage.getWidth()  clipBounds.width ||
 bufferedImage.getHeight()  clipBounds.height)
 bufferedImage = gc.createCompatibleImage(clipBounds.width, 
 clipBounds.height,
 Transparency.OPAQUE);
 if (bufferedImage != null) {
 Graphics2D bufferedImageGraphics = 
 (Graphics2D)bufferedImage.getGraphics();
 bufferedImageGraphics.setClip(0, 0, clipBounds.width,
 ...
 /code 
 Advantages:
 1. it saves from costly allocation of a large object from possibly not-cached 
 memory region
 2. after a few repaints the GC moves this object to the tenured generation, 
 so that the young generation collector is much more efficient (longer times 
 between runs)
 3. the image probably stays most of the time in the L2 or L3 cache, which 
 saves on memory bandwidth and speeds up painting
 Disadvantages:
 1. uses some memory that is probably not required all the time, when the app 
 doesn't need to repaint anything large, however this is almost completely 
 shadowed by the excessive GC overhead due to continuous recreation of the 
 offscreen buffered image
 Anyway, we observed about 2-4x performance increase by this simple change - 
 now when running at 60 FPS it uses only about 25% of CPU for painting, and 
 the rest can be used by the application logic (AI, etc.). Previously 60 FPS 
 was probably the most we could achieve from Core2Duo 2.2 GHz. Of course, this 
 change won't affect any business applications that don't do animations etc.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (PIVOT-778) Optimise DisplayHost.paintBuffered and DisplayHost.paintVolatileBuffered

[
https://issues.apache.org/jira/browse/PIVOT-778?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13071651#comment-13071651
]

Chris Bartlett commented on PIVOT-778:
--

Piotr - This looks interesting, but it would really help if you could supply an
example that can be used as a simple benchmark. Something that paints with the
current method and then with the optimised method.

Optimise DisplayHost.paintBuffered and DisplayHost.paintVolatileBuffered

Key: PIVOT-778
URL: https://issues.apache.org/jira/browse/PIVOT-778
Project: Pivot
Issue Type: Improvement
Components: wtk
Affects Versions: 2.0
Reporter: Piotr Kołaczkowski
Priority: Minor
Labels: DisplayHost, caching, gc, paint, performance, repaint
Fix For: 2.0.1

We are writing sort of a game, which continually calls Component.repaint
method, at 60 FPS. We noticed excessive CPU usage, although the actual amount
of painting done by our component (actually in an overriden Panel.paint) is
ridiculously small. The profiler pointed us to the paintVolatileBuffered
method in the DisplayHost. What you are doing there is:
1. obtain a new, fresh BufferedImage of size equal to the actual clip region,
let's say for a full screen game it can be about 1280x1024. This is 1.3 Mpix
x 4 bytes/pixel = 5.2 MB of raw data, allocated from a probably cold memory
region (not in the L2 cache)
2. then you call actual paint on that buffered image (this is touching at
least 5.2 MB again)
3. then you copy that to the onscreen buffer (which means copying 5.2 MB for
another time)
4. in case GC kicks in after 1 and 3. it has to move the BufferedImage in
memory to compact young generation (= touching 5.2 MB fourth time)
The whole process means allocating from cold memory 5.2 MB per each frame and
touching about 20 MB per frame.
For 60 FPS it makes up ~300 MB/s allocation rate and 1.2GB memory throughput.
It also makes the GC go crazy.
We have found that caching the buffer between the subsequent paint calls
improves performance a lot:
code
/** Stores the prepared offscreen buffer */
private BufferedImage bufferedImage;
/**
* Attempts to paint the display using an offscreen buffer.
*
* @param graphics
* The source graphics context.
*
* @return
* tttrue/tt if the display was painted using the offscreen
* buffer; ttfalse/tt, otherwise.
*/
private boolean paintBuffered(Graphics2D graphics) {
boolean painted = false;
// Paint the display into an offscreen buffer
GraphicsConfiguration gc = graphics.getDeviceConfiguration();
java.awt.Rectangle clipBounds = graphics.getClipBounds();
if (bufferedImage == null ||
bufferedImage.getWidth() clipBounds.width ||
bufferedImage.getHeight() clipBounds.height)
bufferedImage = gc.createCompatibleImage(clipBounds.width,
clipBounds.height,
Transparency.OPAQUE);
if (bufferedImage != null) {
Graphics2D bufferedImageGraphics =
(Graphics2D)bufferedImage.getGraphics();
bufferedImageGraphics.setClip(0, 0, clipBounds.width,
...
/code
Advantages:
1. it saves from costly allocation of a large object from possibly not-cached
memory region
2. after a few repaints the GC moves this object to the tenured generation,
so that the young generation collector is much more efficient (longer times
between runs)
3. the image probably stays most of the time in the L2 or L3 cache, which
saves on memory bandwidth and speeds up painting
Disadvantages:
1. uses some memory that is probably not required all the time, when the app
doesn't need to repaint anything large, however this is almost completely
shadowed by the excessive GC overhead due to continuous recreation of the
offscreen buffered image
Anyway, we observed about 2-4x performance increase by this simple change -
now when running at 60 FPS it uses only about 25% of CPU for painting, and
the rest can be used by the application logic (AI, etc.). Previously 60 FPS
was probably the most we could achieve from Core2Duo 2.2 GHz. Of course, this
change won't affect any business applications that don't do animations etc.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (PIVOT-776) Additional support for mouse wheel adjustments of Component values


[ 
https://issues.apache.org/jira/browse/PIVOT-776?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13071658#comment-13071658
 ] 

Chris Bartlett commented on PIVOT-776:
--

For this ticket, I will just go with whatever approach is preferred.

The concensus seems to be that the target Component should be focused, but what 
about mouse positioning?
Should the mouse when only work when the mouse is over the focused Component?   
 

For my own purposes, I think that I will probably end up using a 
'MouseWheelPolicy' of some kind to allow a change in the way that the mouse 
wheel is handled.  This will allow a 'simple' mode that requires focus and an 
'advanced' mode that doesn't.  This will probably be set at the application 
level and apply to all Component within an app.  


 Additional support for mouse wheel adjustments of Component values
 --

 Key: PIVOT-776
 URL: https://issues.apache.org/jira/browse/PIVOT-776
 Project: Pivot
  Issue Type: Improvement
  Components: wtk, wtk-terra
 Environment: n/a
Reporter: Chris Bartlett
Assignee: Chris Bartlett
Priority: Minor
 Fix For: 2.1


 I imagine that mouse wheel events would be ignored unless the Component was 
 focused *and* the mouse was over the Component.
 Spinner - Equivalent to an UP/DOWN arrow key press when 'circular' property 
 is true
 Slider - Equivalent to an UP/DOWN/LEFT/RIGHT arrow key press
 ListButton - Equivalent to an UP/DOWN arrow key press
 CalendarButton - No current UP/DOWN/LEFT/RIGHT arrow key press functionality 
 to change date by a single day, but would be equivalent to this.  Support for 
 these key presses could be added at the same time. 

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (PIVOT-700) SplashScreen control


[ 
https://issues.apache.org/jira/browse/PIVOT-700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13071659#comment-13071659
 ] 

Chris Bartlett commented on PIVOT-700:
--

I do intend to attach a simple test app, but haven't had time yet.

 SplashScreen control
 

 Key: PIVOT-700
 URL: https://issues.apache.org/jira/browse/PIVOT-700
 Project: Pivot
  Issue Type: New Feature
Affects Versions: 2.0
Reporter: Bojan Vucinic
 Fix For: 2.1

 Attachments: splash_screen.patch


 I've tried to get hold of the SplashScreen that I've defined in my 
 manifest.mf file:
 Manifest-Version: 1.0
 X-COMMENT: Main-Class will be added automatically by build
 SplashScreen-Image: com/macad/enbal/resources/welcome.jpg
 But once the DesktopApplicationContext takes over, it is impossible to get 
 the SplashScreen (null pointer exception is thrown).
 Could you extend the API so that we can show the SplashScreen, and close it 
 once all the init actions have been finished just before we open the 
 application window as the nameless gray pivot window is displayed quite a 
 long time.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

Re: TaskGroup, TaskSequence ExecutorService

Hi Chris,

 I wasted too much time yesterday realizing that TaskGroup 
 TaskSequence don't use the java.util.concurrent.ExecutorService that
 might be passed to their constructors.

If I understand the issue, I think you mean that these classes are not 
guaranteed to execute their subtasks using the same ExecutorService that they 
were given. Is that correct?

 I can't really work out the intent of the classes from the javadocs or
 decide if they are still required (at least in their current forms).
 Neither is used within Pivot itself.

They are just utility classes for executing a set of tasks in sequence or in 
parallel. Even though they are not used by the platform, I believe there is a 
unit test for these classes.

 Are they supposed to use a provided ExecutorService or is the
 constructor just a copy  paste mistake?

Not sure what you mean by this. TaskGroup(ExecutorService) and 
TaskSequence(ExecutorService) do appear to pass the given executor service to 
the base class. Am I missing something?

 If the former, then it is probably worth adding a
 setExecutorService(ExecutorService) method to
 org.apache.pivot.util.concurrent.Task so that TaskGroup  TaskSequence
 can control the ExecutorService to use when running the individual
 tasks.

Is the question whether we should enforce that subtasks are executed by the 
same executor service as their parent? If so, why might we want to do that?

G

[jira] [Commented] (PIVOT-778) Optimise DisplayHost.paintBuffered and DisplayHost.paintVolatileBuffered

[
https://issues.apache.org/jira/browse/PIVOT-778?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13071664#comment-13071664
]

Greg Brown commented on PIVOT-778:
--

Way back, I believe we were doing something like this but switched to the
current approach to save on memory. It may be reasonable to offer a switch to
toggle this behavior.

But I'm wondering why you need to repaint the entire 1280x1024 frame every
time. Is it not possible for you to only update the dirty region of your frame
during your animation?

Optimise DisplayHost.paintBuffered and DisplayHost.paintVolatileBuffered

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (PIVOT-656) FileBrowserSheet seems frozen while browsing network folders

[
https://issues.apache.org/jira/browse/PIVOT-656?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13071665#comment-13071665
]

Chris Bartlett commented on PIVOT-656:
--

I wrote a custom DirectoryChooser component which shows the file system roots
as a tree and retains a MRU list (most recently used) of chosen directories
among other things.

The cleanest solution I came up with was to record the time taken to enumerate
the drives/mount points and special directories, and then store a flag in the
application preferences to identify the ones that loaded slowly. The next time
the DirectoryChooser is used, those slow nodes would be populated on separate
threads, but could just as easily lazy loaded when/if they were accessed

FileBrowserSheet seems frozen while browsing network folders

Key: PIVOT-656
URL: https://issues.apache.org/jira/browse/PIVOT-656
Project: Pivot
Issue Type: Improvement
Affects Versions: 1.5.1
Reporter: A.J.
Fix For: 2.1

Our customer complains about the FileBrowserSheet being unresponsive while
browsing network folders.
I haven't been able to reproduce this in my development environment but I saw
this in 'action' (or inaction in this case).
They click a folder then ... nothing happens, they wait, then wait. They are
free to try other folders (I guess this is asynchronous task) but nothing
happens.
I've asked them to navigate the same network path with the Windows explorer
which has no apparent problem (some slowdowns at worst).
So, it's hard to say where the problems is (no stack trace) but there is a
problem.
One first improvement that could be done would be to display an hourglass
(busy cursor) while the current I/O operation is pending.
Another one could be also to set a timer after which the I/O operation would
be canceled (or abandonned) and give back hand to the calling code.
That's all I'm thinking fo r now.
As soon as I can get some more info on the origin of the problem, I'll update
this issue.
Cheers,
A.J.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

Re: TaskGroup, TaskSequence ExecutorService

Hi Greg.  Answers in line.

On 27 July 2011 18:37, Greg Brown gk_br...@verizon.net wrote:
 I wasted too much time yesterday realizing that TaskGroup 
 TaskSequence don't use the java.util.concurrent.ExecutorService that
 might be passed to their constructors.

 If I understand the issue, I think you mean that these classes are not 
 guaranteed to execute their subtasks using the same ExecutorService that they 
 were given. Is that correct?

No it is a little more than that.  When a standard Task is created, it
either uses a default ExecutorService or the one provided to it.  This
cannot be changed later.

TaskGroup  TaskSequence extend Task and also accept an
ExecutorService, but this is never used (no call to
getExecutorService())

In their execute() methods, they both just iterate over the added
Tasks and call
((TaskObject)task).execute(taskListener);
on each Tasks.

This will execute each Task using its internal ExecutorService, and
not the one supplied to the TaskGroup/TaskSequence.


 I can't really work out the intent of the classes from the javadocs or
 decide if they are still required (at least in their current forms).
 Neither is used within Pivot itself.

 They are just utility classes for executing a set of tasks in sequence or in 
 parallel. Even though they are not used by the platform, I believe there is a 
 unit test for these classes.

There is a 'unit test', but it has no assertions, comments or coverage
of the constructors that take an ExecutorService, so doesn't clarify
anything.


 Are they supposed to use a provided ExecutorService or is the
 constructor just a copy  paste mistake?

 Not sure what you mean by this. TaskGroup(ExecutorService) and 
 TaskSequence(ExecutorService) do appear to pass the given executor service to 
 the base class. Am I missing something?

They pass it to the base class, but it is never accessed.  That is why
I am not sure of the intent.  Is the supplied ExecutorService supposed
to be used?


 If the former, then it is probably worth adding a
 setExecutorService(ExecutorService) method to
 org.apache.pivot.util.concurrent.Task so that TaskGroup  TaskSequence
 can control the ExecutorService to use when running the individual
 tasks.

 Is the question whether we should enforce that subtasks are executed by the 
 same executor service as their parent?
Yes, that is what I thought would happen if I supply an ExecutorService.

 If so, why might we want to do that?
I thought that I would be able to do something like this to execute an
arbitrary group of Tasks in parallel, but limited to only 3 being
started at any given moment.

TaskGroup taskGroup = new TaskGroup(Executors.newFixedThreadPool(3));

Chris

Re: TaskGroup, TaskSequence ExecutorService

 If I understand the issue, I think you mean that these classes are not 
 guaranteed to execute their subtasks using the same ExecutorService that 
 they were given. Is that correct?
 
 No it is a little more than that.  When a standard Task is created, it
 either uses a default ExecutorService or the one provided to it.  This
 cannot be changed later.
 
 TaskGroup  TaskSequence extend Task and also accept an
 ExecutorService, but this is never used (no call to
 getExecutorService())
 
 In their execute() methods, they both just iterate over the added
 Tasks and call
 ((TaskObject)task).execute(taskListener);
 on each Tasks.
 
 This will execute each Task using its internal ExecutorService, and
 not the one supplied to the TaskGroup/TaskSequence.

Right. That's what I said.  ;-)

The ExecutorService given to TaskGroup and TaskSequence is used to execute the 
root task (the group or sequence). It just isn't used to execute the sub-tasks. 
If you want the sub-tasks to use the same executor service, you can just pass 
that service instance to the sub-task constructor.

 I thought that I would be able to do something like this to execute an
 arbitrary group of Tasks in parallel, but limited to only 3 being
 started at any given moment.
 
 TaskGroup taskGroup = new TaskGroup(Executors.newFixedThreadPool(3));

If you pass the fixed thread pool service to each of your individual tasks, you 
will get the behavior you want. You probably don't want to use the same service 
for the root task, since that would require a fourth thread.

G

Re: TaskGroup, TaskSequence ExecutorService

On 27 July 2011 19:21, Greg Brown gk_br...@verizon.net wrote:
 The ExecutorService given to TaskGroup and TaskSequence is used to execute 
 the root task (the group or sequence). It just isn't used to execute the 
 sub-tasks.

Fair enough, but as I said, the intent is far from clear, even with
access to the source.

IMHO this fails the often quoted 'might confuse users' test miserably.
(Partly because I am a user, and this left me miserable after
confusing me!)

If you want the sub-tasks to use the same executor service, you can just pass 
that service instance to the sub-task constructor.  If you pass the fixed 
thread pool service to each of your individual tasks, you will get the 
behavior you want.

Yes, that is what I discovered.  I end up having to tell the task how
it will be executed when it is created.  Or wrapping one task in
another task just to be able to specify the ExecutorService at a later
point.

But by then I couldn't really see what these classes offer.

[jira] [Commented] (PIVOT-694) Improvement in ListButton clear() method

[
https://issues.apache.org/jira/browse/PIVOT-694?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13071706#comment-13071706
]

Sandro Martini commented on PIVOT-694:
--

Hi Greg, thanks for the help ... you have reason, I was more focused only on a
clearSelection but probably for this there is already the
setSelectedIndex(-1) method in many components ... but I'm guessing if this
could be useful in ButtonGroup/RadioButtonGroup for example but this is another
story.
Maybe in my new Pivot694, add button for Clear Selection in all components,
and another for Clear Data, to test both behaviors.

I'll try to reproduce the condition where clear means delete all data inside
the element, and see what happens.
Then in other components, to see what happens.

Bye

Improvement in ListButton clear() method

Key: PIVOT-694
URL: https://issues.apache.org/jira/browse/PIVOT-694
Project: Pivot
Issue Type: Improvement
Components: wtk
Affects Versions: 2.0
Environment: Pivot 2.0
Win 7
Reporter: Luiz Gustavo Stábile de Souza
Assignee: Sandro Martini
Priority: Minor
Fix For: 2.0.1

Attachments: listButton problem.png

I'm having problems with ListButton.
When I try to clean a ListButton the last text selected before remains
selected, even with no item in the ListData of the ListButton.
I tried listButton.clear() and listButton.getListData().clear(), but with
none of this I can scape the problem.
I debugged the application, and the listData of the ListButton is clear after
getListData().clear(), but the last selected text remains showing.
See attached image.
Perhaps listButton.setButtonData(null) should be performed in
ListButton#clear().
Cheers

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

Re: TaskGroup, TaskSequence ExecutorService

 If you want the sub-tasks to use the same executor service, you can just 
 pass that service instance to the sub-task constructor.  If you pass the 
 fixed thread pool service to each of your individual tasks, you will get the 
 behavior you want.
 
 Yes, that is what I discovered.  I end up having to tell the task how
 it will be executed when it is created.

The same is true of any task - you provide the executor service to the 
constructor. A TaskGroup or TaskSequence is just a task that executes other 
tasks.

Might it make more sense to pass the executor service to the execute() method, 
rather than the constructor? That way, the service can easily be propagated to 
the sub-tasks.

 But by then I couldn't really see what these classes offer.

They offer exactly what was described earlier in the thread - a means for 
executing a set of tasks in sequence or in parallel.

[jira] [Resolved] (PIVOT-772) Exceptions thrown when calling getCharacterBounds of TextArea when incomplete lines exist

2011-07-27 Thread Noel Grandin (JIRA)


 [ 
https://issues.apache.org/jira/browse/PIVOT-772?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Noel Grandin resolved PIVOT-772.


Resolution: Fixed

Fixed in rev 1151471

Note that your code needs to change slightly: 

Bounds bounds = logTextArea.getCharacterBounds(textarea.getCharacterCount());
textarea.scrollAreaToVisible(bounds);

Note the absence of the -1 factor, since we actually want the position of the 
piece just past the end.


 Exceptions thrown when calling getCharacterBounds of TextArea when incomplete 
 lines exist
 -

 Key: PIVOT-772
 URL: https://issues.apache.org/jira/browse/PIVOT-772
 Project: Pivot
  Issue Type: Bug
  Components: wtk
Affects Versions: 2.0.1
 Environment: Windows XP SP3, JRE 1.6_25 or JRE 1.7 (beta)
Reporter: Roger Whitcomb
Assignee: Noel Grandin
Priority: Minor
 Fix For: 2.0.1

 Attachments: Scroll.java, Scroll2.java, scroll.bxml, scroll.log, 
 scroll2.log

   Original Estimate: 48h
  Remaining Estimate: 48h

 If partial lines are appended to a TextArea and then getCharacterBounds 
 followed by scrollAreaToVisible is called, Pivot throws 
 java.lang.IndexOutOfBoundsException from 
 TextAreaSkinParagraphView.getCharacterBounds:
 java.lang.IndexOutOfBoundsException: ix = 38
 at sun.font.StandardGlyphVector.getGlyphLogicalBounds(Unknown Source)
 at 
 org.apache.pivot.wtk.skin.TextAreaSkinParagraphView.getCharacterBounds(TextAreaSkinParagraphView.java:393)
 at 
 org.apache.pivot.wtk.skin.TextAreaSkin.getCharacterBounds(TextAreaSkin.java:442)
 at 
 org.apache.pivot.wtk.TextArea.getCharacterBounds(TextArea.java:1243)

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Resolved] (PIVOT-694) Improvement in ListButton clear() method


 [ 
https://issues.apache.org/jira/browse/PIVOT-694?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Greg Brown resolved PIVOT-694.
--

Resolution: Fixed

I just checked in what should be a fix for this. Please re-open if the problem 
persists.


 Improvement in ListButton clear() method
 

 Key: PIVOT-694
 URL: https://issues.apache.org/jira/browse/PIVOT-694
 Project: Pivot
  Issue Type: Improvement
  Components: wtk
Affects Versions: 2.0
 Environment: Pivot 2.0
 Win 7
Reporter: Luiz Gustavo Stábile de Souza
Assignee: Sandro Martini
Priority: Minor
 Fix For: 2.0.1

 Attachments: listButton problem.png


 I'm having problems with ListButton.
 When I try to clean a ListButton the last text selected before remains 
 selected, even with no item in the ListData of the ListButton.
 I tried listButton.clear() and listButton.getListData().clear(), but with 
 none of this I can scape the problem.
 I debugged the application, and the listData of the ListButton is clear after 
 getListData().clear(), but the last selected text remains showing.
 See attached image.
 Perhaps listButton.setButtonData(null)  should be performed in 
 ListButton#clear().
 Cheers

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Assigned] (PIVOT-694) Improvement in ListButton clear() method


 [ 
https://issues.apache.org/jira/browse/PIVOT-694?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Greg Brown reassigned PIVOT-694:


Assignee: Greg Brown  (was: Sandro Martini)

 Improvement in ListButton clear() method
 

 Key: PIVOT-694
 URL: https://issues.apache.org/jira/browse/PIVOT-694
 Project: Pivot
  Issue Type: Improvement
  Components: wtk
Affects Versions: 2.0
 Environment: Pivot 2.0
 Win 7
Reporter: Luiz Gustavo Stábile de Souza
Assignee: Greg Brown
Priority: Minor
 Fix For: 2.0.1

 Attachments: listButton problem.png


 I'm having problems with ListButton.
 When I try to clean a ListButton the last text selected before remains 
 selected, even with no item in the ListData of the ListButton.
 I tried listButton.clear() and listButton.getListData().clear(), but with 
 none of this I can scape the problem.
 I debugged the application, and the listData of the ListButton is clear after 
 getListData().clear(), but the last selected text remains showing.
 See attached image.
 Perhaps listButton.setButtonData(null)  should be performed in 
 ListButton#clear().
 Cheers

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (PIVOT-694) Improvement in ListButton clear() method

[
https://issues.apache.org/jira/browse/PIVOT-694?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13071758#comment-13071758
]

Sandro Martini commented on PIVOT-694:
--

Hi Greg, thank you very much for the fix, I'll try during next days ... and
I'll try even on other components to verify the same behavior.

On the clearSelection on ButtonGroup/RadioButtonGroup what do you think ?
I could put this in a dedicated ticket (maybe for the 2.1) if you think could
be useful to have.

Bye

Improvement in ListButton clear() method

Attachments: listButton problem.png

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

Re: TaskGroup, TaskSequence ExecutorService

On 27 July 2011 20:37, Greg Brown gk_br...@verizon.net wrote:
 If you want the sub-tasks to use the same executor service, you can just 
 pass that service instance to the sub-task constructor.  If you pass the 
 fixed thread pool service to each of your individual tasks, you will get 
 the behavior you want.

 Yes, that is what I discovered.  I end up having to tell the task how
 it will be executed when it is created.

 The same is true of any task - you provide the executor service to the 
 constructor. A TaskGroup or TaskSequence is just a task that executes other 
 tasks.

Yeah, I understand that, and it is part of the reason that I thought
these classes would do something a bit different and that is where
their value would come from.

 Might it make more sense to pass the executor service to the execute() 
 method, rather than the constructor? That way, the service can easily be 
 propagated to the sub-tasks.

I think that would certainly add value.
For now though, I think I'll end up rolling my own rather than use
these, especially as they are not used elsewhere within Pivot.

 But by then I couldn't really see what these classes offer.

 They offer exactly what was described earlier in the thread - a means for 
 executing a set of tasks in sequence or in parallel.

Unfortunately this thread is not referenced in the javadocs :)
As this thread's existence proves, the javadocs still left me with questions.

I just didn't (and still don't) see much difference between these
classes and other utility methods or classes that have been
rejected/declined as not offering much that an individual developer
couldn't quickly knock up themselves, to their exact specifications.

So rightly or wrongly, I was essentially judging these classes with my
preconceived notions gained from reading the mailing lists.

[jira] [Commented] (PIVOT-778) Optimise DisplayHost.paintBuffered and DisplayHost.paintVolatileBuffered

2011-07-27 Thread Noel Grandin (JIRA)

[
https://issues.apache.org/jira/browse/PIVOT-778?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13071762#comment-13071762
]

Noel Grandin commented on PIVOT-778:

We could always use a WeakReference to allow the GC to collect it if necessary.

But yes, it sounds like Piotr's application should be using partial repaints.

Optimise DisplayHost.paintBuffered and DisplayHost.paintVolatileBuffered

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (PIVOT-694) Improvement in ListButton clear() method


[ 
https://issues.apache.org/jira/browse/PIVOT-694?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13071761#comment-13071761
 ] 

Greg Brown commented on PIVOT-694:
--

I'll try even on other components to verify the same behavior.

This fix only applies to ListButton.


 Improvement in ListButton clear() method
 

 Key: PIVOT-694
 URL: https://issues.apache.org/jira/browse/PIVOT-694
 Project: Pivot
  Issue Type: Improvement
  Components: wtk
Affects Versions: 2.0
 Environment: Pivot 2.0
 Win 7
Reporter: Luiz Gustavo Stábile de Souza
Assignee: Greg Brown
Priority: Minor
 Fix For: 2.0.1

 Attachments: listButton problem.png


 I'm having problems with ListButton.
 When I try to clean a ListButton the last text selected before remains 
 selected, even with no item in the ListData of the ListButton.
 I tried listButton.clear() and listButton.getListData().clear(), but with 
 none of this I can scape the problem.
 I debugged the application, and the listData of the ListButton is clear after 
 getListData().clear(), but the last selected text remains showing.
 See attached image.
 Perhaps listButton.setButtonData(null)  should be performed in 
 ListButton#clear().
 Cheers

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (PIVOT-778) Optimise DisplayHost.paintBuffered and DisplayHost.paintVolatileBuffered

[
https://issues.apache.org/jira/browse/PIVOT-778?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13071776#comment-13071776
]

Greg Brown commented on PIVOT-778:
--

Actually, since it is a volatile image, a weak reference might not even be
necessary. The graphics system will reclaim any memory consumed by a volatile
image buffer if it needs to.

Optimise DisplayHost.paintBuffered and DisplayHost.paintVolatileBuffered

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Commented] (PIVOT-694) Improvement in ListButton clear() method

[
https://issues.apache.org/jira/browse/PIVOT-694?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13071785#comment-13071785
]

Sandro Martini commented on PIVOT-694:
--

This fix only applies to ListButton.
I understand this, I'm thinking to make similar tests on other components that
contains some data and could be selected, and see after something has been
selected, if clear reset even the selection ... for example in
ListView/TableView/TreeView/etc.

On the clearSelection on ButtonGroup/RadioButtonGroup what do you think ?
For example in for example ListView/TableView/TreeView it is present, so why
not even there ...
I could put this in a dedicated ticket (maybe for the 2.1) if you think could
be useful to have.
Sorry, but you didn't give me a comment on this ... what do you think ?

Thanks again.

Improvement in ListButton clear() method

Attachments: listButton problem.png

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

[jira] [Updated] (PIVOT-778) Optimise DisplayHost.paintBuffered and DisplayHost.paintVolatileBuffered


 [ 
https://issues.apache.org/jira/browse/PIVOT-778?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Sandro Martini updated PIVOT-778:
-

Comment: was deleted

(was: Comments ?)

 Optimise DisplayHost.paintBuffered and DisplayHost.paintVolatileBuffered
 

 Key: PIVOT-778
 URL: https://issues.apache.org/jira/browse/PIVOT-778
 Project: Pivot
  Issue Type: Improvement
  Components: wtk
Affects Versions: 2.0
Reporter: Piotr Kołaczkowski
Priority: Minor
  Labels: DisplayHost, caching, gc, paint, performance, repaint
 Fix For: 2.0.1


 We are writing sort of a game, which continually calls Component.repaint 
 method, at 60 FPS. We noticed excessive CPU usage, although the actual amount 
 of painting done by our component (actually in an overriden Panel.paint) is 
 ridiculously small. The profiler pointed us to the paintVolatileBuffered 
 method in the DisplayHost. What you are doing there is:
 1. obtain a new, fresh BufferedImage of size equal to the actual clip region, 
 let's say for a full screen game it can be about 1280x1024. This is 1.3 Mpix 
 x 4 bytes/pixel = 5.2 MB of raw data, allocated from a probably cold memory 
 region (not in the L2 cache)
 2. then you call actual paint on that buffered image (this is touching at 
 least 5.2 MB again)
 3. then you copy that to the onscreen buffer (which means copying 5.2 MB for 
 another time)
 4. in case GC kicks in after 1 and 3. it has to move the BufferedImage in 
 memory to compact young generation (= touching 5.2 MB fourth time)
 The whole process means allocating from cold memory 5.2 MB per each frame and 
 touching about 20 MB per frame.
 For 60 FPS it makes up ~300 MB/s allocation rate and 1.2GB memory throughput. 
 It also makes the GC go crazy.
 We have found that caching the buffer between the subsequent paint calls 
 improves performance a lot:
 code
 /** Stores the prepared offscreen buffer */
 private BufferedImage bufferedImage;
 /**
  * Attempts to paint the display using an offscreen buffer.
  *
  * @param graphics
  * The source graphics context.
  *
  * @return
  * tttrue/tt if the display was painted using the offscreen
  * buffer; ttfalse/tt, otherwise.
  */
 private boolean paintBuffered(Graphics2D graphics) {
 boolean painted = false;
 // Paint the display into an offscreen buffer
 GraphicsConfiguration gc = graphics.getDeviceConfiguration();
 java.awt.Rectangle clipBounds = graphics.getClipBounds();
 if (bufferedImage == null ||
 bufferedImage.getWidth()  clipBounds.width ||
 bufferedImage.getHeight()  clipBounds.height)
 bufferedImage = gc.createCompatibleImage(clipBounds.width, 
 clipBounds.height,
 Transparency.OPAQUE);
 if (bufferedImage != null) {
 Graphics2D bufferedImageGraphics = 
 (Graphics2D)bufferedImage.getGraphics();
 bufferedImageGraphics.setClip(0, 0, clipBounds.width,
 ...
 /code 
 Advantages:
 1. it saves from costly allocation of a large object from possibly not-cached 
 memory region
 2. after a few repaints the GC moves this object to the tenured generation, 
 so that the young generation collector is much more efficient (longer times 
 between runs)
 3. the image probably stays most of the time in the L2 or L3 cache, which 
 saves on memory bandwidth and speeds up painting
 Disadvantages:
 1. uses some memory that is probably not required all the time, when the app 
 doesn't need to repaint anything large, however this is almost completely 
 shadowed by the excessive GC overhead due to continuous recreation of the 
 offscreen buffered image
 Anyway, we observed about 2-4x performance increase by this simple change - 
 now when running at 60 FPS it uses only about 25% of CPU for painting, and 
 the rest can be used by the application logic (AI, etc.). Previously 60 FPS 
 was probably the most we could achieve from Core2Duo 2.2 GHz. Of course, this 
 change won't affect any business applications that don't do animations etc.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

Re: [jira] [Commented] (PIVOT-694) Improvement in ListButton clear() method

 This fix only applies to ListButton. 
 I understand this, I'm thinking to make similar tests on other components 
 that contains some data and could be selected, and see after something has 
 been selected, if clear reset even the selection ... for example in 
 ListView/TableView/TreeView/etc.

Good point.

 On the clearSelection on ButtonGroup/RadioButtonGroup what do you think ?
 For example in for example ListView/TableView/TreeView it is present, so why 
 not even there ...

I thought this was already supported, but I haven't verified.

Re: TaskGroup, TaskSequence ExecutorService

On 27 July 2011 21:51, Greg Brown gk_br...@verizon.net wrote:
 Unfortunately this thread is not referenced in the javadocs :)
 As this thread's existence proves, the javadocs still left me with questions.

 The Javadoc says pretty much the same thing as I wrote above:

With the crucial exception of how they accomplish what they
accomplish, and in particular, the use of a supplied ExecutorService.

This is a bit of a weird discussion to be having, because if the
javadocs included more information (such as the info in this thread),
then the thread would probably not exist.  But they don't, so it does
:)

 TaskSequence - Class that runs a sequence of tasks in series and notifies 
 listeners when all tasks are complete.

 TaskGroup - Class that runs a group of tasks in parallel and notifies 
 listeners when all tasks are complete.

I think you would agree that those single statements and the others in
the javadocs are not sufficient to understand how the tasks are
executed and therefore when it might be appropriate to use these
classes ...

 I just didn't (and still don't) see much difference between these
 classes and other utility methods or classes that have been
 rejected/declined as not offering much that an individual developer
 couldn't quickly knock up themselves, to their exact specifications.

 These classes add value because they address a common requirement for 
 application developers. They are non-trivial and aren't likely to vary from 
 application to application, so there's no need to force developers to write 
 them themselves.

... especially if their implementations are considered non-trivial.

I see this primarily as a case of missing/lacking javadocs.  The
purpose of the classes was not sufficiently clear to me as a consumer,
hence the original post.

More detail in the javadocs might save others the time that I lost.

Chris

Re: TaskGroup, TaskSequence ExecutorService

 The Javadoc says pretty much the same thing as I wrote above:
 
 With the crucial exception of how they accomplish what they
 accomplish, and in particular, the use of a supplied ExecutorService.

I agree that the Javadoc could provide more detail. But the fact that TaskGroup 
and TaskSequence might use a different executor service honestly never occurred 
to me, so I didn't mention it.

 TaskSequence - Class that runs a sequence of tasks in series and notifies 
 listeners when all tasks are complete.
 
 TaskGroup - Class that runs a group of tasks in parallel and notifies 
 listeners when all tasks are complete.
 
 I think you would agree that those single statements and the others in
 the javadocs are not sufficient to understand how the tasks are
 executed and therefore when it might be appropriate to use these
 classes ...

Actually, I think they are sufficient. The only subtlety is which executor 
service runs the tasks. Otherwise, the classes do exactly what the Javadoc says.

I think this may be more of an API design question than a documentation issue. 
The question is - do you want to allow callers to execute sub-tasks in a 
different executor service from the parent or not? Or, put another way, is a 
developer likely to expect that sub-tasks are executed in the same executor as 
the parent or not?

If the former is true, then I think the existing API and documentation is 
sufficient (though admittedly sparse). If the latter is true, then moving the 
ExecutorService to the execute() method and adding more detail to the docs 
would definitely be appropriate.

Re: TaskGroup, TaskSequence ExecutorService

(I'm not ignoring your points/questions - I will get back to them in a
separate reply)

The name TaskGroup, in combination with the javadocs, made me think of
a Group of Tasks that will be executed in parallel using the (default
or optionally supplied) ExecutorService.

I suppose I saw it as a ParallelTaskExecutor or TaskGroupExecutor, and
not a ParallelTask or TaskGroupTask.  This is probably where the
initial confusion came from.

Once you think of TaskGroup a just another Task, it follows that the
ExecutorService provided in the constructor would be used to just run
the execute() method of TaskGroup, as would happen with any other
Task.

So I might have approached it differently if the class names had an
extra 'Task' appended to them (TaskGroupTask/TaskSequenceTask), and/or
the javadocs said 'Task that runs...' rather than 'Class that runs
...'

Chris

On 27 July 2011 23:09, Greg Brown gk_br...@verizon.net wrote:
 The Javadoc says pretty much the same thing as I wrote above:

 With the crucial exception of how they accomplish what they
 accomplish, and in particular, the use of a supplied ExecutorService.

 I agree that the Javadoc could provide more detail. But the fact that 
 TaskGroup and TaskSequence might use a different executor service honestly 
 never occurred to me, so I didn't mention it.

 TaskSequence - Class that runs a sequence of tasks in series and notifies 
 listeners when all tasks are complete.

 TaskGroup - Class that runs a group of tasks in parallel and notifies 
 listeners when all tasks are complete.

 I think you would agree that those single statements and the others in
 the javadocs are not sufficient to understand how the tasks are
 executed and therefore when it might be appropriate to use these
 classes ...

 Actually, I think they are sufficient. The only subtlety is which executor 
 service runs the tasks. Otherwise, the classes do exactly what the Javadoc 
 says.

 I think this may be more of an API design question than a documentation 
 issue. The question is - do you want to allow callers to execute sub-tasks in 
 a different executor service from the parent or not? Or, put another way, is 
 a developer likely to expect that sub-tasks are executed in the same executor 
 as the parent or not?

 If the former is true, then I think the existing API and documentation is 
 sufficient (though admittedly sparse). If the latter is true, then moving the 
 ExecutorService to the execute() method and adding more detail to the docs 
 would definitely be appropriate.

Re: TaskGroup, TaskSequence ExecutorService

 I suppose I saw it as a ParallelTaskExecutor or TaskGroupExecutor, and
 not a ParallelTask or TaskGroupTask.  This is probably where the
 initial confusion came from.

I can understand that. OTOH, TaskGroupTask (or ParallelTaskGroupTask) is sort 
of verbose and arguably redundant, and as you noted, once you recognize that 
TaskGroup and TaskSequence extend Task, the existing behavior makes more sense.

G

Re: TaskGroup, TaskSequence ExecutorService

In hindsight I could have rolled this into the last email if I had
read it more thoroughly first - sorry about that!

Responses below...

On 27 July 2011 23:09, Greg Brown gk_br...@verizon.net wrote:
 The Javadoc says pretty much the same thing as I wrote above:

 With the crucial exception of how they accomplish what they
 accomplish, and in particular, the use of a supplied ExecutorService.

 I agree that the Javadoc could provide more detail. But the fact that 
 TaskGroup and TaskSequence might use a different executor service honestly 
 never occurred to me, so I didn't mention it.

 TaskSequence - Class that runs a sequence of tasks in series and notifies 
 listeners when all tasks are complete.

 TaskGroup - Class that runs a group of tasks in parallel and notifies 
 listeners when all tasks are complete.

 I think you would agree that those single statements and the others in
 the javadocs are not sufficient to understand how the tasks are
 executed and therefore when it might be appropriate to use these
 classes ...

 Actually, I think they are sufficient. The only subtlety is which executor 
 service runs the tasks. Otherwise, the classes do exactly what the Javadoc 
 says.

Again, that was the crucial, missing part for me.  See the other email
I just sent for why.


 I think this may be more of an API design question than a documentation 
 issue. The question is - do you want to allow callers to execute sub-tasks in 
 a different executor service from the parent or not? Or, put another way, is 
 a developer likely to expect that sub-tasks are executed in the same executor 
 as the parent or not?

 If the former is true, then I think the existing API and documentation is 
 sufficient (though admittedly sparse). If the latter is true, then moving the 
 ExecutorService to the execute() method and adding more detail to the docs 
 would definitely be appropriate.


This is kind of why it didn't seem to me to belong in Pivot - having
to second guess how it will be used, and making it flexible enough to
justify its existence.

Looking at this from the point of view that these classes are
primarily Tasks with an in built listener (which seems to be how they
were designed), I can see how they could be useful to others.  That is
not how it looked from the perspective of these classes being some
kind of TaskExecutors, without them using a supplied ExecutorService.
(Refer to my other mail if you have not already)

Chris

Re: TaskGroup, TaskSequence ExecutorService

On 27 July 2011 23:43, Greg Brown gk_br...@verizon.net wrote:
 I suppose I saw it as a ParallelTaskExecutor or TaskGroupExecutor, and
 not a ParallelTask or TaskGroupTask.  This is probably where the
 initial confusion came from.

 I can understand that. OTOH, TaskGroupTask (or ParallelTaskGroupTask) is sort 
 of verbose and arguably redundant, and as you noted, once you recognize that 
 TaskGroup and TaskSequence extend Task, the existing behavior makes more 
 sense.


(I was/am not pushing to change the names - just explaining how I came
to the conclusions I did)

Long, but I wouldn't say verbose.  Especially in the world of (pre
1.7) java generics!

The name 'TaskGroupTask' captures something that is lost when the last
'Task' is removed.  That information is obviously available in the
javadocs themselves, source code or your IDE of choice, but it clearly
passed me by.  Arguably, a total lack of javadocs might have helped me
pick up on the fact that it is just a Task.

While TaskGroupTask initially looks redundant, I think it is easily
justifiable simply because it is a Task that does something with a
Group of Tasks (which in Pivot would be called a TaskGroup)

GroupedTask is shorter and makes it clear that it is a Task but loses
something along the way.  What is it that is grouped?
GroupedTasksTask is just hideous ;)

Re: TaskGroup, TaskSequence ExecutorService

By that logic, we'd need to qualify every class in the frame work. E.g. we 
wouldn't just have a Button, we'd have a ButtonComponent. That is obviously 
overkill, and I think it would be overkill here as well.

On Jul 27, 2011, at 1:11 PM, Chris Bartlett wrote:

 On 27 July 2011 23:43, Greg Brown gk_br...@verizon.net wrote:
 I suppose I saw it as a ParallelTaskExecutor or TaskGroupExecutor, and
 not a ParallelTask or TaskGroupTask.  This is probably where the
 initial confusion came from.
 
 I can understand that. OTOH, TaskGroupTask (or ParallelTaskGroupTask) is 
 sort of verbose and arguably redundant, and as you noted, once you recognize 
 that TaskGroup and TaskSequence extend Task, the existing behavior makes 
 more sense.
 
 
 (I was/am not pushing to change the names - just explaining how I came
 to the conclusions I did)
 
 Long, but I wouldn't say verbose.  Especially in the world of (pre
 1.7) java generics!
 
 The name 'TaskGroupTask' captures something that is lost when the last
 'Task' is removed.  That information is obviously available in the
 javadocs themselves, source code or your IDE of choice, but it clearly
 passed me by.  Arguably, a total lack of javadocs might have helped me
 pick up on the fact that it is just a Task.
 
 While TaskGroupTask initially looks redundant, I think it is easily
 justifiable simply because it is a Task that does something with a
 Group of Tasks (which in Pivot would be called a TaskGroup)
 
 GroupedTask is shorter and makes it clear that it is a Task but loses
 something along the way.  What is it that is grouped?
 GroupedTasksTask is just hideous ;)

Re: TaskGroup, TaskSequence ExecutorService

Yes, that absolutely makes sense. The fact that it says class draws attention 
away from the fact that it is actually a Task.

On Jul 27, 2011, at 1:13 PM, Chris Bartlett wrote:

 On 27 July 2011 23:37, Chris Bartlett cbartlet...@gmail.com wrote:
 So I might have approached it differently if ...
 the javadocs said 'Task that runs...' rather than 'Class that runs
 
 If nobody has any objections I will make that small change today.

Re: TaskGroup, TaskSequence ExecutorService

 (I was/am not pushing to change the names - just explaining how I came
 to the conclusions I did)

Re: TaskGroup, TaskSequence ExecutorService

Right - and I'm just explaining why they are not named that way.  :-)

On Jul 27, 2011, at 1:22 PM, Chris Bartlett wrote:

 (I was/am not pushing to change the names - just explaining how I came
 to the conclusions I did)

[jira] [Commented] (PIVOT-772) Exceptions thrown when calling getCharacterBounds of TextArea when incomplete lines exist

2011-07-27 Thread Roger Whitcomb (JIRA)


[ 
https://issues.apache.org/jira/browse/PIVOT-772?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13071898#comment-13071898
 ] 

Roger Whitcomb commented on PIVOT-772:
--

Hi Noel,
Just tried it and it seems to work for the normal cases, but if I do a paste 
into a TextArea the code goes into an infinite loop, eventually leading to 
StackOverflowError:
Exception in thread AWT-EventQueue-0 java.lang.StackOverflowError
at java.awt.Component.repaint(Unknown Source)
at java.awt.Component.repaint(Unknown Source)
at 
org.apache.pivot.wtk.ApplicationContext$DisplayHost.repaint(ApplicationContext.java:414)
at org.apache.pivot.wtk.Display.repaint(Display.java:97)
at org.apache.pivot.wtk.Component.repaint(Component.java:2041)
at org.apache.pivot.wtk.Component.repaint(Component.java:2041)
at org.apache.pivot.wtk.Component.repaint(Component.java:2041)
at org.apache.pivot.wtk.Component.repaint(Component.java:2041)
at org.apache.pivot.wtk.Component.repaint(Component.java:2041)
at org.apache.pivot.wtk.Component.repaint(Component.java:2041)
at org.apache.pivot.wtk.Component.repaint(Component.java:2041)
at org.apache.pivot.wtk.Component.repaint(Component.java:2041)
at org.apache.pivot.wtk.Component.repaint(Component.java:2041)
at org.apache.pivot.wtk.Viewport.repaint(Viewport.java:174)
at org.apache.pivot.wtk.Component.repaint(Component.java:2000)
at org.apache.pivot.wtk.Component.repaint(Component.java:1985)
at org.apache.pivot.wtk.Component.setSize(Component.java:935)
at 
org.apache.pivot.wtk.skin.ScrollPaneSkin.layoutHelper(ScrollPaneSkin.java:730)
at 
org.apache.pivot.wtk.skin.ScrollPaneSkin.layout(ScrollPaneSkin.java:585)
at org.apache.pivot.wtk.Component.layout(Component.java:1960)
at org.apache.pivot.wtk.Container.layout(Container.java:348)
at org.apache.pivot.wtk.Component.validate(Component.java:1951)
at org.apache.pivot.wtk.Container.layout(Container.java:352)
at org.apache.pivot.wtk.Component.validate(Component.java:1951)
at org.apache.pivot.wtk.Container.layout(Container.java:352)
at org.apache.pivot.wtk.Component.validate(Component.java:1951)
at org.apache.pivot.wtk.Container.layout(Container.java:352)
at org.apache.pivot.wtk.Component.validate(Component.java:1951)
at org.apache.pivot.wtk.Container.layout(Container.java:352)
at org.apache.pivot.wtk.Component.validate(Component.java:1951)
at org.apache.pivot.wtk.Container.layout(Container.java:352)
at org.apache.pivot.wtk.Component.validate(Component.java:1951)
at org.apache.pivot.wtk.Container.layout(Container.java:352)
at org.apache.pivot.wtk.Component.validate(Component.java:1951)
at org.apache.pivot.wtk.Container.layout(Container.java:352)
at org.apache.pivot.wtk.Component.validate(Component.java:1951)
at org.apache.pivot.wtk.Container.layout(Container.java:352)
at org.apache.pivot.wtk.Component.validate(Component.java:1951)
at org.apache.pivot.wtk.Container.layout(Container.java:352)
at org.apache.pivot.wtk.Component.validate(Component.java:1951)
at 
org.apache.pivot.wtk.Component.scrollAreaToVisible(Component.java:1831)
at 
org.apache.pivot.wtk.skin.TextAreaSkin.scrollCharacterToVisible(TextAreaSkin.java:462)
at org.apache.pivot.wtk.skin.TextAreaSkin.layout(TextAreaSkin.java:238)
at org.apache.pivot.wtk.Component.layout(Component.java:1960)
at org.apache.pivot.wtk.Component.validate(Component.java:1951)
at org.apache.pivot.wtk.Container.layout(Container.java:352)
at org.apache.pivot.wtk.Component.validate(Component.java:1951)
at org.apache.pivot.wtk.Container.layout(Container.java:352)
at org.apache.pivot.wtk.Component.validate(Component.java:1951)
at org.apache.pivot.wtk.Container.layout(Container.java:352)
at org.apache.pivot.wtk.Component.validate(Component.java:1951)
at org.apache.pivot.wtk.Container.layout(Container.java:352)
at org.apache.pivot.wtk.Component.validate(Component.java:1951)
at org.apache.pivot.wtk.Container.layout(Container.java:352)
at org.apache.pivot.wtk.Component.validate(Component.java:1951)
at org.apache.pivot.wtk.Container.layout(Container.java:352)
at org.apache.pivot.wtk.Component.validate(Component.java:1951)
at org.apache.pivot.wtk.Container.layout(Container.java:352)
at org.apache.pivot.wtk.Component.validate(Component.java:1951)
at org.apache.pivot.wtk.Container.layout(Container.java:352)
at org.apache.pivot.wtk.Component.validate(Component.java:1951)
at org.apache.pivot.wtk.Container.layout(Container.java:352)
at

Re: TaskGroup, TaskSequence ExecutorService

Actually, I'm not sure that logic does follow.  Obviously the
Component suffix is (sanely) not used, but plenty of others are.

The 'TaskGroupTask' does actually capture something extra and the
'Task' suffix is certainly not without precedent.  In fact, as far as
I can see, TaskGroup and TaskSequence are actually the only named
subclasses of Task that *do not* have the 'Task' suffix.

(IOTask, LoadTask, LoadDataTask, RefreshFileListTask, SampleTask 
SleepTask x 2)
(DeleteQuery, GetQuery, PostQuery  PutQuery are all subclasses of
org.apache.pivot.web.Query so understandably have the 'Query' suffix
instead.)


TaskGroup implements a GroupTask and extends Task, but its name
doesn't reflect that it is a Task, perhaps just because the word
'Task' would be included twice.

ButtonGroup is a GroupButton so by the same logic, TaskGroup is fine
for GroupTask, but it just seems to (choose to) forget about the
Task part.

What if TaskGroup had a companion class that also extended Task and
implemented GroupRunnable?  It might wrap each Runnable in a Task
and delegate to TaskGroup for execution.  (Kind of a parallel Runnable
- Task adapter).

Should that be named RunnableGroup or RunnableGroupTask?


This is not a big deal, but the more I think about it, the more I
think it might have saved me a lot of time (and maybe even the thread)
if these classes did have the 'Task' suffix,


On 28 July 2011 00:20, Greg Brown gk_br...@verizon.net wrote:
 By that logic, we'd need to qualify every class in the frame work. E.g. we 
 wouldn't just have a Button, we'd have a ButtonComponent. That is 
 obviously overkill, and I think it would be overkill here as well.

 On Jul 27, 2011, at 1:11 PM, Chris Bartlett wrote:

 On 27 July 2011 23:43, Greg Brown gk_br...@verizon.net wrote:
 I suppose I saw it as a ParallelTaskExecutor or TaskGroupExecutor, and
 not a ParallelTask or TaskGroupTask.  This is probably where the
 initial confusion came from.

 I can understand that. OTOH, TaskGroupTask (or ParallelTaskGroupTask) is 
 sort of verbose and arguably redundant, and as you noted, once you 
 recognize that TaskGroup and TaskSequence extend Task, the existing 
 behavior makes more sense.


 (I was/am not pushing to change the names - just explaining how I came
 to the conclusions I did)

 Long, but I wouldn't say verbose.  Especially in the world of (pre
 1.7) java generics!

 The name 'TaskGroupTask' captures something that is lost when the last
 'Task' is removed.  That information is obviously available in the
 javadocs themselves, source code or your IDE of choice, but it clearly
 passed me by.  Arguably, a total lack of javadocs might have helped me
 pick up on the fact that it is just a Task.

 While TaskGroupTask initially looks redundant, I think it is easily
 justifiable simply because it is a Task that does something with a
 Group of Tasks (which in Pivot would be called a TaskGroup)

 GroupedTask is shorter and makes it clear that it is a Task but loses
 something along the way.  What is it that is grouped?
 GroupedTasksTask is just hideous ;)

Re: TaskGroup, TaskSequence ExecutorService

 Actually, I'm not sure that logic does follow.  Obviously the
 Component suffix is (sanely) not used, but plenty of others are.

We have used suffixes where there is no common alternative. For example, we 
have TabPane but not AccordionPane. Similarly, we have 
org.apache.pivot.web.Query, not QueryTask.

 TaskGroup implements a GroupTask and extends Task, but its name
 doesn't reflect that it is a Task, perhaps just because the word
 'Task' would be included twice.

Correct - it is redundant. You only need to qualify things if their meaning is 
ambiguous. Since a task group is defined as a task that is a group of 
tasks, the there is no ambiguity and the Task suffix is unnecessary. Same 
for task sequence.

 What if TaskGroup had a companion class that also extended Task and
 implemented GroupRunnable?  It might wrap each Runnable in a Task
 and delegate to TaskGroup for execution.  (Kind of a parallel Runnable
 - Task adapter).
 
 Should that be named RunnableGroup or RunnableGroupTask?

Difficult to answer since the question is somewhat contrived. We tend to focus 
heavily on real use cases since that also helps us come up with the most 
accurate names for the things that are being modeled.

Re: TaskGroup, TaskSequence ExecutorService

 Thanks for bearing with me.  I assure you these are genuine
 questions asked because I get the impression you have studied API
 design quite a bit, or at the very least have an interest in it, and
 some strong opinions. (What other kind are worth a damn?)

:-)

 We have used suffixes where there is no common alternative. For example, we 
 have TabPane but not AccordionPane. Similarly, we have 
 org.apache.pivot.web.Query, not QueryTask.
 
 Yes, and then there are DeleteQuery, GetQuery, PostQuery  PutQuery.

That's because these names need to be qualified. You wouldn't know what a 
Delete is, so we need to make it explicit.

 a task group is defined as a task that is a group of tasks, the there is 
 no ambiguity and the Task suffix is unnecessary. Same for task sequence.
 
 Where is task group defined like that?   

In the API. That's what the TaskGroup class represents (though the Javadoc 
obviously does not make this clear).  ;-)

 That does make perfect sense but only with the knowledge of
 the class itself...

Right - some classes are just like that. You can't always bake everything you 
need to know about a class into the name.

 Both parts seem equally important to me. The 'TaskGroup' part to know
 that the Group interface is used to provide  manipulate the Tasks,

I think that making TaskGroup a Group was a mistake on my part. There is no 
need to ensure uniqueness here. 

In retrospect, I think these classes should have been called ParallelTaskGroup 
and SerialTaskGroup (no association with the Group interface). They should 
extend an abstract base class called TaskGroup that defines a sequence of tasks 
(i.e. getTasks():TaskSequence). TaskGroup would itself extend Task. I think 
that API helps makes it clear what each of these classes does.

 and the 'Task' suffix to know that it is a Task and can be executed
 and used as such.

Not to be rude, but...RTFM?  :-)  It is pretty clear from the Javadoc that 
TaskGroup extends Task. 

 If all of the other examples of classes that extend Task have a suffix
 (be it 'Task' or 'Query', with org.apache.pivot.web.Query itself being
 a special case) then I see a discrepancy which could lead to thinking
 that TaskGroup and TaskSequence are not Tasks until they are more
 closely examined.  

This comes back to my Accordion example. Did you assume that an Accordion was 
not a component because it was not called AccordionComponent? Probably not. 
Accordion is a natural name for this type of control, so a suffix is not 
necessary. Also, Accordion lives in the org.apache.pivot.wtk package, which you 
know primarily contains user interface components. Similarly, TaskGroup and 
TaskSequence live in org.apache.pivot.util.concurrent along with a handful of 
other classes, all of which are used to support the Task class. So that's a 
pretty strong clue as to what these classes do.

 If...other classes have a suffix, and are instantly identifiable
 as a Task just by class name, then why not these?  Or should the
 others not have them either? Or is it just personal preference and
 readability?

It is readability. We don't design class names for machine parsing. We design 
them for use by people, and the English language does not always map perfectly 
to things we want to model in code.

 It is a genuine question asked in an attempt to understand the naming
 of these two classes.  My logic (understandably) makes sense to me,
 but I just can't see where/why we differ regarding the suffix.
 
 Here is a quick dump of how my brain works...
 
 We both seem to agree on the Group naming
 GroupFoo == FooGroup  // Group of Foos
 GroupTask == TaskGroup  // Group of Tasks
 GroupRunnable == RunnableGroup  // Group of Runnables
 
 So the above question was intended to ask if you would consider the
 'Task' prefix to be redundant on things like this...

Again, I think part of the problem is that TaskGroup shouldn't actually *be* a 
Group. It should *have* a sequence of sub-tasks. I think that would help 
alleviate some confusion.

Re: TaskGroup, TaskSequence ExecutorService

On 28 July 2011 06:06, Greg Brown gk_br...@verizon.net wrote:
 We have used suffixes where there is no common alternative. For example, we 
 have TabPane but not AccordionPane. Similarly, we have 
 org.apache.pivot.web.Query, not QueryTask.

 Yes, and then there are DeleteQuery, GetQuery, PostQuery  PutQuery.

 That's because these names need to be qualified. You wouldn't know what a 
 Delete is, so we need to make it explicit.

Yeah, I agree they wouldn't mean much on their own, but that brings me
back to the 'TaskGroup == GroupTask' thing.  Without a suffix, some
meaning is also lost.  Without the 'Task' I don't know what a
TaskGroup is, just the data type  structure that it probably
represents.  Adding the Task suffix won't tell me everything, but it
would help.

My main point here was more the value that the qualifying suffix can
give, regardless of whether it is strictly required.

This kind of thing can certainly be taken to extremes though.  For
instance some organizations have arcane database naming rules that
prefix a kind of Hungarian Notation into the names of everything.  It
gets worse when they also choose to abbreviate by removing vowels and
the like.  In the end the only people who can decipher them at a
glance are the DBAs with tools that make the naming redundant.


 a task group is defined as a task that is a group of tasks, the there 
 is no ambiguity and the Task suffix is unnecessary. Same for task 
 sequence.

 Where is task group defined like that?

 In the API. That's what the TaskGroup class represents (though the Javadoc 
 obviously does not make this clear).  ;-)

That makes me feel a little less stupid.
It is a bit of a stretch to get to 'is defined as' from 'what it
represents', and expect users to  fill in the gaps.  If I know of a
class and its intent, then the name could be seen as having little
consequence other than aesthetics.

My original questions were about the intent of the classes, so that
side of things could certainly be improved to help the slower users
like myself.


 That does make perfect sense but only with the knowledge of
 the class itself...

 Right - some classes are just like that. You can't always bake everything you 
 need to know about a class into the name.

Yes, unless you are fond of 2 page wide class names, a class is
unlikely to capture every bit of pertinent information, but with these
classes, it seems like more of an over eager pruning of information
has occurred just to make the class names sound a little more natural.

They could easily have the Task suffix 'baked in' to indicate that
they are Tasks, but they don't because it was deemed redundant (or
just ugly).  I don't see Task bit as redundant, but do see its 'loss'
as a missed opportunity.

If there is a problem with the readability of a class name such
'TaskGroupTask' for instance, it suggests that the name is not right
and perhaps there is a better way to express it (maybe by looking at
it from the another angle such as how it will be used as opposed to
what it is).  Discarding part of it so that it reads better seems like
a hack for want of a better word and only addresses part of the
problem.
(Your suggestion in an upcoming paragraph is much better IMO)


 Both parts seem equally important to me. The 'TaskGroup' part to know
 that the Group interface is used to provide  manipulate the Tasks,

 I think that making TaskGroup a Group was a mistake on my part. There is no 
 need to ensure uniqueness here.

 In retrospect, I think these classes should have been called 
 ParallelTaskGroup and SerialTaskGroup (no association with the Group 
 interface). They should extend an abstract base class called TaskGroup that 
 defines a sequence of tasks (i.e. getTasks():TaskSequence). TaskGroup would 
 itself extend Task. I think that API helps makes it clear what each of these 
 classes does.

Yes that would certainly be clearer in my eyes.  The reuse of 'Group'
is unfortunate but understandable.

The name ParallelTaskGroup would certainly raise a few questions in my
mind and drive me to investigate further, unlike the 'camouflaged'
TaskGroup.


 and the 'Task' suffix to know that it is a Task and can be executed
 and used as such.

 Not to be rude, but...RTFM?  :-)  It is pretty clear from the Javadoc that 
 TaskGroup extends Task.

Yes, it is perfectly clear, but only once you get that far.  The point
there is obviously that if the suffix existed in the name, then it
might direct someone to the javadocs in the first place.  It is not as
if it is supposed to be a hidden implementation detail after all.  It
is how you make it do the one thing it does.

If TaskGroup was named ParallelTaskGroup, ParallelTaskExecutor or
something similarly far removed from its current name, I wouldn't be
stressing the 'Task' suffix.  I'm mainly doing it now because without
it, it just sounds like a GroupTasks, and it is used in the other
named Task subclasses.

'TaskGroupTask' is undoubtedly clunkly, but at least is

Re: TaskGroup, TaskSequence ExecutorService