Hi Larry,
this all fits together very well and I have
a couple of comments/ideas. This is most radical one:
Let me switch to "software designer mode" for a moment.
(This is what my company is paying me for besides working
with legacy code... ;-)
We have actually two problems:
1 - We cannot determine exactly when a rendering
has ended (successfully or not).
This is important for e.g. printing and zooming+flashing.
The 'wakeup' Runnable trick only works if all jobs are
processed in a serial manner. It fails when DB or WMS layers
are involved because these are processed truly parallel.
What we really want is some kind of a reliable barrier
mechanism that triggers certain things after a rendering.
(unlocking a wait in the printing case and flashing in case of
zoom to selection)
2 - We have a javax,swing.Timer in RenderingManager
that triggers a repaint any x ms.
To what value we should set x?
A x too large makes the the GUI feels sluggish. x choosen too
small costs a lot of performance. I've got interesting profiling
numbers from JProbe that told me that the copyTo() stuff can
take up to 25% of the rendering time when you set x too low.
If we ask how we have to set x, we're asking the wrong question.
The question should state:
Has a renderer made enough progress to do a repaint?
If the on screen differences between two repaints would be too
small, why should we waste time to do a redraw? Only a significant
change (say a quarter of features drawn more) is worth this expense.
With a fixed scheduled timer you cannot test this efficiently.
This has to be done in cooperation with the particular renderer.
How to address these issues?
In my "software designer" dreamland i would propose the following:
1 - Remove the fixed scheduled timer
2 - Change the Renderer interface (Gulp!)
I would introduce an inner interface 'RenderingMaster':
interface RenderingMaster {
void doRepaint(Renderer thisRenderer);
void renderingDone(Renderer thisRenderer, boolean success);
}
and change the signature of createRunnable() to:
Runnable createRunnable(RenderingMaster master);
The idea of this extra callback should be pretty obvious.
Each time a renderer has produced a significant part of
output it calls master.doRepaint(this). On the implementors
side of RenderingMaster we can do a repaint.
If a renderer finished its job it calls
master.renderingDone(this, true). (false after canceling)
On the implementors side of RenderingMaster we can do some
reference counting and kick some listeners when all jobs are
done.
>From designer dreamland back to the reality of legacy code:
I would estimate about 1-2 days of work to refactor the core code
to use this new pattern + some testing.
The real problem arises from the fact that there is a sky
full of JUMPs and a lot of non-core Renderers out there.
This modification would be a heavy one and I
don't really dare to think about the consequences ...
Things like mouse wheel zooming may profit from this change too.
So I don't want to wipe off this idea.
Any comments?
Regards,
Sascha
PS: If you don't like it all I have some 'workaround' ideas too ...
Larry Becker schrieb:
> Hi Sascha,
>
> I have figured out what is different about rendering timing in
> SkyJUMP and OpenJump. The randomly delayed drawing in OpenJump is
> caused by the repaintTimer in RenderingManager. In OpenJump and JUMP it
> is set to 1 second, and in SkyJUMP it is set to 400 ms. This makes
> SkyJUMP rendering more responsive to support Mouse Wheel zooming,
> although still somewhat random.
>
> When the number of items withing the current window for a given layer
> falls below the maxFeatures threshold, the
> simpleFeatureCollectionRenderer is used, otherwise
> imageCachingFeatureCollectionRenderer is used. Image Caching implies
> that we have to wait for the repaintTimer to expire before all of the
> results of render operations are guaranteed to be copied on screen.
> When image Caching is disabled, drawing is synchronized but unresponsive
> because nothing appears on the screen until the end of the process.
>
> I'm not sure we can do anything about the repaintTimer's apparent
> randomness. The whole point of having a repaint timer is to be
> unsynchronized with the layer rendering process. Setting the timer
> value to 400 ms seems to make it responsive enough for the
> ZoomToSelectedItemsPlugIn. We'll need to do that anyway when mouse
> wheel zooming in ported over.
>
> regards,
> Larry
>
> On 6/18/07, *Larry Becker* <[EMAIL PROTECTED]
> <mailto:[EMAIL PROTECTED]>> wrote:
>
> Sascha,
>
> I replaced the ThreadQueue.Listener with the following code:
>
> panel.getRenderingManager().getDefaultRendererThreadQueue().add(
> new Runnable() {
> public void run() {
> try {
> flash(geometries, panel);
> } catch (NoninvertibleTransformException e) {};
> }
> });
>
> This works great in SkyJUMP where I also used it to fix my refresh
> timer and ZoomRealtime, however although it is better than the
> Listener in OpenJump, it still occasionally flashes first and then
> zooms. Clearly there is something wrong, but it is not in your
> ThreadQueue code. I'll look some more tomorrow.
>
> regards,
> Larry
>
>
> On 6/18/07, *Larry Becker* < [EMAIL PROTECTED]
> <mailto:[EMAIL PROTECTED]>> wrote:
>
> Sascha,
>
> > Don't you have the same effects with the original one?
>
> I begin to see... I can reproduce flash problems easily in JUMP
> and OpenJump, but not in SkyJUMP. That explains why we are both
> surprised. I have no idea why there is a difference, but I will
> investigate further.
>
> regards,
> Larry
>
> On 6/18/07, Sascha L. Teichmann < [EMAIL PROTECTED]
> <mailto:[EMAIL PROTECTED]>> wrote:
> > Larry,
> >
> > _exactly_ this the thread lottery we are playing with the
> > assumption that no running threads means there no more
> rendering jobs!
> >
> > I get the same irritating behavior with the original ThreadQueue.
> > I put an System.err.println("flash!") into the listener of
> > the zoom plug-in. Sometimes it gets printed before the display
> > is in the right 'mood' to display a flash. Result: no visible
> > flash or only a shortened variant.
> >
> > Don't you have the same effects with the original one?
> > I have!
> >
> > Register a println Listener yourself to the ThreadQueue
> > and be surprised how often it is called.
> >
> > The zoom plug-in builds on this assumption the even more venturous
> > assumption that the zoom is done when there no more running
> threads.
> > It does not take into account that the hole
> repaint()/erase()/copyTo()
> > stuff also takes some time. The invokeLater() call does not make it
> > more predictable.
> >
> > Let us compare the TQs:
> >
> > 1) Original TQ:
> >
> > public void add(final Runnable runnable) {
> > queuedRunnables.add(new Runnable() {
> > public void run() {
> > try {
> > runnable.run();
> > } finally {
> > setRunningThreads(getRunningThreads() - 1);
> > processQueue();
> > }
> > }
> > });
> > processQueue();
> > }
> >
> > private void setRunningThreads(int runningThreads) {
> > this.runningThreads = runningThreads;
> > if (runningThreads == 0) { fireAllRunningThreadsFinished(); }
> > }
> >
> > The defaultRenderingThread has only one Thread running.
> > -> runningThreads == 1 during try block of new Runnable.run().
> >
> > setRunningThread() in the finally sets it to zero
> > -> listeners get there kick.
> >
> > This means that after each and every job the listeners get
> kicked.
> >
> > 2) New TQ: (in worker thread's run())
> >
> > for (;;) {
> > // unimportant part
> > try {
> > runnable.run();
> > }
> > catch (Exception e) {
> > e.printStackTrace();
> > }
> >
> > boolean lastRunningThread;
> > synchronized (runningThreads) {
> > lastRunningThread = runningThreads[0] == 1;
> > }
> > if (lastRunningThread) {
> > fireAllRunningThreadsFinished();
> > }
> > }
> >
> > The defaultRenderingThread has only one Thread running.
> > -> runningThreads[0] == 1
> >
> > after the try block lastRunningThread is set to true
> > if runningThreads[0] == 1. This is always fulfilled for
> > the defaultRenderingThread.
> > -> listeners get there kick.
> >
> > This means that after each and every job the listeners get
> kicked.
> >
> > => Same behavior for 1) and 2)
> >
> > Maybe I bore you a bit by repeating it.
> >
> > Regards,
> > Sascha
> >
> >
> > Larry Becker schrieb:
> > > Sascha,
> > >
> > > Thanks for your patience. I like the idea of preserving the
> > > original behavior, however this version doesn't seem to flash
> > > consistently. Sometimes it doesn't flash, sometimes it does.
> > >
> > > regards,
> > > Larry
> > >
> > > On 6/18/07, Sascha L. Teichmann < [EMAIL PROTECTED]
> <mailto:[EMAIL PROTECTED]>> wrote:
> > >> Larry,
> > >>
> > >> there is probably somebody out there (younger than us)
> > >> how says that 400ms feels slow too.
> > >>
> > >> I've thought a bit about the compromise and came to the
> > >> conclusion that we don't need a make a compromise here.
> > >>
> > >> We have simply to restore the behavior of the
> > >> original TheadQueue. The original one fires the
> > >> Listeners when the running threads went down to zero.
> > >> We can do the same when we're in the situation that we
> > >> are the last remaining thread with our job done.
> > >>
> > >> In this case the number of running threads is
> > >> one but this measure wasn't reliable in the old
> > >> ThreadQueue too. So it doesn't matter.
> > >> But in difference to the original we keep the
> > >> worker thread alive afterwards instead of killing it.
> > >>
> > >> Find attached a new version of the ThreadQueue that
> > >> implements this behavior.
> > >>
> > >> regards,
> > >> Sascha
> > >>
> > >> Larry Becker schrieb:
> > >>> Sascha,
> > >>>
> > >>> I tried one second, and it feels slow. When I am
> arrowing through a
> > >>> selection of records in View/Edit Attributes it makes me
> wait for the
> > >>> flash before I move on. Really, this is becoming an issue of
> > >>> compromising the interactivity of the application to
> achieve some
> > >>> theoretical benefit that can't be seen or measured.
> > >>>
> > >>> How about 400 ms? That is about the average reaction time.
> > >>>
> > >>> regards,
> > >>> Larry Becker
> > >>
>
> -------------------------------------------------------------------------
> > >> This SF.net email is sponsored by DB2 Express
> > >> Download DB2 Express C - the FREE version of DB2 express and
> take
> > >> control of your XML. No limits. Just data. Click to get it now.
> > >> http://sourceforge.net/powerbar/db2/
> <http://sourceforge.net/powerbar/db2/>
> > >> _______________________________________________
> > >> Jump-pilot-devel mailing list
> > >> Jump-pilot-devel@lists.sourceforge.net
> <mailto:Jump-pilot-devel@lists.sourceforge.net>
> > >> https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel
> > >>
> > >>
> > >>
> > >
> > >
> >
> >
> -------------------------------------------------------------------------
>
> > This SF.net email is sponsored by DB2 Express
> > Download DB2 Express C - the FREE version of DB2 express and take
> > control of your XML. No limits. Just data. Click to get it now.
> > http://sourceforge.net/powerbar/db2/
> > _______________________________________________
> > Jump-pilot-devel mailing list
> > Jump-pilot-devel@lists.sourceforge.net
> <mailto:Jump-pilot-devel@lists.sourceforge.net>
> > https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel
> >
>
>
> --
> http://amusingprogrammer.blogspot.com/
>
>
>
>
> --
> http://amusingprogrammer.blogspot.com/
>
>
>
>
> --
> http://amusingprogrammer.blogspot.com/
>
>
> ------------------------------------------------------------------------
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by DB2 Express
> Download DB2 Express C - the FREE version of DB2 express and take
> control of your XML. No limits. Just data. Click to get it now.
> http://sourceforge.net/powerbar/db2/
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> Jump-pilot-devel mailing list
> Jump-pilot-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
Jump-pilot-devel mailing list
Jump-pilot-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/jump-pilot-devel
